diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td --- a/clang/include/clang/Basic/DiagnosticParseKinds.td +++ b/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; " diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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; 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 --- a/clang/include/clang/Parse/Parser.h +++ b/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, diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/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 OMPDeclareVariantScopes; diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp --- a/clang/lib/Parse/ParseOpenMP.cpp +++ b/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> 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 DiagUnknownTrait = [this, Loc]( diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/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)); } 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 --- /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]*]] +// CHECK-NEXT: | | `-ReturnStmt [[ADDR_2:0x[a-z0-9]*]] +// CHECK-NEXT: | | `-IntegerLiteral [[ADDR_3:0x[a-z0-9]*]] 'int' 1 +// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_4:0x[a-z0-9]*]] <> 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]*]] '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]*]] col:5 implicit used also_after 'int ({{.*}})' +// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_8:0x[a-z0-9]*]] <> 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]*]] '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]] 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]*]] +// CHECK-NEXT: | `-ReturnStmt [[ADDR_12:0x[a-z0-9]*]] +// CHECK-NEXT: | `-IntegerLiteral [[ADDR_13:0x[a-z0-9]*]] 'int' 0 +// CHECK-NEXT: |-FunctionDecl [[ADDR_6]] 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]*]] +// CHECK-NEXT: | `-ReturnStmt [[ADDR_15:0x[a-z0-9]*]] +// CHECK-NEXT: | `-IntegerLiteral [[ADDR_16:0x[a-z0-9]*]] 'int' 0 +// CHECK-NEXT: |-FunctionDecl [[ADDR_17:0x[a-z0-9]*]] prev [[ADDR_7]] line:22:5 used also_after 'int ({{.*}})' +// CHECK-NEXT: | |-CompoundStmt [[ADDR_18:0x[a-z0-9]*]] +// CHECK-NEXT: | | `-ReturnStmt [[ADDR_19:0x[a-z0-9]*]] +// CHECK-NEXT: | | `-IntegerLiteral [[ADDR_20:0x[a-z0-9]*]] 'int' 2 +// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_21:0x[a-z0-9]*]] <> Inherited Implicit device={kind(any, cpu)}, implementation={dynamic_allocators, vendor(llvm, pgi), extension(match_any)}, user={condition(1)} +// CHECK-NEXT: | `-DeclRefExpr [[ADDR_9]] '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:5 referenced test 'int ({{.*}})' +// CHECK-NEXT: | `-CompoundStmt [[ADDR_23:0x[a-z0-9]*]] +// CHECK-NEXT: | `-ReturnStmt [[ADDR_24:0x[a-z0-9]*]] +// CHECK-NEXT: | `-BinaryOperator [[ADDR_25:0x[a-z0-9]*]] 'int' '+' +// CHECK-NEXT: | |-PseudoObjectExpr [[ADDR_26:0x[a-z0-9]*]] 'int' +// CHECK-NEXT: | | |-CallExpr [[ADDR_27:0x[a-z0-9]*]] 'int' +// CHECK-NEXT: | | | `-ImplicitCastExpr [[ADDR_28:0x[a-z0-9]*]] 'int (*)({{.*}})' +// CHECK-NEXT: | | | `-DeclRefExpr [[ADDR_29:0x[a-z0-9]*]] 'int ({{.*}})' {{.*}}Function [[ADDR_17]] 'also_after' 'int ({{.*}})' +// CHECK-NEXT: | | `-CallExpr [[ADDR_30:0x[a-z0-9]*]] 'int' +// CHECK-NEXT: | | `-ImplicitCastExpr [[ADDR_31:0x[a-z0-9]*]] 'int (*)({{.*}})' +// CHECK-NEXT: | | `-DeclRefExpr [[ADDR_9]] '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]*]] 'int' +// CHECK-NEXT: | |-CallExpr [[ADDR_33:0x[a-z0-9]*]] 'int' +// CHECK-NEXT: | | `-ImplicitCastExpr [[ADDR_34:0x[a-z0-9]*]] 'int (*)({{.*}})' +// CHECK-NEXT: | | `-DeclRefExpr [[ADDR_35:0x[a-z0-9]*]] 'int ({{.*}})' {{.*}}Function [[ADDR_0]] 'also_before' 'int ({{.*}})' +// CHECK-NEXT: | `-CallExpr [[ADDR_36:0x[a-z0-9]*]] 'int' +// CHECK-NEXT: | `-ImplicitCastExpr [[ADDR_37:0x[a-z0-9]*]] 'int (*)({{.*}})' +// CHECK-NEXT: | `-DeclRefExpr [[ADDR_5]] '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]*]] col:5 equivalent_isa_trait 'int ({{.*}})' +// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_39:0x[a-z0-9]*]] Implicit device={isa(sse)} +// CHECK-NEXT: | `-DeclRefExpr [[ADDR_40:0x[a-z0-9]*]] 'int ({{.*}})' {{.*}}Function [[ADDR_22]] 'test' 'int ({{.*}})' non_odr_use_unevaluated +// CHECK-NEXT: `-FunctionDecl [[ADDR_41:0x[a-z0-9]*]] col:5 non_equivalent_isa_trait 'int ({{.*}})' +// CHECK-NEXT: `-OMPDeclareVariantAttr [[ADDR_42:0x[a-z0-9]*]] Implicit device={isa(sse2, sse)} +// CHECK-NEXT: `-DeclRefExpr [[ADDR_43:0x[a-z0-9]*]] '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 --- a/clang/test/OpenMP/declare_variant_messages.c +++ b/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