jdoerfert created this revision.
jdoerfert added reviewers: JonChesterfield, jhuber6, ABataev.
Herald added subscribers: guansong, bollu, yaxunl.
Herald added a project: clang.
jdoerfert requested review of this revision.
Herald added a subscriber: sstefan1.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D85877
Files:
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/AST/ast-dump-openmp-begin-declare-variant_nested.c
clang/test/OpenMP/declare_variant_messages.c
Index: clang/test/OpenMP/declare_variant_messages.c
===================================================================
--- clang/test/OpenMP/declare_variant_messages.c
+++ clang/test/OpenMP/declare_variant_messages.c
@@ -153,3 +153,17 @@
#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 {{nested OpenMP context selector contains duplicated trait 'llvm' in selector 'vendor' and set 'implementation' with different score}}
+int conflicting_nested_score(void);
+#pragma omp end declare variant
+
+// FIXME: We should build the conjuction of different 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
Index: clang/test/AST/ast-dump-openmp-begin-declare-variant_nested.c
===================================================================
--- /dev/null
+++ 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:59> 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:62> 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
Index: clang/lib/Sema/SemaOpenMP.cpp
===================================================================
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -2402,10 +2402,6 @@
void Sema::ActOnOpenMPBeginDeclareVariant(SourceLocation Loc,
OMPTraitInfo &TI) {
- if (!OMPDeclareVariantScopes.empty()) {
- Diag(Loc, diag::warn_nested_declare_variant);
- return;
- }
OMPDeclareVariantScopes.push_back(OMPDeclareVariantScope(TI));
}
Index: clang/lib/Parse/ParseOpenMP.cpp
===================================================================
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -1385,8 +1385,10 @@
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 @@
}
bool Parser::parseOMPDeclareVariantMatchClause(SourceLocation Loc,
- OMPTraitInfo &TI) {
+ OMPTraitInfo &TI,
+ OMPTraitInfo *ParentTI) {
// Parse 'match'.
OpenMPClauseKind CKind = Tok.isAnnotation()
? OMPC_unknown
@@ -1438,6 +1441,66 @@
// 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 @@
// { #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 @@
ParsingOpenMPDirectiveRAII NormalScope(*this, /*Value=*/false);
VariantMatchInfo VMI;
- ASTContext &ASTCtx = Actions.getASTContext();
TI.getAsVariantMatchInfo(ASTCtx, VMI);
std::function<void(StringRef)> DiagUnknownTrait = [this, Loc](
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -10019,6 +10019,12 @@
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;
Index: clang/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -3074,7 +3074,8 @@
/// 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,
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10336,10 +10336,6 @@
"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 "
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -1273,6 +1273,11 @@
"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 different 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; "
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
