jdoerfert updated this revision to Diff 285386.
jdoerfert added a comment.
Minor tweaks
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85877/new/
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@-1 {{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: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
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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
