Skip to content

[flang] Updated the parsing structure of some OpenAcc constructs to give better/more uniform inspection #138076

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions flang/include/flang/Parser/dump-parse-tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,15 @@ class ParseTreeDumper {
NODE(format, IntrinsicTypeDataEditDesc)
NODE(format::IntrinsicTypeDataEditDesc, Kind)
NODE(parser, Abstract)
NODE(parser, AccAtomicCaptureDirective)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These used to be in alphabetic order. Now they are not.

NODE(parser, AccAtomicCapture)
NODE(AccAtomicCapture, Stmt1)
NODE(AccAtomicCapture, Stmt2)
NODE(parser, AccAtomicReadDirective)
NODE(parser, AccAtomicRead)
NODE(parser, AccAtomicUpdateDirective)
NODE(parser, AccAtomicUpdate)
NODE(parser, AccAtomicWriteDirective)
NODE(parser, AccAtomicWrite)
NODE(parser, AccBeginBlockDirective)
NODE(parser, AccBeginCombinedDirective)
Expand Down
45 changes: 39 additions & 6 deletions flang/include/flang/Parser/parse-tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -5243,38 +5243,65 @@ struct AccEndBlockDirective {
};

// ACC END ATOMIC
EMPTY_CLASS(AccEndAtomic);
struct AccEndAtomic {
WRAPPER_CLASS_BOILERPLATE(AccEndAtomic, Verbatim);
CharBlock source;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the Verbatim parse tree node has a source data member. That is its point. This new class doesn't add any value.

};

// ACC ATOMIC READ
struct AccAtomicReadDirective {
TUPLE_CLASS_BOILERPLATE(AccAtomicReadDirective);
std::tuple<Verbatim, AccClauseList> t;
CharBlock source;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both Verbatim and AccClauseList nodes have source data members. Why do you need this one?

};

struct AccAtomicRead {
TUPLE_CLASS_BOILERPLATE(AccAtomicRead);
std::tuple<Verbatim, AccClauseList, Statement<AssignmentStmt>,
std::tuple<AccAtomicReadDirective, Statement<AssignmentStmt>,
std::optional<AccEndAtomic>>
t;
};

// ACC ATOMIC WRITE
struct AccAtomicWriteDirective {
TUPLE_CLASS_BOILERPLATE(AccAtomicWriteDirective);
std::tuple<Verbatim, AccClauseList> t;
CharBlock source;
};

struct AccAtomicWrite {
TUPLE_CLASS_BOILERPLATE(AccAtomicWrite);
std::tuple<Verbatim, AccClauseList, Statement<AssignmentStmt>,
std::tuple<AccAtomicWriteDirective, Statement<AssignmentStmt>,
std::optional<AccEndAtomic>>
t;
};

// ACC ATOMIC UPDATE
struct AccAtomicUpdateDirective {
TUPLE_CLASS_BOILERPLATE(AccAtomicUpdateDirective);
std::tuple<std::optional<Verbatim>, AccClauseList> t;
CharBlock source;
};

struct AccAtomicUpdate {
TUPLE_CLASS_BOILERPLATE(AccAtomicUpdate);
std::tuple<std::optional<Verbatim>, AccClauseList, Statement<AssignmentStmt>,
std::tuple<AccAtomicUpdateDirective, Statement<AssignmentStmt>,
std::optional<AccEndAtomic>>
t;
};

// ACC ATOMIC CAPTURE
struct AccAtomicCaptureDirective {
TUPLE_CLASS_BOILERPLATE(AccAtomicCaptureDirective);
std::tuple<Verbatim, AccClauseList> t;
CharBlock source;
};

struct AccAtomicCapture {
TUPLE_CLASS_BOILERPLATE(AccAtomicCapture);
WRAPPER_CLASS(Stmt1, Statement<AssignmentStmt>);
WRAPPER_CLASS(Stmt2, Statement<AssignmentStmt>);
std::tuple<Verbatim, AccClauseList, Stmt1, Stmt2, AccEndAtomic> t;
std::tuple<AccAtomicCaptureDirective, Stmt1, Stmt2, AccEndAtomic> t;
};

struct OpenACCAtomicConstruct {
Expand All @@ -5287,6 +5314,7 @@ struct OpenACCAtomicConstruct {
struct OpenACCBlockConstruct {
TUPLE_CLASS_BOILERPLATE(OpenACCBlockConstruct);
std::tuple<AccBeginBlockDirective, Block, AccEndBlockDirective> t;
CharBlock source;
};

struct OpenACCStandaloneDeclarativeConstruct {
Expand Down Expand Up @@ -5324,14 +5352,19 @@ struct OpenACCDeclarativeConstruct {
};

// OpenACC directives enclosing do loop
EMPTY_CLASS(AccEndLoop);
struct AccEndLoop {
WRAPPER_CLASS_BOILERPLATE(AccEndLoop, Verbatim);
CharBlock source;
};

struct OpenACCLoopConstruct {
TUPLE_CLASS_BOILERPLATE(OpenACCLoopConstruct);
OpenACCLoopConstruct(AccBeginLoopDirective &&a)
: t({std::move(a), std::nullopt, std::nullopt}) {}
std::tuple<AccBeginLoopDirective, std::optional<DoConstruct>,
std::optional<AccEndLoop>>
t;
CharBlock source;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need a source data member that covers an entire construct?

};

struct OpenACCEndConstruct {
Expand Down
51 changes: 28 additions & 23 deletions flang/lib/Parser/openacc-parsers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -169,12 +169,12 @@ TYPE_PARSER(sourced(construct<AccLoopDirective>(
TYPE_PARSER(construct<AccBeginLoopDirective>(
sourced(Parser<AccLoopDirective>{}), Parser<AccClauseList>{}))

TYPE_PARSER(construct<AccEndLoop>("END LOOP"_tok))
TYPE_PARSER(
sourced(construct<AccEndLoop>(startAccLine >> verbatim("END LOOP"_tok))))

TYPE_PARSER(construct<OpenACCLoopConstruct>(
TYPE_PARSER(sourced(construct<OpenACCLoopConstruct>(
sourced(Parser<AccBeginLoopDirective>{} / endAccLine),
maybe(Parser<DoConstruct>{}),
maybe(startAccLine >> Parser<AccEndLoop>{} / endAccLine)))
maybe(Parser<DoConstruct>{}), maybe(Parser<AccEndLoop>{} / endAccLine))))

// 2.15.1 Routine directive
TYPE_PARSER(sourced(construct<OpenACCRoutineConstruct>(verbatim("ROUTINE"_tok),
Expand All @@ -190,27 +190,32 @@ TYPE_PARSER(construct<AccBeginCombinedDirective>(
sourced(Parser<AccCombinedDirective>{}), Parser<AccClauseList>{}))

// 2.12 Atomic constructs
TYPE_PARSER(construct<AccEndAtomic>(startAccLine >> "END ATOMIC"_tok))
TYPE_PARSER(sourced(
construct<AccEndAtomic>(startAccLine >> verbatim("END ATOMIC"_tok))))

TYPE_PARSER("ATOMIC" >> construct<AccAtomicRead>(verbatim("READ"_tok),
Parser<AccClauseList>{} / endAccLine,
statement(assignmentStmt),
maybe(Parser<AccEndAtomic>{} / endAccLine)))
TYPE_PARSER(sourced(construct<AccAtomicReadDirective>(
"ATOMIC" >> verbatim("READ"_tok), Parser<AccClauseList>{})))
TYPE_PARSER(
construct<AccAtomicRead>(Parser<AccAtomicReadDirective>{} / endAccLine,
statement(assignmentStmt), maybe(Parser<AccEndAtomic>{} / endAccLine)))

TYPE_PARSER("ATOMIC" >> construct<AccAtomicWrite>(verbatim("WRITE"_tok),
Parser<AccClauseList>{} / endAccLine,
statement(assignmentStmt),
maybe(Parser<AccEndAtomic>{} / endAccLine)))
TYPE_PARSER(sourced(construct<AccAtomicWriteDirective>(
"ATOMIC" >> verbatim("WRITE"_tok), Parser<AccClauseList>{})))
TYPE_PARSER(
construct<AccAtomicWrite>(Parser<AccAtomicWriteDirective>{} / endAccLine,
statement(assignmentStmt), maybe(Parser<AccEndAtomic>{} / endAccLine)))

TYPE_PARSER("ATOMIC" >>
construct<AccAtomicUpdate>(maybe(verbatim("UPDATE"_tok)),
Parser<AccClauseList>{} / endAccLine, statement(assignmentStmt),
maybe(Parser<AccEndAtomic>{} / endAccLine)))
TYPE_PARSER(sourced(construct<AccAtomicUpdateDirective>(
"ATOMIC" >> maybe(verbatim("UPDATE"_tok)), Parser<AccClauseList>{})))
TYPE_PARSER(
construct<AccAtomicUpdate>(Parser<AccAtomicUpdateDirective>{} / endAccLine,
statement(assignmentStmt), maybe(Parser<AccEndAtomic>{} / endAccLine)))

TYPE_PARSER("ATOMIC" >>
construct<AccAtomicCapture>(verbatim("CAPTURE"_tok),
Parser<AccClauseList>{} / endAccLine, statement(assignmentStmt),
statement(assignmentStmt), Parser<AccEndAtomic>{} / endAccLine))
TYPE_PARSER(sourced(construct<AccAtomicCaptureDirective>(
"ATOMIC" >> verbatim("CAPTURE"_tok), Parser<AccClauseList>{})))
TYPE_PARSER(construct<AccAtomicCapture>(
Parser<AccAtomicCaptureDirective>{} / endAccLine, statement(assignmentStmt),
statement(assignmentStmt), Parser<AccEndAtomic>{} / endAccLine))

TYPE_PARSER(
sourced(construct<OpenACCAtomicConstruct>(Parser<AccAtomicRead>{})) ||
Expand Down Expand Up @@ -240,14 +245,14 @@ TYPE_PARSER(startAccLine >> sourced(construct<AccEndBlockDirective>("END"_tok >>
construct<AccBlockDirective>(pure(
llvm::acc::Directive::ACCD_data))))))

TYPE_PARSER(construct<OpenACCBlockConstruct>(
TYPE_PARSER(sourced(construct<OpenACCBlockConstruct>(
Parser<AccBeginBlockDirective>{} / endAccLine, block,
// NB, This allows mismatched directives, but semantics checks that they
// match.
recovery(withMessage("expected OpenACC end block directive"_err_en_US,
attempt(Parser<AccEndBlockDirective>{} / endAccLine)),
construct<AccEndBlockDirective>(construct<AccBlockDirective>(
pure(llvm::acc::Directive::ACCD_data))))))
pure(llvm::acc::Directive::ACCD_data)))))))

// Standalone constructs
TYPE_PARSER(construct<OpenACCStandaloneConstruct>(
Expand Down