[flang][OpenMP] Introduce variant argument, customize OmpArgument par… #160372
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…sing The DECLARE_VARIANT directive takes two names separated by a colon as an argument: base-name:variant-name. Define OmpBaseVariantNames to represent this, since no existing argument alternative matches it. However, there is an issue. The syntax "name1:name2" can be the argument to DECLARE_VARIANT (if both names are OmpObjects), but it can also be a reduction-specifier if "name2" is a type. This conflict can only be resolved once we know what the names are, which is after name resolution has visited them. The problem is that name resolution has side-effects that may be (practically) impossible to undo (e.g. creating new symbols, emitting diagnostic messages). To avoid this problem this PR makes the parsing of OmpArgument directive- sensitive: when the directive is DECLARE_VARIANT, don't attempt to parse a reduction-specifier, consider OmpBaseVariantNames instead. Otherwise ignore OmpBaseVariantNames in favor of reduction-specifier.
llvmbot
added
flang
Member
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-parser Author: Krzysztof Parzyszek (kparzysz) Changes…sing The DECLARE_VARIANT directive takes two names separated by a colon as an argument: base-name:variant-name. Define OmpBaseVariantNames to represent this, since no existing argument alternative matches it. However, there is an issue. The syntax "name1:name2" can be the argument to DECLARE_VARIANT (if both names are OmpObjects), but it can also be a reduction-specifier if "name2" is a type. This conflict can only be resolved once we know what the names are, which is after name resolution has visited them. The problem is that name resolution has side-effects that may be (practically) impossible to undo (e.g. creating new symbols, emitting diagnostic messages). To avoid this problem this PR makes the parsing of OmpArgument directive- sensitive: when the directive is DECLARE_VARIANT, don't attempt to parse a reduction-specifier, consider OmpBaseVariantNames instead. Otherwise ignore OmpBaseVariantNames in favor of reduction-specifier. Full diff: https://github.com/llvm/llvm-project/pull/160372.diff 5 Files Affected:
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index b2341226c7688..7540d38baa584 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -522,6 +522,7 @@ class ParseTreeDumper {
NODE(parser, OmpAtomicDefaultMemOrderClause)
NODE(parser, OmpAutomapModifier)
NODE_ENUM(OmpAutomapModifier, Value)
+ NODE(parser, OmpBaseVariantNames)
NODE(parser, OmpBeginDirective)
NODE(parser, OmpBeginLoopDirective)
NODE(parser, OmpBeginSectionsDirective)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 39dbeb5e7cfbe..4808a5b844a6f 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3555,6 +3555,18 @@ struct OmpLocator {
WRAPPER_CLASS(OmpLocatorList, std::list<OmpLocator>);
+// Ref: [4.5:58-60], [5.0:58-60], [5.1:63-68], [5.2:197-198], [6.0:334-336]
+//
+// Argument to DECLARE VARIANT with the base-name present. (When only
+// variant-name is present, it is a simple OmpObject).
+//
+// base-name-variant-name -> // since 4.5
+// base-name : variant-name
+struct OmpBaseVariantNames {
+ TUPLE_CLASS_BOILERPLATE(OmpBaseVariantNames);
+ std::tuple<OmpObject, OmpObject> t;
+};
+
// Ref: [5.0:326:10-16], [5.1:359:5-11], [5.2:163:2-7], [6.0:293:16-21]
//
// mapper-specifier ->
@@ -3584,6 +3596,7 @@ struct OmpArgument {
CharBlock source;
UNION_CLASS_BOILERPLATE(OmpArgument);
std::variant<OmpLocator, // {variable, extended, locator}-list-item
+ OmpBaseVariantNames, // base-name:variant-name
OmpMapperSpecifier, OmpReductionSpecifier>
u;
};
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 30bc02ce851db..3b32e1a4a67b1 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -315,15 +315,56 @@ TYPE_PARSER( //
construct<OmpLocator>(Parser<OmpObject>{}) ||
construct<OmpLocator>(Parser<FunctionReference>{}))
-TYPE_PARSER(sourced( //
- construct<OmpArgument>(Parser<OmpMapperSpecifier>{}) ||
- construct<OmpArgument>(Parser<OmpReductionSpecifier>{}) ||
- construct<OmpArgument>(Parser<OmpLocator>{})))
+TYPE_PARSER(construct<OmpBaseVariantNames>(
+ Parser<OmpObject>{} / ":", Parser<OmpObject>{}))
+
+// Make the parsing of OmpArgument directive-sensitive. The issue is that
+// name1:name2 can match either OmpBaseVariantNames or OmpReductionSpecifier.
+// In the former case, "name2" is a name of a function, in the latter, of a
+// type. To resolve the conflict we need information provided by name
+// resolution, but by that time we can't modify the AST anymore, and the
+// name resolution may have implicitly declared a symbol, or issued a message.
+template <llvm::omp::Directive Id = llvm::omp::Directive::OMPD_unknown>
+struct OmpArgumentParser {
+ using resultType = OmpArgument;
+
+ std::optional<resultType> Parse(ParseState &state) const {
+ constexpr auto parser{sourced(first( //
+ construct<OmpArgument>(Parser<OmpMapperSpecifier>{}),
+ // By default, prefer OmpReductionSpecifier over OmpBaseVariantNames.
+ construct<OmpArgument>(Parser<OmpReductionSpecifier>{}),
+ construct<OmpArgument>(Parser<OmpLocator>{})))};
+ return parser.Parse(state);
+ }
+};
+
+template <>
+struct OmpArgumentParser<llvm::omp::Directive::OMPD_declare_variant> {
+ using resultType = OmpArgument;
+
+ std::optional<resultType> Parse(ParseState &state) const {
+ constexpr auto parser{sourced(first( //
+ construct<OmpArgument>(Parser<OmpMapperSpecifier>{}),
+ // In DECLARE_VARIANT parse OmpBaseVariantNames instead of
+ // OmpReductionSpecifier.
+ construct<OmpArgument>(Parser<OmpBaseVariantNames>{}),
+ construct<OmpArgument>(Parser<OmpLocator>{})))};
+ return parser.Parse(state);
+ }
+};
TYPE_PARSER(construct<OmpLocatorList>(nonemptyList(Parser<OmpLocator>{})))
-TYPE_PARSER(sourced( //
- construct<OmpArgumentList>(nonemptyList(Parser<OmpArgument>{}))))
+template <llvm::omp::Directive Id = llvm::omp::Directive::OMPD_unknown>
+struct OmpArgumentListParser {
+ using resultType = OmpArgumentList;
+
+ std::optional<resultType> Parse(ParseState &state) const {
+ return sourced(
+ construct<OmpArgumentList>(nonemptyList(OmpArgumentParser<Id>{})))
+ .Parse(state);
+ }
+};
TYPE_PARSER( //
construct<OmpTypeSpecifier>(Parser<DeclarationTypeSpec>{}) ||
@@ -1312,12 +1353,23 @@ TYPE_PARSER(
applyFunction<OmpDirectiveSpecification>(makeFlushFromOldSyntax,
verbatim("FLUSH"_tok) / !lookAhead("("_tok),
maybe(Parser<OmpClauseList>{}),
- maybe(parenthesized(Parser<OmpArgumentList>{})),
+ maybe(parenthesized(
+ OmpArgumentListParser<llvm::omp::Directive::OMPD_flush>{})),
pure(OmpDirectiveSpecification::Flags::DeprecatedSyntax)))) ||
+ // Parse DECLARE_VARIANT individually, because the "[base:]variant"
+ // argument will conflict with DECLARE_REDUCTION's "ident:types...".
+ predicated(Parser<OmpDirectiveName>{},
+ IsDirective(llvm::omp::Directive::OMPD_declare_variant)) >=
+ sourced(construct<OmpDirectiveSpecification>(
+ sourced(OmpDirectiveNameParser{}),
+ maybe(parenthesized(OmpArgumentListParser<
+ llvm::omp::Directive::OMPD_declare_variant>{})),
+ maybe(Parser<OmpClauseList>{}),
+ pure(OmpDirectiveSpecification::Flags::None))) ||
// Parse the standard syntax: directive [(arguments)] [clauses]
sourced(construct<OmpDirectiveSpecification>( //
sourced(OmpDirectiveNameParser{}),
- maybe(parenthesized(Parser<OmpArgumentList>{})),
+ maybe(parenthesized(OmpArgumentListParser<>{})),
maybe(Parser<OmpClauseList>{}),
pure(OmpDirectiveSpecification::Flags::None))))
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 3455b535ccb51..1b3eef0eefba3 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2089,6 +2089,11 @@ class UnparseVisitor {
// OpenMP Clauses & Directives
void Unparse(const OmpArgumentList &x) { Walk(x.v, ", "); }
+ void Unparse(const OmpBaseVariantNames &x) {
+ Walk(std::get<0>(x.t)); // OmpObject
+ Put(":");
+ Walk(std::get<1>(x.t)); // OmpObject
+ }
void Unparse(const OmpTypeNameList &x) { //
Walk(x.v, ",");
}
diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp
index 699cb562da8cc..b73d794c11d31 100644
--- a/flang/lib/Semantics/resolve-names.cpp
+++ b/flang/lib/Semantics/resolve-names.cpp
@@ -1998,6 +1998,10 @@ bool OmpVisitor::Pre(const parser::OmpDirectiveSpecification &x) {
ProcessReductionSpecifier(spec, clauses);
visitClauses = false;
},
+ [&](const parser::OmpBaseVariantNames &names) {
+ Walk(std::get<0>(names.t));
+ Walk(std::get<1>(names.t));
+ },
[&](const parser::OmpLocator &locator) {
// Manually resolve names in CRITICAL directives. This is because
// these names do not denote Fortran objects, and the CRITICAL
|
This was referenced Sep 23, 2025
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/157/builds/40422 Here is the relevant piece of the build log for the reference |
mahesh-attarde
pushed a commit
to mahesh-attarde/llvm-project
that referenced
this pull request
Oct 3, 2025
llvm#160372) …sing The DECLARE_VARIANT directive takes two names separated by a colon as an argument: base-name:variant-name. Define OmpBaseVariantNames to represent this, since no existing argument alternative matches it. However, there is an issue. The syntax "name1:name2" can be the argument to DECLARE_VARIANT (if both names are OmpObjects), but it can also be a reduction-specifier if "name2" is a type. This conflict can only be resolved once we know what the names are, which is after name resolution has visited them. The problem is that name resolution has side-effects that may be (practically) impossible to undo (e.g. creating new symbols, emitting diagnostic messages). To avoid this problem this PR makes the parsing of OmpArgument directive- sensitive: when the directive is DECLARE_VARIANT, don't attempt to parse a reduction-specifier, consider OmpBaseVariantNames instead. Otherwise ignore OmpBaseVariantNames in favor of reduction-specifier.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…sing
The DECLARE_VARIANT directive takes two names separated by a colon as an argument: base-name:variant-name. Define OmpBaseVariantNames to represent this, since no existing argument alternative matches it.
However, there is an issue. The syntax "name1:name2" can be the argument to DECLARE_VARIANT (if both names are OmpObjects), but it can also be a reduction-specifier if "name2" is a type. This conflict can only be resolved once we know what the names are, which is after name resolution has visited them. The problem is that name resolution has side-effects that may be (practically) impossible to undo (e.g. creating new symbols, emitting diagnostic messages).
To avoid this problem this PR makes the parsing of OmpArgument directive- sensitive: when the directive is DECLARE_VARIANT, don't attempt to parse a reduction-specifier, consider OmpBaseVariantNames instead. Otherwise ignore OmpBaseVariantNames in favor of reduction-specifier.