[flang-commits] [flang] c859323 - [flang][OpenMP] Make parsing of trait properties more context-sensitive (#122900)
via flang-commits
flang-commits at lists.llvm.org
Wed Jan 29 09:54:56 PST 2025
Author: Krzysztof Parzyszek
Date: 2025-01-29T11:54:52-06:00
New Revision: c8593239a3b5a8864c2a315db33bbcb7b68f3da0
URL: https://github.com/llvm/llvm-project/commit/c8593239a3b5a8864c2a315db33bbcb7b68f3da0
DIFF: https://github.com/llvm/llvm-project/commit/c8593239a3b5a8864c2a315db33bbcb7b68f3da0.diff
LOG: [flang][OpenMP] Make parsing of trait properties more context-sensitive (#122900)
A trait poperty can be one of serveral alternatives (name, expression,
etc.), and each property in a list was parsed as if it could be any of
these alternatives independently from other properties. This made the
parsing vulnerable to certain ambiguities in the trait grammar (provided
in the OpenMP spec).
At the same time the OpenMP spec gives the expected types of properties
for almost every trait: all properties listed for a given trait are
usually of the same type, e.g. names, clauses, etc.
Incorporate these restrictions into the parser, and additionally use
property extensions as the fallback if the parsing of the expected
property type failed. This is intended to allow the parser to succeed,
and instead let the semantic-checking code emit a more user-friendly
message.
Added:
Modified:
flang/include/flang/Parser/dump-parse-tree.h
flang/include/flang/Parser/parse-tree.h
flang/lib/Parser/openmp-parsers.cpp
flang/lib/Parser/unparse.cpp
Removed:
################################################################################
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 3f332ec9806a36..1ff006e36334c5 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -480,7 +480,7 @@ class ParseTreeDumper {
NODE(parser, OmpTraitPropertyName)
NODE(parser, OmpTraitScore)
NODE(parser, OmpTraitPropertyExtension)
- NODE(OmpTraitPropertyExtension, ExtensionValue)
+ NODE(OmpTraitPropertyExtension, Complex)
NODE(parser, OmpTraitProperty)
NODE(parser, OmpTraitSelectorName)
NODE_ENUM(OmpTraitSelectorName, Value)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index d1e4048edd9b95..4eac8bca48978f 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3508,37 +3508,22 @@ struct OmpTraitScore {
};
// trait-property-extension ->
-// trait-property-name (trait-property-value, ...)
-// trait-property-value ->
// trait-property-name |
-// scalar-integer-expression |
-// trait-property-extension
-//
-// The grammar in OpenMP 5.2+ spec is ambiguous, the above is a
diff erent
-// version (but equivalent) that doesn't have ambiguities.
-// The ambiguity is in
-// trait-property:
-// trait-property-name <- (a)
-// trait-property-clause
-// trait-property-expression <- (b)
-// trait-property-extension <- this conflicts with (a) and (b)
-// trait-property-extension:
-// trait-property-name <- conflict with (a)
-// identifier(trait-property-extension[, trait-property-extension[, ...]])
-// constant integer expression <- conflict with (b)
+// scalar-expr |
+// trait-property-name (trait-property-extension, ...)
//
struct OmpTraitPropertyExtension {
CharBlock source;
- TUPLE_CLASS_BOILERPLATE(OmpTraitPropertyExtension);
- struct ExtensionValue {
+ UNION_CLASS_BOILERPLATE(OmpTraitPropertyExtension);
+ struct Complex { // name (prop-ext, prop-ext, ...)
CharBlock source;
- UNION_CLASS_BOILERPLATE(ExtensionValue);
- std::variant<OmpTraitPropertyName, ScalarExpr,
- common::Indirection<OmpTraitPropertyExtension>>
- u;
+ TUPLE_CLASS_BOILERPLATE(Complex);
+ std::tuple<OmpTraitPropertyName,
+ std::list<common::Indirection<OmpTraitPropertyExtension>>>
+ t;
};
- using ExtensionList = std::list<ExtensionValue>;
- std::tuple<OmpTraitPropertyName, ExtensionList> t;
+
+ std::variant<OmpTraitPropertyName, ScalarExpr, Complex> u;
};
// trait-property ->
@@ -3571,9 +3556,10 @@ struct OmpTraitProperty {
// UID | T // unique-string-id /impl-defined
// VENDOR | I // name-list (vendor-id /add-def-doc)
// EXTENSION | I // name-list (ext_name /impl-defined)
-// ATOMIC_DEFAULT_MEM_ORDER I | // value of admo
+// ATOMIC_DEFAULT_MEM_ORDER I | // clause-list (value of admo)
// REQUIRES | I // clause-list (from requires)
// CONDITION U // logical-expr
+// <other name> I // treated as extension
//
// Trait-set-selectors:
// [D]evice, [T]arget_device, [C]onstruct, [I]mplementation, [U]ser.
@@ -3582,7 +3568,7 @@ struct OmpTraitSelectorName {
UNION_CLASS_BOILERPLATE(OmpTraitSelectorName);
ENUM_CLASS(Value, Arch, Atomic_Default_Mem_Order, Condition, Device_Num,
Extension, Isa, Kind, Requires, Simd, Uid, Vendor)
- std::variant<Value, llvm::omp::Directive> u;
+ std::variant<Value, llvm::omp::Directive, std::string> u;
};
// trait-selector ->
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index aa2fec01bc640c..f5cafe71acf4e2 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -158,75 +158,153 @@ static TypeDeclarationStmt makeIterSpecDecl(std::list<ObjectName> &&names) {
static std::string nameToString(Name &&name) { return name.ToString(); }
TYPE_PARSER(sourced(construct<OmpTraitPropertyName>( //
- (space >> charLiteralConstantWithoutKind) ||
- applyFunction(nameToString, Parser<Name>{}))))
+ construct<OmpTraitPropertyName>(space >> charLiteralConstantWithoutKind) ||
+ construct<OmpTraitPropertyName>(
+ applyFunction(nameToString, Parser<Name>{})))))
TYPE_PARSER(sourced(construct<OmpTraitScore>( //
- "SCORE" >> parenthesized(scalarIntExpr))))
+ "SCORE"_id >> parenthesized(scalarIntExpr))))
-TYPE_PARSER(sourced(construct<OmpTraitPropertyExtension::ExtensionValue>(
- // Parse nested extension first.
- construct<OmpTraitPropertyExtension::ExtensionValue>(
- indirect(Parser<OmpTraitPropertyExtension>{})) ||
- construct<OmpTraitPropertyExtension::ExtensionValue>(
- Parser<OmpTraitPropertyName>{}) ||
- construct<OmpTraitPropertyExtension::ExtensionValue>(scalarExpr))))
-
-TYPE_PARSER(sourced(construct<OmpTraitPropertyExtension>( //
+TYPE_PARSER(sourced(construct<OmpTraitPropertyExtension::Complex>(
Parser<OmpTraitPropertyName>{},
parenthesized(nonemptySeparated(
- Parser<OmpTraitPropertyExtension::ExtensionValue>{}, ","_tok)))))
+ indirect(Parser<OmpTraitPropertyExtension>{}), ",")))))
-TYPE_PARSER(sourced(construct<OmpTraitProperty>(
- // Try clause first, then extension before OmpTraitPropertyName.
- construct<OmpTraitProperty>(indirect(Parser<OmpClause>{})) ||
- construct<OmpTraitProperty>(Parser<OmpTraitPropertyExtension>{}) ||
- construct<OmpTraitProperty>(Parser<OmpTraitPropertyName>{}) ||
- construct<OmpTraitProperty>(scalarExpr))))
+TYPE_PARSER(sourced(construct<OmpTraitPropertyExtension>(
+ construct<OmpTraitPropertyExtension>(
+ Parser<OmpTraitPropertyExtension::Complex>{}) ||
+ construct<OmpTraitPropertyExtension>(Parser<OmpTraitPropertyName>{}) ||
+ construct<OmpTraitPropertyExtension>(scalarExpr))))
TYPE_PARSER(construct<OmpTraitSelectorName::Value>(
- "ARCH" >> pure(OmpTraitSelectorName::Value::Arch) ||
- "ATOMIC_DEFAULT_MEM_ORDER" >>
+ "ARCH"_id >> pure(OmpTraitSelectorName::Value::Arch) ||
+ "ATOMIC_DEFAULT_MEM_ORDER"_id >>
pure(OmpTraitSelectorName::Value::Atomic_Default_Mem_Order) ||
- "CONDITION" >> pure(OmpTraitSelectorName::Value::Condition) ||
- "DEVICE_NUM" >> pure(OmpTraitSelectorName::Value::Device_Num) ||
- "EXTENSION" >> pure(OmpTraitSelectorName::Value::Extension) ||
- "ISA" >> pure(OmpTraitSelectorName::Value::Isa) ||
- "KIND" >> pure(OmpTraitSelectorName::Value::Kind) ||
- "REQUIRES" >> pure(OmpTraitSelectorName::Value::Requires) ||
- "SIMD" >> pure(OmpTraitSelectorName::Value::Simd) ||
- "UID" >> pure(OmpTraitSelectorName::Value::Uid) ||
- "VENDOR" >> pure(OmpTraitSelectorName::Value::Vendor)))
+ "CONDITION"_id >> pure(OmpTraitSelectorName::Value::Condition) ||
+ "DEVICE_NUM"_id >> pure(OmpTraitSelectorName::Value::Device_Num) ||
+ "EXTENSION"_id >> pure(OmpTraitSelectorName::Value::Extension) ||
+ "ISA"_id >> pure(OmpTraitSelectorName::Value::Isa) ||
+ "KIND"_id >> pure(OmpTraitSelectorName::Value::Kind) ||
+ "REQUIRES"_id >> pure(OmpTraitSelectorName::Value::Requires) ||
+ "SIMD"_id >> pure(OmpTraitSelectorName::Value::Simd) ||
+ "UID"_id >> pure(OmpTraitSelectorName::Value::Uid) ||
+ "VENDOR"_id >> pure(OmpTraitSelectorName::Value::Vendor)))
TYPE_PARSER(sourced(construct<OmpTraitSelectorName>(
// Parse predefined names first (because of SIMD).
construct<OmpTraitSelectorName>(Parser<OmpTraitSelectorName::Value>{}) ||
- construct<OmpTraitSelectorName>(OmpDirectiveNameParser{}))))
+ construct<OmpTraitSelectorName>(OmpDirectiveNameParser{}) ||
+ // identifier-or-string for extensions
+ construct<OmpTraitSelectorName>(
+ applyFunction(nameToString, Parser<Name>{})) ||
+ construct<OmpTraitSelectorName>(space >> charLiteralConstantWithoutKind))))
+
+// Parser for OmpTraitSelector::Properties
+template <typename... PropParser>
+static constexpr auto propertyListParser(PropParser... pp) {
+ // Parse the property list "(score(expr): item1...)" in three steps:
+ // 1. Parse the "("
+ // 2. Parse the optional "score(expr):"
+ // 3. Parse the "item1, ...)", together with the ")".
+ // The reason for including the ")" in the 3rd step is to force parsing
+ // the entire list in each of the alternative property parsers. Otherwise,
+ // the name parser could stop after "foo" in "(foo, bar(1))", without
+ // allowing the next parser to give the list a try.
+ auto listOf{[](auto parser) { //
+ return nonemptySeparated(parser, ",");
+ }};
+
+ using P = OmpTraitProperty;
+ return maybe("(" >> //
+ construct<OmpTraitSelector::Properties>(
+ maybe(Parser<OmpTraitScore>{} / ":"),
+ (attempt(listOf(sourced(construct<P>(pp))) / ")") || ...)));
+}
+
+// Parser for OmpTraitSelector
+struct TraitSelectorParser {
+ using resultType = OmpTraitSelector;
+
+ constexpr TraitSelectorParser(Parser<OmpTraitSelectorName> p) : np(p) {}
-TYPE_PARSER(construct<OmpTraitSelector::Properties>(
- maybe(Parser<OmpTraitScore>{} / ":"_tok),
- nonemptySeparated(Parser<OmpTraitProperty>{}, ","_tok)))
+ std::optional<resultType> Parse(ParseState &state) const {
+ auto name{attempt(np).Parse(state)};
+ if (!name.has_value()) {
+ return std::nullopt;
+ }
+
+ // Default fallback parser for lists that cannot be parser using the
+ // primary property parser.
+ auto extParser{Parser<OmpTraitPropertyExtension>{}};
+
+ if (auto *v{std::get_if<OmpTraitSelectorName::Value>(&name->u)}) {
+ // (*) The comments below show the sections of the OpenMP spec that
+ // describe given trait. The cases marked with a (*) are those where
+ // the spec doesn't assign any list-type to these traits, but for
+ // convenience they can be treated as if they were.
+ switch (*v) {
+ // name-list properties
+ case OmpTraitSelectorName::Value::Arch: // [6.0:319:18]
+ case OmpTraitSelectorName::Value::Extension: // [6.0:319:30]
+ case OmpTraitSelectorName::Value::Isa: // [6.0:319:15]
+ case OmpTraitSelectorName::Value::Kind: // [6.0:319:10]
+ case OmpTraitSelectorName::Value::Uid: // [6.0:319:23](*)
+ case OmpTraitSelectorName::Value::Vendor: { // [6.0:319:27]
+ auto pp{propertyListParser(Parser<OmpTraitPropertyName>{}, extParser)};
+ return OmpTraitSelector(std::move(*name), std::move(*pp.Parse(state)));
+ }
+ // clause-list
+ case OmpTraitSelectorName::Value::Atomic_Default_Mem_Order:
+ // [6.0:321:26-29](*)
+ case OmpTraitSelectorName::Value::Requires: // [6.0:319:33]
+ case OmpTraitSelectorName::Value::Simd: { // [6.0:318:31]
+ auto pp{propertyListParser(indirect(Parser<OmpClause>{}), extParser)};
+ return OmpTraitSelector(std::move(*name), std::move(*pp.Parse(state)));
+ }
+ // expr-list
+ case OmpTraitSelectorName::Value::Condition: // [6.0:321:33](*)
+ case OmpTraitSelectorName::Value::Device_Num: { // [6.0:321:23-24](*)
+ auto pp{propertyListParser(scalarExpr, extParser)};
+ return OmpTraitSelector(std::move(*name), std::move(*pp.Parse(state)));
+ }
+ } // switch
+ } else {
+ // The other alternatives are `llvm::omp::Directive`, and `std::string`.
+ // The former doesn't take any properties[1], the latter is a name of an
+ // extension[2].
+ // [1] [6.0:319:1-2]
+ // [2] [6.0:319:36-37]
+ auto pp{propertyListParser(extParser)};
+ return OmpTraitSelector(std::move(*name), std::move(*pp.Parse(state)));
+ }
+
+ llvm_unreachable("Unhandled trait name?");
+ }
+
+private:
+ const Parser<OmpTraitSelectorName> np;
+};
-TYPE_PARSER(sourced(construct<OmpTraitSelector>( //
- Parser<OmpTraitSelectorName>{}, //
- maybe(parenthesized(Parser<OmpTraitSelector::Properties>{})))))
+TYPE_PARSER(sourced(construct<OmpTraitSelector>(
+ sourced(TraitSelectorParser(Parser<OmpTraitSelectorName>{})))))
TYPE_PARSER(construct<OmpTraitSetSelectorName::Value>(
- "CONSTRUCT" >> pure(OmpTraitSetSelectorName::Value::Construct) ||
- "DEVICE" >> pure(OmpTraitSetSelectorName::Value::Device) ||
- "IMPLEMENTATION" >> pure(OmpTraitSetSelectorName::Value::Implementation) ||
- "TARGET_DEVICE" >> pure(OmpTraitSetSelectorName::Value::Target_Device) ||
- "USER" >> pure(OmpTraitSetSelectorName::Value::User)))
+ "CONSTRUCT"_id >> pure(OmpTraitSetSelectorName::Value::Construct) ||
+ "DEVICE"_id >> pure(OmpTraitSetSelectorName::Value::Device) ||
+ "IMPLEMENTATION"_id >>
+ pure(OmpTraitSetSelectorName::Value::Implementation) ||
+ "TARGET_DEVICE"_id >> pure(OmpTraitSetSelectorName::Value::Target_Device) ||
+ "USER"_id >> pure(OmpTraitSetSelectorName::Value::User)))
TYPE_PARSER(sourced(construct<OmpTraitSetSelectorName>(
Parser<OmpTraitSetSelectorName::Value>{})))
TYPE_PARSER(sourced(construct<OmpTraitSetSelector>( //
Parser<OmpTraitSetSelectorName>{},
- "=" >> braced(nonemptySeparated(Parser<OmpTraitSelector>{}, ","_tok)))))
+ "=" >> braced(nonemptySeparated(Parser<OmpTraitSelector>{}, ",")))))
TYPE_PARSER(sourced(construct<OmpContextSelectorSpecification>(
- nonemptySeparated(Parser<OmpTraitSetSelector>{}, ","_tok))))
+ nonemptySeparated(Parser<OmpTraitSetSelector>{}, ","))))
// Parser<OmpContextSelector> == Parser<traits::OmpContextSelectorSpecification>
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index e68f098670b1f6..5946aca5718f2d 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2079,10 +2079,11 @@ class UnparseVisitor {
Walk(x.v);
Put(")");
}
- void Unparse(const OmpTraitPropertyExtension &x) {
+ void Unparse(const OmpTraitPropertyExtension::Complex &x) {
+ using PropList = std::list<common::Indirection<OmpTraitPropertyExtension>>;
Walk(std::get<OmpTraitPropertyName>(x.t));
Put("(");
- Walk(std::get<OmpTraitPropertyExtension::ExtensionList>(x.t), ",");
+ Walk(std::get<PropList>(x.t), ",");
Put(")");
}
void Unparse(const OmpTraitSelector &x) {
More information about the flang-commits
mailing list