[flang-commits] [flang] [flang][OpenMP] Introduce variant argument, customize OmpArgument par… (PR #160372)
via flang-commits
flang-commits at lists.llvm.org
Tue Sep 23 12:16:11 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-parser
Author: Krzysztof Parzyszek (kparzysz)
<details>
<summary>Changes</summary>
…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:
- (modified) flang/include/flang/Parser/dump-parse-tree.h (+1)
- (modified) flang/include/flang/Parser/parse-tree.h (+13)
- (modified) flang/lib/Parser/openmp-parsers.cpp (+60-8)
- (modified) flang/lib/Parser/unparse.cpp (+5)
- (modified) flang/lib/Semantics/resolve-names.cpp (+4)
``````````diff
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
``````````
</details>
https://github.com/llvm/llvm-project/pull/160372
More information about the flang-commits
mailing list