-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[flang][OpenMP] Parsing context selectors for METADIRECTIVE #121815
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
Changes from 11 commits
fa9b23d
fa0d221
520f43c
790a808
215c7e6
de4be10
aeb5fbb
a9f48dd
5602de1
1cff12b
60100af
0e317db
419bdfa
c4b9007
d6ca30b
e0ca347
ca7584b
c91b506
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3453,6 +3453,17 @@ WRAPPER_CLASS(PauseStmt, std::optional<StopCode>); | |
|
|
||
| // --- Common definitions | ||
|
|
||
| struct OmpClause; | ||
| struct OmpClauseList; | ||
|
|
||
| struct OmpDirectiveSpecification { | ||
| TUPLE_CLASS_BOILERPLATE(OmpDirectiveSpecification); | ||
| std::tuple<llvm::omp::Directive, | ||
| std::optional<common::Indirection<OmpClauseList>>> | ||
| t; | ||
| CharBlock source; | ||
| }; | ||
|
|
||
| // 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. | ||
|
|
@@ -3474,6 +3485,128 @@ WRAPPER_CLASS(OmpObjectList, std::list<OmpObject>); | |
|
|
||
| #define MODIFIERS() std::optional<std::list<Modifier>> | ||
|
|
||
| inline namespace traits { | ||
| // trait-property-name -> | ||
| // identifier | string-literal | ||
| struct OmpTraitPropertyName { | ||
| WRAPPER_CLASS_BOILERPLATE(OmpTraitPropertyName, std::string); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did we miss the identifier part, ie
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a problematic one. The spec says that a word in quotes, and the same word without quotes are equivalent. I decided to parse both as a string, but I'm not insisting that this is the final solution. The problem is that trait-property can be (among other things) a trait-property-name or a trait-property-expression. A simple identifier can be either, there is no reasonably simple way of telling them apart in the parser. There is a similar issue with extensions. I think that we will need to do some of that disambiguation in the "canonicalization" pass and then rewrite some of those AST nodes into different ones. Could we leave this as-is for now until there is more clarity about what to do? Or do you have strong opinions about what to do with this? For the purposes of just parsing the METADIRECTIVE construct in this LLVM version this should be fine.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure. Thanks for the explanation. Can you add this explanation to the representation? |
||
| }; | ||
|
|
||
| // trait-score -> | ||
| // SCORE(non-negative-const-integer-expression) | ||
| struct OmpTraitScore { | ||
| WRAPPER_CLASS_BOILERPLATE(OmpTraitScore, ScalarIntExpr); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to be able to print a meaningful message (in semantic checks) when it's not a constant expression. Otherwise, the user would just get a syntax error. |
||
| }; | ||
|
|
||
| // 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 different | ||
| // version (but equivalent) that doesn't have ambiguities. | ||
|
Comment on lines
+3514
to
+3515
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this reported?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To the OpenMP ARB? No. I think their grammar is there to show the syntax to the user (just like the in-text grammar for the compound constructs). I'll mention it to @mjklemm and let him weigh in on what we should do with this.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The grammar that is printed in the spec is no longer normative. The only thing that matters is how the syntax is is specified in the JSON database. If that syntax is ambiguous, then it might be a good idea to file a ticket for the OpenMP language committee to see if it can be resolved. |
||
| // 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) | ||
| // | ||
| struct OmpTraitPropertyExtension { | ||
| TUPLE_CLASS_BOILERPLATE(OmpTraitPropertyExtension); | ||
| struct ExtensionValue { | ||
| UNION_CLASS_BOILERPLATE(ExtensionValue); | ||
| std::variant<OmpTraitPropertyName, ScalarExpr, | ||
| common::Indirection<OmpTraitPropertyExtension>> | ||
| u; | ||
| }; | ||
| using ExtensionList = std::list<ExtensionValue>; | ||
| std::tuple<OmpTraitPropertyName, ExtensionList> t; | ||
| }; | ||
|
|
||
| // trait-property -> | ||
| // trait-property-name | OmpClause | | ||
| // trait-property-expression | trait-property-extension | ||
| // trait-property-expression -> | ||
| // scalar-logical-expression | scalar-integer-expression | ||
| // | ||
| // The parser for a logical expression will accept an integer expression, | ||
| // and if it's not logical, it will flag an error later. The same thing | ||
| // will happen if the scalar integer expression sees a logical expresion. | ||
| // To avoid this, parse all expressions as scalar expressions. | ||
| struct OmpTraitProperty { | ||
| UNION_CLASS_BOILERPLATE(OmpTraitProperty); | ||
| std::variant<OmpTraitPropertyName, common::Indirection<OmpClause>, | ||
| ScalarExpr, // trait-property-expresion | ||
| OmpTraitPropertyExtension> | ||
| u; | ||
| }; | ||
|
|
||
| // trait-selector-name -> | ||
| // KIND | DT // name-list (host, nohost, +/add-def-doc) | ||
| // ISA | DT // name-list (isa_name, ... /impl-defined) | ||
| // ARCH | DT // name-list (arch_name, ... /impl-defined) | ||
| // directive-name | C // no properties | ||
| // SIMD | C // clause-list (from declare_simd) | ||
| // // (at least simdlen, inbranch/notinbranch) | ||
| // DEVICE_NUM | T // device-number | ||
| // 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 | ||
| // REQUIRES | I // clause-list (from requires) | ||
| // CONDITION U // logical-expr | ||
| // | ||
| // Trait-set-selectors: | ||
| // [D]evice, [T]arget_device, [C]onstruct, [I]mplementation, [U]ser. | ||
| struct OmpTraitSelectorName { | ||
| UNION_CLASS_BOILERPLATE(OmpTraitSelectorName); | ||
| ENUM_CLASS(Value, Arch, Atomic_Default_Mem_Order, Condition, Device_Num, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are underscores generally used in this file?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, see BindAttr at lines 1130-1131, or ImageSelectorSpec at lines 1687 and 1689. Additionally, this spelling (ignoring upper/lower case) follows the spelling to be used in source code. |
||
| Extension, Isa, Kind, Requires, Simd, Uid, Vendor) | ||
| std::variant<Value, llvm::omp::Directive> u; | ||
| }; | ||
|
|
||
| // trait-selector -> | ||
| // trait-selector-name | | ||
| // trait-selector-name ([trait-score:] trait-property, ...) | ||
| struct OmpTraitSelector { | ||
| TUPLE_CLASS_BOILERPLATE(OmpTraitSelector); | ||
| struct Properties { | ||
| TUPLE_CLASS_BOILERPLATE(Properties); | ||
| std::tuple<std::optional<OmpTraitScore>, std::list<OmpTraitProperty>> t; | ||
| }; | ||
| std::tuple<OmpTraitSelectorName, std::optional<Properties>> t; | ||
| }; | ||
|
|
||
| // trait-set-selector-name -> | ||
| // CONSTRUCT | DEVICE | IMPLEMENTATION | USER | // since 5.0 | ||
| // TARGET_DEVICE // since 5.1 | ||
| struct OmpTraitSetSelectorName { | ||
| ENUM_CLASS(Value, Construct, Device, Implementation, Target_Device, User) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question as above. |
||
| WRAPPER_CLASS_BOILERPLATE(OmpTraitSetSelectorName, Value); | ||
| }; | ||
|
|
||
| // trait-set-selector -> | ||
| // trait-set-selector-name = {trait-selector, ...} | ||
| struct OmpTraitSetSelector { | ||
| TUPLE_CLASS_BOILERPLATE(OmpTraitSetSelector); | ||
| std::tuple<OmpTraitSetSelectorName, std::list<OmpTraitSelector>> t; | ||
| }; | ||
|
|
||
| // context-selector-specification -> | ||
| // trait-set-selector, ... | ||
| struct OmpContextSelectorSpecification { // Modifier | ||
| WRAPPER_CLASS_BOILERPLATE( | ||
| OmpContextSelectorSpecification, std::list<OmpTraitSetSelector>); | ||
| }; | ||
| } // namespace traits | ||
|
|
||
| inline namespace modifier { | ||
| // For uniformity, in all keyword modifiers the name of the type defined | ||
| // by ENUM_CLASS is "Value", e.g. | ||
|
|
@@ -3744,6 +3877,9 @@ struct OmpVariableCategory { | |
| ENUM_CLASS(Value, Aggregate, All, Allocatable, Pointer, Scalar) | ||
| WRAPPER_CLASS_BOILERPLATE(OmpVariableCategory, Value); | ||
| }; | ||
|
|
||
| // context-selector | ||
| using OmpContextSelector = traits::OmpContextSelectorSpecification; | ||
| } // namespace modifier | ||
|
|
||
| // --- Clauses | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -153,6 +153,84 @@ static TypeDeclarationStmt makeIterSpecDecl(std::list<ObjectName> &&names) { | |
| makeEntityList(std::move(names))); | ||
| } | ||
|
|
||
| TYPE_PARSER(sourced(construct<OmpDirectiveSpecification>( | ||
| OmpDirectiveNameParser{}, maybe(indirect(Parser<OmpClauseList>{}))))) | ||
|
|
||
| // --- Parsers for context traits ------------------------------------- | ||
|
|
||
| TYPE_PARSER(construct<OmpTraitPropertyName>( // | ||
| (space >> charLiteralConstantWithoutKind) || | ||
| applyMem(&Name::ToString, Parser<Name>{}))) | ||
|
|
||
| TYPE_PARSER(construct<OmpTraitScore>( // | ||
| "SCORE" >> parenthesized(scalarIntExpr))) | ||
|
|
||
| TYPE_PARSER(construct<OmpTraitPropertyExtension::ExtensionValue>( | ||
| // Parse nested extension first. | ||
| construct<OmpTraitPropertyExtension::ExtensionValue>( | ||
| indirect(Parser<OmpTraitPropertyExtension>{})) || | ||
| construct<OmpTraitPropertyExtension::ExtensionValue>( | ||
| Parser<OmpTraitPropertyName>{}) || | ||
| construct<OmpTraitPropertyExtension::ExtensionValue>(scalarExpr))) | ||
|
|
||
| TYPE_PARSER(construct<OmpTraitPropertyExtension>( // | ||
| Parser<OmpTraitPropertyName>{}, | ||
| parenthesized(nonemptySeparated( | ||
| Parser<OmpTraitPropertyExtension::ExtensionValue>{}, ","_tok)))) | ||
|
|
||
| TYPE_PARSER(construct<OmpTraitProperty>( | ||
| // Try extension first, before OmpTraitPropertyName. | ||
| construct<OmpTraitProperty>(Parser<OmpTraitPropertyExtension>{}) || | ||
| construct<OmpTraitProperty>(Parser<OmpTraitPropertyName>{}) || | ||
| construct<OmpTraitProperty>(indirect(Parser<OmpClause>{})) || | ||
| construct<OmpTraitProperty>(scalarExpr))) | ||
|
|
||
| TYPE_PARSER(construct<OmpTraitSelectorName::Value>( | ||
| "ARCH" >> pure(OmpTraitSelectorName::Value::Arch) || | ||
| "ATOMIC_DEFAULT_MEM_ORDER" >> | ||
| 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))) | ||
|
|
||
| TYPE_PARSER(construct<OmpTraitSelectorName>( | ||
| // Parse predefined names first (because of SIMD). | ||
| construct<OmpTraitSelectorName>(Parser<OmpTraitSelectorName::Value>{}) || | ||
| construct<OmpTraitSelectorName>(OmpDirectiveNameParser{}))) | ||
|
|
||
| TYPE_PARSER(construct<OmpTraitSelector::Properties>( | ||
| maybe(Parser<OmpTraitScore>{} / ":"_tok), | ||
| nonemptySeparated(Parser<OmpTraitProperty>{}, ","_tok))) | ||
|
|
||
| TYPE_PARSER(construct<OmpTraitSelector>( // | ||
| Parser<OmpTraitSelectorName>{}, // | ||
| maybe(parenthesized(Parser<OmpTraitSelector::Properties>{})))) | ||
|
|
||
| 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))) | ||
|
|
||
| TYPE_PARSER(construct<OmpTraitSetSelectorName>( | ||
| Parser<OmpTraitSetSelectorName::Value>{})) | ||
|
|
||
| TYPE_PARSER(construct<OmpTraitSetSelector>( // | ||
| Parser<OmpTraitSetSelectorName>{}, | ||
| "=" >> braced(nonemptySeparated(Parser<OmpTraitSelector>{}, ","_tok)))) | ||
|
|
||
| TYPE_PARSER(construct<OmpContextSelectorSpecification>( | ||
| nonemptySeparated(Parser<OmpTraitSetSelector>{}, ","_tok))) | ||
|
|
||
| // Parser<OmpContextSelector> == Parser<traits::OmpContextSelectorSpecification> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Commented code?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a section with parsers for all modifiers. You'd expect to find the parser for OmpContextSelector here as well, but it's the same as the parser for the OmpContextSelectorSpecification. To avoid confusion why it's not here, I added a comment that's intended to explain it. |
||
|
|
||
| // --- Parsers for clause modifiers ----------------------------------- | ||
|
|
||
| TYPE_PARSER(construct<OmpAlignment>(scalarIntExpr)) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs a comment. Is this strictly part of context-selectors?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is the type of the argument to
whenandotherwisedirectivesclauses I can move this to PR121817 (the PR with the actual clauses).