[clang] 5c63ae1 - [OpenMP] Support nested OpenMP context selectors (declare variant)
Johannes Doerfert via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 16 11:39:58 PDT 2020
Author: Johannes Doerfert
Date: 2020-09-16T13:37:09-05:00
New Revision: 5c63ae156e96a20ce96570d4bd2c48a9c8170a9d
URL: https://github.com/llvm/llvm-project/commit/5c63ae156e96a20ce96570d4bd2c48a9c8170a9d
DIFF: https://github.com/llvm/llvm-project/commit/5c63ae156e96a20ce96570d4bd2c48a9c8170a9d.diff
LOG: [OpenMP] Support nested OpenMP context selectors (declare variant)
Due to `omp begin/end declare variant`, OpenMP context selectors can be
nested. This patch adds initial support for this so we can use it for
target math variants. We should improve the detection of "equivalent"
scores and user conditions, we should also revisit the data structures
of the OMPTraitInfo object, however, both are not pressing issues right
now.
Reviewed By: JonChesterfield
Differential Revision: https://reviews.llvm.org/D85877
Added:
clang/test/AST/ast-dump-openmp-begin-declare-variant_nested.c
Modified:
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Parse/Parser.h
clang/include/clang/Sema/Sema.h
clang/lib/Parse/ParseOpenMP.cpp
clang/lib/Sema/SemaOpenMP.cpp
clang/test/OpenMP/declare_variant_messages.c
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 1c8d741ab54f..1ac1e9d10a7a 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1293,6 +1293,11 @@ def err_omp_mapper_expected_declarator : Error<
"expected declarator on 'omp declare mapper' directive">;
def err_omp_declare_variant_wrong_clause : Error<
"expected '%0' clause on 'omp declare variant' directive">;
+def err_omp_declare_variant_duplicate_nested_trait : Error<
+ "nested OpenMP context selector contains duplicated trait '%0'"
+ " in selector '%1' and set '%2' with
diff erent score">;
+def err_omp_declare_variant_nested_user_condition : Error<
+ "nested user conditions in OpenMP context selector not supported (yet)">;
def warn_omp_declare_variant_string_literal_or_identifier
: Warning<"expected identifier or string literal describing a context "
"%select{set|selector|property}0; "
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index f6ded1b4ee26..a9bd448ba026 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10367,10 +10367,6 @@ def err_omp_non_lvalue_in_map_or_motion_clauses: Error<
"expected addressable lvalue in '%0' clause">;
def err_omp_var_expected : Error<
"expected variable of the '%0' type%select{|, not %2}1">;
-def warn_nested_declare_variant
- : Warning<"nesting `omp begin/end declare variant` is not supported yet; "
- "nested context ignored">,
- InGroup<SourceUsesOpenMP>;
def warn_unknown_declare_variant_isa_trait
: Warning<"isa trait '%0' is not known to the current target; verify the "
"spelling or consider restricting the context selector with the "
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index af8cf47e5667..211827e99de8 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3098,7 +3098,8 @@ class Parser : public CodeCompletionHandler {
/// Parse a `match` clause for an '#pragma omp declare variant'. Return true
/// if there was an error.
- bool parseOMPDeclareVariantMatchClause(SourceLocation Loc, OMPTraitInfo &TI);
+ bool parseOMPDeclareVariantMatchClause(SourceLocation Loc, OMPTraitInfo &TI,
+ OMPTraitInfo *ParentTI);
/// Parse clauses for '#pragma omp declare variant'.
void ParseOMPDeclareVariantClauses(DeclGroupPtrTy Ptr, CachedTokens &Toks,
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index 129ac0355c87..9502c104be68 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10019,6 +10019,12 @@ class Sema final {
OMPDeclareVariantScope(OMPTraitInfo &TI);
};
+ /// Return the OMPTraitInfo for the surrounding scope, if any.
+ OMPTraitInfo *getOMPTraitInfoForSurroundingScope() {
+ return OMPDeclareVariantScopes.empty() ? nullptr
+ : OMPDeclareVariantScopes.back().TI;
+ }
+
/// The current `omp begin/end declare variant` scopes.
SmallVector<OMPDeclareVariantScope, 4> OMPDeclareVariantScopes;
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index ceb91dce186c..40124264fdb9 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -1385,8 +1385,10 @@ void Parser::ParseOMPDeclareVariantClauses(Parser::DeclGroupPtrTy Ptr,
return;
}
- OMPTraitInfo &TI = Actions.getASTContext().getNewOMPTraitInfo();
- if (parseOMPDeclareVariantMatchClause(Loc, TI))
+ OMPTraitInfo *ParentTI = Actions.getOMPTraitInfoForSurroundingScope();
+ ASTContext &ASTCtx = Actions.getASTContext();
+ OMPTraitInfo &TI = ASTCtx.getNewOMPTraitInfo();
+ if (parseOMPDeclareVariantMatchClause(Loc, TI, ParentTI))
return;
Optional<std::pair<FunctionDecl *, Expr *>> DeclVarData =
@@ -1407,7 +1409,8 @@ void Parser::ParseOMPDeclareVariantClauses(Parser::DeclGroupPtrTy Ptr,
}
bool Parser::parseOMPDeclareVariantMatchClause(SourceLocation Loc,
- OMPTraitInfo &TI) {
+ OMPTraitInfo &TI,
+ OMPTraitInfo *ParentTI) {
// Parse 'match'.
OpenMPClauseKind CKind = Tok.isAnnotation()
? OMPC_unknown
@@ -1438,6 +1441,66 @@ bool Parser::parseOMPDeclareVariantMatchClause(SourceLocation Loc,
// Parse ')'
(void)T.consumeClose();
+
+ if (!ParentTI)
+ return false;
+
+ // Merge the parent/outer trait info into the one we just parsed and diagnose
+ // problems.
+ // TODO: Keep some source location in the TI to provide better diagnostics.
+ // TODO: Perform some kind of equivalence check on the condition and score
+ // expressions.
+ for (const OMPTraitSet &ParentSet : ParentTI->Sets) {
+ bool MergedSet = false;
+ for (OMPTraitSet &Set : TI.Sets) {
+ if (Set.Kind != ParentSet.Kind)
+ continue;
+ MergedSet = true;
+ for (const OMPTraitSelector &ParentSelector : ParentSet.Selectors) {
+ bool MergedSelector = false;
+ for (OMPTraitSelector &Selector : Set.Selectors) {
+ if (Selector.Kind != ParentSelector.Kind)
+ continue;
+ MergedSelector = true;
+ for (const OMPTraitProperty &ParentProperty :
+ ParentSelector.Properties) {
+ bool MergedProperty = false;
+ for (OMPTraitProperty &Property : Selector.Properties) {
+ // Ignore "equivalent" properties.
+ if (Property.Kind != ParentProperty.Kind)
+ continue;
+
+ // If the kind is the same but the raw string not, we don't want
+ // to skip out on the property.
+ MergedProperty |= Property.RawString == ParentProperty.RawString;
+
+ if (Property.RawString == ParentProperty.RawString &&
+ Selector.ScoreOrCondition == ParentSelector.ScoreOrCondition)
+ continue;
+
+ if (Selector.Kind == llvm::omp::TraitSelector::user_condition) {
+ Diag(Loc, diag::err_omp_declare_variant_nested_user_condition);
+ } else if (Selector.ScoreOrCondition !=
+ ParentSelector.ScoreOrCondition) {
+ Diag(Loc, diag::err_omp_declare_variant_duplicate_nested_trait)
+ << getOpenMPContextTraitPropertyName(
+ ParentProperty.Kind, ParentProperty.RawString)
+ << getOpenMPContextTraitSelectorName(ParentSelector.Kind)
+ << getOpenMPContextTraitSetName(ParentSet.Kind);
+ }
+ }
+ if (!MergedProperty)
+ Selector.Properties.push_back(ParentProperty);
+ }
+ }
+ if (!MergedSelector)
+ Set.Selectors.push_back(ParentSelector);
+ }
+ }
+ if (!MergedSet)
+ TI.Sets.push_back(ParentSet);
+ }
+
return false;
}
@@ -1811,8 +1874,10 @@ Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
// { #pragma omp end declare variant }
//
ConsumeToken();
- OMPTraitInfo &TI = Actions.getASTContext().getNewOMPTraitInfo();
- if (parseOMPDeclareVariantMatchClause(Loc, TI))
+ OMPTraitInfo *ParentTI = Actions.getOMPTraitInfoForSurroundingScope();
+ ASTContext &ASTCtx = Actions.getASTContext();
+ OMPTraitInfo &TI = ASTCtx.getNewOMPTraitInfo();
+ if (parseOMPDeclareVariantMatchClause(Loc, TI, ParentTI))
break;
// Skip last tokens.
@@ -1821,7 +1886,6 @@ Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
ParsingOpenMPDirectiveRAII NormalScope(*this, /*Value=*/false);
VariantMatchInfo VMI;
- ASTContext &ASTCtx = Actions.getASTContext();
TI.getAsVariantMatchInfo(ASTCtx, VMI);
std::function<void(StringRef)> DiagUnknownTrait = [this, Loc](
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 1a0470a9606d..aef043b06299 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -2441,10 +2441,6 @@ void Sema::DestroyDataSharingAttributesStack() { delete DSAStack; }
void Sema::ActOnOpenMPBeginDeclareVariant(SourceLocation Loc,
OMPTraitInfo &TI) {
- if (!OMPDeclareVariantScopes.empty()) {
- Diag(Loc, diag::warn_nested_declare_variant);
- return;
- }
OMPDeclareVariantScopes.push_back(OMPDeclareVariantScope(TI));
}
diff --git a/clang/test/AST/ast-dump-openmp-begin-declare-variant_nested.c b/clang/test/AST/ast-dump-openmp-begin-declare-variant_nested.c
new file mode 100644
index 000000000000..e4b5b39ae87a
--- /dev/null
+++ b/clang/test/AST/ast-dump-openmp-begin-declare-variant_nested.c
@@ -0,0 +1,87 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify -ast-dump %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify -ast-dump %s -x c++| FileCheck %s
+// expected-no-diagnostics
+
+int also_before(void) {
+ return 1;
+}
+
+#pragma omp begin declare variant match(user = {condition(1)}, device = {kind(cpu)}, implementation = {vendor(llvm)})
+#pragma omp begin declare variant match(device = {kind(cpu)}, implementation = {vendor(llvm, pgi), extension(match_any)})
+#pragma omp begin declare variant match(device = {kind(any)}, implementation = {dynamic_allocators})
+int also_after(void) {
+ return 0;
+}
+int also_before(void) {
+ return 0;
+}
+#pragma omp end declare variant
+#pragma omp end declare variant
+#pragma omp end declare variant
+
+int also_after(void) {
+ return 2;
+}
+
+int test() {
+ // Should return 0.
+ return also_after() + also_before();
+}
+
+#pragma omp begin declare variant match(device = {isa("sse")})
+#pragma omp declare variant(test) match(device = {isa(sse)})
+int equivalent_isa_trait(void);
+#pragma omp end declare variant
+
+#pragma omp begin declare variant match(device = {isa("sse")})
+#pragma omp declare variant(test) match(device = {isa("sse2")})
+int non_equivalent_isa_trait(void);
+#pragma omp end declare variant
+
+// CHECK: |-FunctionDecl [[ADDR_0:0x[a-z0-9]*]] <{{.*}}, line:7:1> line:5:5 used also_before 'int ({{.*}})'
+// CHECK-NEXT: | |-CompoundStmt [[ADDR_1:0x[a-z0-9]*]] <col:23, line:7:1>
+// CHECK-NEXT: | | `-ReturnStmt [[ADDR_2:0x[a-z0-9]*]] <line:6:3, col:10>
+// CHECK-NEXT: | | `-IntegerLiteral [[ADDR_3:0x[a-z0-9]*]] <col:10> 'int' 1
+// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_4:0x[a-z0-9]*]] <<invalid sloc>> Implicit device={kind(any, cpu)}, implementation={dynamic_allocators, vendor(llvm, pgi), extension(match_any)}, user={condition(1)}
+// CHECK-NEXT: | `-DeclRefExpr [[ADDR_5:0x[a-z0-9]*]] <line:15:1> 'int ({{.*}})' {{.*}}Function [[ADDR_6:0x[a-z0-9]*]] 'also_before[device={kind(any, cpu)}, implementation={dynamic_allocators, vendor(llvm, pgi), extension(match_any)}, user={condition(...)}]' 'int ({{.*}})'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_7:0x[a-z0-9]*]] <line:12:1, col:20> col:5 implicit used also_after 'int ({{.*}})'
+// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_8:0x[a-z0-9]*]] <<invalid sloc>> Implicit device={kind(any, cpu)}, implementation={dynamic_allocators, vendor(llvm, pgi), extension(match_any)}, user={condition(1)}
+// CHECK-NEXT: | `-DeclRefExpr [[ADDR_9:0x[a-z0-9]*]] <col:1> 'int ({{.*}})' {{.*}}Function [[ADDR_10:0x[a-z0-9]*]] 'also_after[device={kind(any, cpu)}, implementation={dynamic_allocators, vendor(llvm, pgi), extension(match_any)}, user={condition(...)}]' 'int ({{.*}})'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_10]] <col:1, line:14:1> line:12:1 also_after[device={kind(any, cpu)}, implementation={dynamic_allocators, vendor(llvm, pgi), extension(match_any)}, user={condition(...)}] 'int ({{.*}})'
+// CHECK-NEXT: | `-CompoundStmt [[ADDR_11:0x[a-z0-9]*]] <col:22, line:14:1>
+// CHECK-NEXT: | `-ReturnStmt [[ADDR_12:0x[a-z0-9]*]] <line:13:3, col:10>
+// CHECK-NEXT: | `-IntegerLiteral [[ADDR_13:0x[a-z0-9]*]] <col:10> 'int' 0
+// CHECK-NEXT: |-FunctionDecl [[ADDR_6]] <line:15:1, line:17:1> line:15:1 also_before[device={kind(any, cpu)}, implementation={dynamic_allocators, vendor(llvm, pgi), extension(match_any)}, user={condition(...)}] 'int ({{.*}})'
+// CHECK-NEXT: | `-CompoundStmt [[ADDR_14:0x[a-z0-9]*]] <col:23, line:17:1>
+// CHECK-NEXT: | `-ReturnStmt [[ADDR_15:0x[a-z0-9]*]] <line:16:3, col:10>
+// CHECK-NEXT: | `-IntegerLiteral [[ADDR_16:0x[a-z0-9]*]] <col:10> 'int' 0
+// CHECK-NEXT: |-FunctionDecl [[ADDR_17:0x[a-z0-9]*]] prev [[ADDR_7]] <line:22:1, line:24:1> line:22:5 used also_after 'int ({{.*}})'
+// CHECK-NEXT: | |-CompoundStmt [[ADDR_18:0x[a-z0-9]*]] <col:22, line:24:1>
+// CHECK-NEXT: | | `-ReturnStmt [[ADDR_19:0x[a-z0-9]*]] <line:23:3, col:10>
+// CHECK-NEXT: | | `-IntegerLiteral [[ADDR_20:0x[a-z0-9]*]] <col:10> 'int' 2
+// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_21:0x[a-z0-9]*]] <<invalid sloc>> Inherited Implicit device={kind(any, cpu)}, implementation={dynamic_allocators, vendor(llvm, pgi), extension(match_any)}, user={condition(1)}
+// CHECK-NEXT: | `-DeclRefExpr [[ADDR_9]] <line:12:1> 'int ({{.*}})' {{.*}}Function [[ADDR_10]] 'also_after[device={kind(any, cpu)}, implementation={dynamic_allocators, vendor(llvm, pgi), extension(match_any)}, user={condition(...)}]' 'int ({{.*}})'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_22:0x[a-z0-9]*]] <line:26:1, line:29:1> line:26:5 referenced test 'int ({{.*}})'
+// CHECK-NEXT: | `-CompoundStmt [[ADDR_23:0x[a-z0-9]*]] <col:12, line:29:1>
+// CHECK-NEXT: | `-ReturnStmt [[ADDR_24:0x[a-z0-9]*]] <line:28:3, col:37>
+// CHECK-NEXT: | `-BinaryOperator [[ADDR_25:0x[a-z0-9]*]] <col:10, col:37> 'int' '+'
+// CHECK-NEXT: | |-PseudoObjectExpr [[ADDR_26:0x[a-z0-9]*]] <col:10, col:21> 'int'
+// CHECK-NEXT: | | |-CallExpr [[ADDR_27:0x[a-z0-9]*]] <col:10, col:21> 'int'
+// CHECK-NEXT: | | | `-ImplicitCastExpr [[ADDR_28:0x[a-z0-9]*]] <col:10> 'int (*)({{.*}})' <FunctionToPointerDecay>
+// CHECK-NEXT: | | | `-DeclRefExpr [[ADDR_29:0x[a-z0-9]*]] <col:10> 'int ({{.*}})' {{.*}}Function [[ADDR_17]] 'also_after' 'int ({{.*}})'
+// CHECK-NEXT: | | `-CallExpr [[ADDR_30:0x[a-z0-9]*]] <line:12:1, line:28:21> 'int'
+// CHECK-NEXT: | | `-ImplicitCastExpr [[ADDR_31:0x[a-z0-9]*]] <line:12:1> 'int (*)({{.*}})' <FunctionToPointerDecay>
+// CHECK-NEXT: | | `-DeclRefExpr [[ADDR_9]] <col:1> 'int ({{.*}})' {{.*}}Function [[ADDR_10]] 'also_after[device={kind(any, cpu)}, implementation={dynamic_allocators, vendor(llvm, pgi), extension(match_any)}, user={condition(...)}]' 'int ({{.*}})'
+// CHECK-NEXT: | `-PseudoObjectExpr [[ADDR_32:0x[a-z0-9]*]] <line:28:25, col:37> 'int'
+// CHECK-NEXT: | |-CallExpr [[ADDR_33:0x[a-z0-9]*]] <col:25, col:37> 'int'
+// CHECK-NEXT: | | `-ImplicitCastExpr [[ADDR_34:0x[a-z0-9]*]] <col:25> 'int (*)({{.*}})' <FunctionToPointerDecay>
+// CHECK-NEXT: | | `-DeclRefExpr [[ADDR_35:0x[a-z0-9]*]] <col:25> 'int ({{.*}})' {{.*}}Function [[ADDR_0]] 'also_before' 'int ({{.*}})'
+// CHECK-NEXT: | `-CallExpr [[ADDR_36:0x[a-z0-9]*]] <line:15:1, line:28:37> 'int'
+// CHECK-NEXT: | `-ImplicitCastExpr [[ADDR_37:0x[a-z0-9]*]] <line:15:1> 'int (*)({{.*}})' <FunctionToPointerDecay>
+// CHECK-NEXT: | `-DeclRefExpr [[ADDR_5]] <col:1> 'int ({{.*}})' {{.*}}Function [[ADDR_6]] 'also_before[device={kind(any, cpu)}, implementation={dynamic_allocators, vendor(llvm, pgi), extension(match_any)}, user={condition(...)}]' 'int ({{.*}})'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_38:0x[a-z0-9]*]] <line:33:1, col:30> col:5 equivalent_isa_trait 'int ({{.*}})'
+// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_39:0x[a-z0-9]*]] <line:32:1, col:61> Implicit device={isa(sse)}
+// CHECK-NEXT: | `-DeclRefExpr [[ADDR_40:0x[a-z0-9]*]] <col:29> 'int ({{.*}})' {{.*}}Function [[ADDR_22]] 'test' 'int ({{.*}})' non_odr_use_unevaluated
+// CHECK-NEXT: `-FunctionDecl [[ADDR_41:0x[a-z0-9]*]] <line:38:1, col:34> col:5 non_equivalent_isa_trait 'int ({{.*}})'
+// CHECK-NEXT: `-OMPDeclareVariantAttr [[ADDR_42:0x[a-z0-9]*]] <line:37:1, col:64> Implicit device={isa(sse2, sse)}
+// CHECK-NEXT: `-DeclRefExpr [[ADDR_43:0x[a-z0-9]*]] <col:29> 'int ({{.*}})' {{.*}}Function [[ADDR_22]] 'test' 'int ({{.*}})' non_odr_use_unevaluated
diff --git a/clang/test/OpenMP/declare_variant_messages.c b/clang/test/OpenMP/declare_variant_messages.c
index 84a56c5fd409..2c63ca206fbb 100644
--- a/clang/test/OpenMP/declare_variant_messages.c
+++ b/clang/test/OpenMP/declare_variant_messages.c
@@ -153,3 +153,17 @@ void caller() {
#pragma omp declare variant // expected-error {{function declaration is expected after 'declare variant' directive}}
#pragma omp declare variant // expected-error {{function declaration is expected after 'declare variant' directive}}
+
+// FIXME: If the scores are equivalent we should detect that and allow it.
+#pragma omp begin declare variant match(implementation = {vendor(score(2) \
+ : llvm)})
+#pragma omp declare variant(foo) match(implementation = {vendor(score(2) \
+ : llvm)}) // expected-error at -1 {{nested OpenMP context selector contains duplicated trait 'llvm' in selector 'vendor' and set 'implementation' with
diff erent score}}
+int conflicting_nested_score(void);
+#pragma omp end declare variant
+
+// FIXME: We should build the conjuction of
diff erent conditions, see also the score fixme above.
+#pragma omp begin declare variant match(user = {condition(1)})
+#pragma omp declare variant(foo) match(user = {condition(1)}) // expected-error {{nested user conditions in OpenMP context selector not supported (yet)}}
+int conflicting_nested_condition(void);
+#pragma omp end declare variant
More information about the cfe-commits
mailing list