Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions flang/include/flang/Parser/dump-parse-tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,7 @@ class ParseTreeDumper {
NODE(parser, OmpOtherwiseClause)
NODE(parser, OmpWhenClause)
NODE(OmpWhenClause, Modifier)
NODE(parser, OmpDirectiveName)
NODE(parser, OmpDirectiveSpecification)
NODE(parser, OmpTraitPropertyName)
NODE(parser, OmpTraitScore)
Expand Down
17 changes: 16 additions & 1 deletion flang/include/flang/Parser/parse-tree.h
Original file line number Diff line number Diff line change
Expand Up @@ -3464,6 +3464,18 @@ WRAPPER_CLASS(PauseStmt, std::optional<StopCode>);
struct OmpClause;
struct OmpDirectiveSpecification;

struct OmpDirectiveName {
// No boilerplates: this class should be copyable, movable, etc.
constexpr OmpDirectiveName() = default;
constexpr OmpDirectiveName(const OmpDirectiveName &) = default;
// Construct from an already parsed text. Use Verbatim for this because
// Verbatim's source corresponds to an actual source location.
OmpDirectiveName(const Verbatim &name);
using WrapperTrait = std::true_type;
CharBlock source;
llvm::omp::Directive v{llvm::omp::Directive::OMPD_unknown};
};

// 2.1 Directives or clauses may accept a list or extended-list.
// A list item is a variable, array section or common block name (enclosed
// in slashes). An extended list item is a list item or a procedure Name.
Expand Down Expand Up @@ -4493,7 +4505,10 @@ struct OmpClauseList {
struct OmpDirectiveSpecification {
CharBlock source;
TUPLE_CLASS_BOILERPLATE(OmpDirectiveSpecification);
std::tuple<llvm::omp::Directive, std::optional<std::list<OmpArgument>>,
llvm::omp::Directive DirId() const { //
return std::get<OmpDirectiveName>(t).v;
}
std::tuple<OmpDirectiveName, std::optional<std::list<OmpArgument>>,
std::optional<OmpClauseList>>
t;
};
Expand Down
38 changes: 31 additions & 7 deletions flang/lib/Parser/openmp-parsers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,38 @@ namespace Fortran::parser {
constexpr auto startOmpLine = skipStuffBeforeStatement >> "!$OMP "_sptok;
constexpr auto endOmpLine = space >> endOfLine;

template <typename Parser> struct UnwrapParser {
Copy link
Contributor

Choose a reason for hiding this comment

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

Just for my understanding, this is basically just a way to get to the .v (Directive) of the outer directive struct, becuse we used to directly store the id and without the wrapper.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's the .v part, but in a form of a parser, so that it is composable with other parsers.

static_assert(
Parser::resultType::WrapperTrait::value && "Wrapper class required");
using resultType = decltype(Parser::resultType::v);
constexpr UnwrapParser(Parser p) : parser_(p) {}

std::optional<resultType> Parse(ParseState &state) const {
if (auto result{parser_.Parse(state)}) {
return result->v;
}
return std::nullopt;
}

private:
const Parser parser_;
};

template <typename Parser> constexpr auto unwrap(const Parser &p) {
return UnwrapParser<Parser>(p);
}

/// Parse OpenMP directive name (this includes compound directives).
struct OmpDirectiveNameParser {
using resultType = llvm::omp::Directive;
using resultType = OmpDirectiveName;
using Token = TokenStringMatch<false, false>;

std::optional<resultType> Parse(ParseState &state) const {
for (const NameWithId &nid : directives()) {
if (attempt(Token(nid.first.data())).Parse(state)) {
return nid.second;
OmpDirectiveName n;
n.v = nid.second;
return n;
}
}
return std::nullopt;
Expand Down Expand Up @@ -218,7 +241,7 @@ TYPE_PARSER(construct<OmpTraitSelectorName::Value>(
TYPE_PARSER(sourced(construct<OmpTraitSelectorName>(
// Parse predefined names first (because of SIMD).
construct<OmpTraitSelectorName>(Parser<OmpTraitSelectorName::Value>{}) ||
construct<OmpTraitSelectorName>(OmpDirectiveNameParser{}) ||
construct<OmpTraitSelectorName>(unwrap(OmpDirectiveNameParser{})) ||
// identifier-or-string for extensions
construct<OmpTraitSelectorName>(
applyFunction(nameToString, Parser<Name>{})) ||
Expand Down Expand Up @@ -480,7 +503,8 @@ TYPE_PARSER(sourced(construct<OmpFromClause::Modifier>(
TYPE_PARSER(sourced(
construct<OmpGrainsizeClause::Modifier>(Parser<OmpPrescriptiveness>{})))

TYPE_PARSER(sourced(construct<OmpIfClause::Modifier>(OmpDirectiveNameParser{})))
TYPE_PARSER(
sourced(construct<OmpIfClause::Modifier>(unwrap(OmpDirectiveNameParser{}))))

TYPE_PARSER(sourced(construct<OmpInReductionClause::Modifier>(
Parser<OmpReductionIdentifier>{})))
Expand Down Expand Up @@ -775,9 +799,9 @@ TYPE_PARSER(construct<OmpMessageClause>(expr))

TYPE_PARSER(construct<OmpHoldsClause>(indirect(expr)))
TYPE_PARSER(construct<OmpAbsentClause>(many(maybe(","_tok) >>
construct<llvm::omp::Directive>(OmpDirectiveNameParser{}))))
construct<llvm::omp::Directive>(unwrap(OmpDirectiveNameParser{})))))
TYPE_PARSER(construct<OmpContainsClause>(many(maybe(","_tok) >>
construct<llvm::omp::Directive>(OmpDirectiveNameParser{}))))
construct<llvm::omp::Directive>(unwrap(OmpDirectiveNameParser{})))))

TYPE_PARSER("ABSENT" >> construct<OmpClause>(construct<OmpClause::Absent>(
parenthesized(Parser<OmpAbsentClause>{}))) ||
Expand Down Expand Up @@ -972,7 +996,7 @@ TYPE_PARSER(sourced(construct<OmpErrorDirective>(
// --- Parsers for directives and constructs --------------------------

TYPE_PARSER(sourced(construct<OmpDirectiveSpecification>( //
OmpDirectiveNameParser{},
sourced(OmpDirectiveNameParser{}),
maybe(parenthesized(nonemptyList(Parser<OmpArgument>{}))),
maybe(Parser<OmpClauseList>{}))))

Expand Down
15 changes: 15 additions & 0 deletions flang/lib/Parser/parse-tree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,21 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &os, const Name &x) {
return os << x.ToString();
}

OmpDirectiveName::OmpDirectiveName(const Verbatim &name) {
std::string_view nameView{name.source.begin(), name.source.size()};
std::string nameLower{ToLowerCaseLetters(nameView)};
// If the name was actually "unknown" then accept it, otherwise flag
Copy link
Contributor

Choose a reason for hiding this comment

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

This reads a bit "akward", at first I thought it was in fact backwards. Perhaps If the name is not "unknown", look up the kind. Result should not be OMPD_unknown. Accept "unknown" if that is the name.

Or, perhaps rearrange the code so it says if (nameLower == "unknown") v = OMPD_unkown; else ... .

Feel free to fix in any other way that you see fit.

Or, could we just do something like this:

   v = omp::getOpenMPDIrectiveKind(nameLower)
   assert(v != llvm::omp::Directive::OMPD_unknown || nameLower == "unknown" && "Unrecognized directive");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I get it. Let me rephrase that in a clearer way...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I'm just going to assert on "unknown" as well. There is no need to treat it as a special case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, simpler code with much clearer comment = good thing! :)

// OMPD_unknown (the default return value from getOpenMPDirectiveKind)
// as an error.
if (nameLower != "unknown") {
v = llvm::omp::getOpenMPDirectiveKind(nameLower);
assert(v != llvm::omp::Directive::OMPD_unknown && "Unrecognized directive");
} else {
v = llvm::omp::Directive::OMPD_unknown;
}
source = name.source;
}

OmpDependenceType::Value OmpDoacross::GetDepType() const {
return common::visit( //
common::visitors{
Expand Down
2 changes: 1 addition & 1 deletion flang/lib/Parser/unparse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2095,7 +2095,7 @@ class UnparseVisitor {
}
void Unparse(const OmpDirectiveSpecification &x) {
using ArgList = std::list<parser::OmpArgument>;
Walk(std::get<llvm::omp::Directive>(x.t));
Walk(std::get<OmpDirectiveName>(x.t));
if (auto &args{std::get<std::optional<ArgList>>(x.t)}) {
Put("(");
Walk(*args);
Expand Down
2 changes: 1 addition & 1 deletion flang/lib/Semantics/check-omp-structure.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ void OmpStructureChecker::CheckHintClause(
}

void OmpStructureChecker::Enter(const parser::OmpDirectiveSpecification &x) {
PushContextAndClauseSets(x.source, std::get<llvm::omp::Directive>(x.t));
PushContextAndClauseSets(x.source, x.DirId());
}

void OmpStructureChecker::Leave(const parser::OmpDirectiveSpecification &) {
Expand Down
2 changes: 1 addition & 1 deletion flang/lib/Semantics/resolve-directives.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
}

bool Pre(const parser::OmpDirectiveSpecification &x) {
PushContext(x.source, std::get<llvm::omp::Directive>(x.t));
PushContext(x.source, x.DirId());
return true;
}
void Post(const parser::OmpDirectiveSpecification &) { PopContext(); }
Expand Down
3 changes: 1 addition & 2 deletions flang/lib/Semantics/resolve-names.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1775,11 +1775,10 @@ bool OmpVisitor::Pre(const parser::OmpDirectiveSpecification &x) {
// Disable the semantic analysis for it for now to allow the compiler to
// parse METADIRECTIVE without flagging errors.
AddOmpSourceRange(x.source);
auto dirId{std::get<llvm::omp::Directive>(x.t)};
auto &maybeArgs{std::get<std::optional<std::list<parser::OmpArgument>>>(x.t)};
auto &maybeClauses{std::get<std::optional<parser::OmpClauseList>>(x.t)};

switch (dirId) {
switch (x.DirId()) {
case llvm::omp::Directive::OMPD_declare_mapper:
if (maybeArgs && maybeClauses) {
const parser::OmpArgument &first{maybeArgs->front()};
Expand Down
16 changes: 8 additions & 8 deletions flang/test/Parser/OpenMP/metadirective-dirspec.f90
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ subroutine f00(x)
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = allocate
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = allocate
!PARSE-TREE: | | | OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 'x'
!PARSE-TREE: | | | OmpClauseList ->

Expand All @@ -51,7 +51,7 @@ subroutine f01(x)
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = critical
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = critical
!PARSE-TREE: | | | OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 'x'
!PARSE-TREE: | | | OmpClauseList ->

Expand All @@ -76,7 +76,7 @@ subroutine f02
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = declare mapper
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = declare mapper
!PARSE-TREE: | | | OmpArgument -> OmpMapperSpecifier
!PARSE-TREE: | | | | Name = 'mymapper'
!PARSE-TREE: | | | | TypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec ->
Expand Down Expand Up @@ -120,7 +120,7 @@ subroutine f03
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = declare reduction
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = declare reduction
!PARSE-TREE: | | | OmpArgument -> OmpReductionSpecifier
!PARSE-TREE: | | | | OmpReductionIdentifier -> DefinedOperator -> IntrinsicOperator = Add
!PARSE-TREE: | | | | OmpTypeNameList -> OmpTypeSpecifier -> TypeSpec -> DerivedTypeSpec
Expand Down Expand Up @@ -158,7 +158,7 @@ subroutine f04
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = declare simd
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = declare simd
!PARSE-TREE: | | | OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 'f04'
!PARSE-TREE: | | | OmpClauseList ->
!PARSE-TREE: ImplicitPart ->
Expand All @@ -183,7 +183,7 @@ subroutine f05
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = declare target
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = declare target
!PARSE-TREE: | | | OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 'f05'
!PARSE-TREE: | | | OmpClauseList ->
!PARSE-TREE: ImplicitPart ->
Expand All @@ -210,7 +210,7 @@ subroutine f06(x, y)
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = flush
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = flush
!PARSE-TREE: | | | OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 'x'
!PARSE-TREE: | | | OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 'y'
!PARSE-TREE: | | | OmpClauseList ->
Expand All @@ -237,6 +237,6 @@ subroutine f07
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = threadprivate
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = threadprivate
!PARSE-TREE: | | | OmpArgument -> OmpLocator -> OmpObject -> Designator -> DataRef -> Name = 't'
!PARSE-TREE: | | | OmpClauseList ->
4 changes: 2 additions & 2 deletions flang/test/Parser/OpenMP/metadirective-v50.f90
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ subroutine f01
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpClauseList ->
!PARSE-TREE: | OmpClause -> Default -> OmpDefaultClause -> OmpDirectiveSpecification
!PARSE-TREE: | | llvm::omp::Directive = nothing
!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | OmpClauseList ->
24 changes: 12 additions & 12 deletions flang/test/Parser/OpenMP/metadirective.f90
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ subroutine f00
!PARSE-TREE: | | | OmpTraitSelector
!PARSE-TREE: | | | | OmpTraitSelectorName -> llvm::omp::Directive = parallel
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpClauseList ->

subroutine f01
Expand All @@ -47,7 +47,7 @@ subroutine f01
!PARSE-TREE: | | | | | OmpTraitProperty -> Scalar -> Expr = '1_4'
!PARSE-TREE: | | | | | | LiteralConstant -> IntLiteralConstant = '1'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpClauseList ->

subroutine f02
Expand All @@ -74,7 +74,7 @@ subroutine f02
!PARSE-TREE: | | | | | OmpTraitProperty -> Scalar -> Expr = '7_4'
!PARSE-TREE: | | | | | | LiteralConstant -> IntLiteralConstant = '7'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpClauseList ->

subroutine f03
Expand All @@ -98,7 +98,7 @@ subroutine f03
!PARSE-TREE: | | | | Properties
!PARSE-TREE: | | | | | OmpTraitProperty -> OmpClause -> AcqRel
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpClauseList ->

subroutine f04
Expand Down Expand Up @@ -132,7 +132,7 @@ subroutine f04
!PARSE-TREE: | | | | | | | OmpTraitPropertyExtension -> Scalar -> Expr = '1_4'
!PARSE-TREE: | | | | | | | | LiteralConstant -> IntLiteralConstant = '1'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpClauseList ->

subroutine f05(x)
Expand Down Expand Up @@ -168,12 +168,12 @@ subroutine f05(x)
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = parallel do
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = parallel do
!PARSE-TREE: | | | OmpClauseList -> OmpClause -> Reduction -> OmpReductionClause
!PARSE-TREE: | | | | Modifier -> OmpReductionIdentifier -> DefinedOperator -> IntrinsicOperator = Add
!PARSE-TREE: | | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
!PARSE-TREE: | OmpClause -> Otherwise -> OmpOtherwiseClause -> OmpDirectiveSpecification
!PARSE-TREE: | | llvm::omp::Directive = nothing
!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | OmpClauseList ->

subroutine f06
Expand Down Expand Up @@ -207,7 +207,7 @@ subroutine f06
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | | OmpClauseList ->

subroutine f07
Expand All @@ -232,7 +232,7 @@ subroutine f07
!PARSE-TREE: | | | | Properties
!PARSE-TREE: | | | | | OmpTraitProperty -> OmpTraitPropertyName -> string = 'amd'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = declare simd
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = declare simd
!PARSE-TREE: | | | OmpClauseList ->
!PARSE-TREE: | OmpClause -> When -> OmpWhenClause
!PARSE-TREE: | | Modifier -> OmpContextSelectorSpecification -> OmpTraitSetSelector
Expand All @@ -244,8 +244,8 @@ subroutine f07
!PARSE-TREE: | | | | | | LiteralConstant -> LogicalLiteralConstant
!PARSE-TREE: | | | | | | | bool = 'true'
!PARSE-TREE: | | OmpDirectiveSpecification
!PARSE-TREE: | | | llvm::omp::Directive = declare target
!PARSE-TREE: | | | OmpDirectiveName -> llvm::omp::Directive = declare target
!PARSE-TREE: | | | OmpClauseList ->
!PARSE-TREE: | OmpClause -> Otherwise -> OmpOtherwiseClause -> OmpDirectiveSpecification
!PARSE-TREE: | | llvm::omp::Directive = nothing
!PARSE-TREE: | | OmpClauseList ->
!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = nothing
!PARSE-TREE: | | OmpClauseList ->
Loading