[OpenMP] implementation set controls elision for begin declare variant (#139287)

jdoerfert · web-flow · commit 73165de4e681 · 2025-05-09T16:32:39.000-07:00
The device and implementation set should trigger elision of tokens if they do not match statically in a begin/end declare variant. This simply extends the logic from the device set only and includes the implementation set. Reported by @kkwli.
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2282,7 +2282,8 @@ Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(
         /* ConstructTraits */ ArrayRef<llvm::omp::TraitProperty>(),
         Actions.OpenMP().getOpenMPDeviceNum());
 
-    if (isVariantApplicableInContext(VMI, OMPCtx, /* DeviceSetOnly */ true)) {
+    if (isVariantApplicableInContext(VMI, OMPCtx,
+                                     /*DeviceOrImplementationSetOnly=*/true)) {
       Actions.OpenMP().ActOnOpenMPBeginDeclareVariant(Loc, TI);
       break;
     }
diff --git a/clang/test/AST/ast-dump-openmp-begin-declare-variant_6.c b/clang/test/AST/ast-dump-openmp-begin-declare-variant_6.c
@@ -29,28 +29,13 @@ int main(void) {
 //  - we do use the original pointers for the calls as the variants are not applicable (this is not the ibm compiler).
 
 // 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' 0
-// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_4:0x[a-z0-9]*]] <<invalid sloc>> Implicit implementation={vendor(ibm)}
-// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_5:0x[a-z0-9]*]] <line:13:1> 'int ({{.*}})' Function [[ADDR_6:0x[a-z0-9]*]] 'also_before[implementation={vendor(ibm)}]' 'int ({{.*}})'
-// CHECK-NEXT: |-FunctionDecl [[ADDR_7:0x[a-z0-9]*]] <line:10:1, col:20> col:5 implicit used also_after 'int ({{.*}})'
-// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_8:0x[a-z0-9]*]] <<invalid sloc>> Implicit implementation={vendor(ibm)}
-// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_9:0x[a-z0-9]*]] <col:1> 'int ({{.*}})' Function [[ADDR_10:0x[a-z0-9]*]] 'also_after[implementation={vendor(ibm)}]' 'int ({{.*}})'
-// CHECK-NEXT: |-FunctionDecl [[ADDR_10]] <col:1, line:12:1> line:10:1 also_after[implementation={vendor(ibm)}] 'int ({{.*}})'
-// CHECK-NEXT: | `-CompoundStmt [[ADDR_11:0x[a-z0-9]*]] <col:22, line:12:1>
-// CHECK-NEXT: |   `-ReturnStmt [[ADDR_12:0x[a-z0-9]*]] <line:11:3, col:10>
-// CHECK-NEXT: |     `-IntegerLiteral [[ADDR_13:0x[a-z0-9]*]] <col:10> 'int' 1
-// CHECK-NEXT: |-FunctionDecl [[ADDR_6]] <line:13:1, line:15:1> line:13:1 also_before[implementation={vendor(ibm)}] 'int ({{.*}})'
-// CHECK-NEXT: | `-CompoundStmt [[ADDR_14:0x[a-z0-9]*]] <col:23, line:15:1>
-// CHECK-NEXT: |   `-ReturnStmt [[ADDR_15:0x[a-z0-9]*]] <line:14:3, col:10>
-// CHECK-NEXT: |     `-IntegerLiteral [[ADDR_16:0x[a-z0-9]*]] <col:10> 'int' 2
-// CHECK-NEXT: |-FunctionDecl [[ADDR_17:0x[a-z0-9]*]] prev [[ADDR_7]] <line:18:1, line:20:1> line:18:5 used also_after 'int ({{.*}})'
-// CHECK-NEXT: | |-CompoundStmt [[ADDR_18:0x[a-z0-9]*]] <col:22, line:20:1>
-// CHECK-NEXT: | | `-ReturnStmt [[ADDR_19:0x[a-z0-9]*]] <line:19:3, col:10>
-// CHECK-NEXT: | |   `-IntegerLiteral [[ADDR_20:0x[a-z0-9]*]] <col:10> 'int' 0
-// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_21:0x[a-z0-9]*]] <<invalid sloc>> Inherited Implicit implementation={vendor(ibm)}
-// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_9]] <line:10:1> 'int ({{.*}})' Function [[ADDR_10]] 'also_after[implementation={vendor(ibm)}]' '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' 0
+// CHECK-NEXT: |-FunctionDecl [[ADDR_17:0x[a-z0-9]*]] <line:18:1, line:20:1> line:18:5 used also_after 'int ({{.*}})'
+// CHECK-NEXT: | `-CompoundStmt [[ADDR_18:0x[a-z0-9]*]] <col:22, line:20:1>
+// CHECK-NEXT: |  `-ReturnStmt [[ADDR_19:0x[a-z0-9]*]] <line:19:3, col:10>
+// CHECK-NEXT: |    `-IntegerLiteral [[ADDR_20:0x[a-z0-9]*]] <col:10> 'int' 0
 // CHECK-NEXT: `-FunctionDecl [[ADDR_22:0x[a-z0-9]*]] <line:22:1, line:25:1> line:22:5 main 'int ({{.*}})'
 // CHECK-NEXT:   `-CompoundStmt [[ADDR_23:0x[a-z0-9]*]] <col:16, line:25:1>
 // CHECK-NEXT:     `-ReturnStmt [[ADDR_24:0x[a-z0-9]*]] <line:24:3, col:37>
diff --git a/clang/test/AST/ast-dump-openmp-begin-declare-variant_7.c b/clang/test/AST/ast-dump-openmp-begin-declare-variant_7.c
@@ -1,6 +1,6 @@
-// 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
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify=C   -ast-dump %s       | FileCheck %s --check-prefixes=CHECK,C
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify=CXX -ast-dump %s -x c++| FileCheck %s --check-prefixes=CHECK,CXX
+
 
 int OK_1(void);
 
@@ -23,59 +23,30 @@ int OK_3(void);
 
 int test(void) {
   // Should cause an error due to not_OK()
-  return OK_1() + not_OK() + OK_3();
+  return OK_1() + not_OK() + OK_3(); // CXX-error {{use of undeclared identifier 'not_OK'}} C-error {{call to undeclared function 'not_OK'; ISO C99 and later do not support implicit function declarations}}
 }
 
 // Make sure:
 //  - we see a single error for `not_OK`
 //  - we do not see errors for OK_{1,2,3}
-//  FIXME: We actually do not see there error here.
-//         This case is unlikely to happen in practise and hard to diagnose during SEMA.
-//         We will issue an error during code generation instead. This is similar to the
-//         diagnosis in other multi-versioning schemes.
 
 // CHECK:      |-FunctionDecl [[ADDR_0:0x[a-z0-9]*]] <{{.*}}, col:14> col:5 used OK_1 'int ({{.*}})'
-// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_1:0x[a-z0-9]*]] <<invalid sloc>> Implicit implementation={vendor(intel)}
-// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_2:0x[a-z0-9]*]] <line:8:1> 'int ({{.*}})' Function [[ADDR_3:0x[a-z0-9]*]] 'OK_1[implementation={vendor(intel)}]' 'int ({{.*}})'
-// CHECK-NEXT: |-FunctionDecl [[ADDR_3]] <col:1, line:10:1> line:8:1 OK_1[implementation={vendor(intel)}] 'int ({{.*}})'
-// CHECK-NEXT: | `-CompoundStmt [[ADDR_4:0x[a-z0-9]*]] <col:16, line:10:1>
-// CHECK-NEXT: |   `-ReturnStmt [[ADDR_5:0x[a-z0-9]*]] <line:9:3, col:10>
-// CHECK-NEXT: |     `-IntegerLiteral [[ADDR_6:0x[a-z0-9]*]] <col:10> 'int' 1
-// CHECK-NEXT: |-FunctionDecl [[ADDR_7:0x[a-z0-9]*]] <line:11:1, col:14> col:5 implicit OK_2 'int ({{.*}})'
-// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_8:0x[a-z0-9]*]] <<invalid sloc>> Implicit implementation={vendor(intel)}
-// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_9:0x[a-z0-9]*]] <col:1> 'int ({{.*}})' Function [[ADDR_10:0x[a-z0-9]*]] 'OK_2[implementation={vendor(intel)}]' 'int ({{.*}})'
-// CHECK-NEXT: |-FunctionDecl [[ADDR_10]] <col:1, line:13:1> line:11:1 OK_2[implementation={vendor(intel)}] 'int ({{.*}})'
-// CHECK-NEXT: | `-CompoundStmt [[ADDR_11:0x[a-z0-9]*]] <col:16, line:13:1>
-// CHECK-NEXT: |   `-ReturnStmt [[ADDR_12:0x[a-z0-9]*]] <line:12:3, col:10>
-// CHECK-NEXT: |     `-IntegerLiteral [[ADDR_13:0x[a-z0-9]*]] <col:10> 'int' 1
-// CHECK-NEXT: |-FunctionDecl [[ADDR_14:0x[a-z0-9]*]] <line:14:1, col:16> col:5 implicit used not_OK 'int ({{.*}})'
-// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_15:0x[a-z0-9]*]] <<invalid sloc>> Implicit implementation={vendor(intel)}
-// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_16:0x[a-z0-9]*]] <col:1> 'int ({{.*}})' Function [[ADDR_17:0x[a-z0-9]*]] 'not_OK[implementation={vendor(intel)}]' 'int ({{.*}})'
-// CHECK-NEXT: |-FunctionDecl [[ADDR_17]] <col:1, line:16:1> line:14:1 not_OK[implementation={vendor(intel)}] 'int ({{.*}})'
-// CHECK-NEXT: | `-CompoundStmt [[ADDR_18:0x[a-z0-9]*]] <col:18, line:16:1>
-// CHECK-NEXT: |   `-ReturnStmt [[ADDR_19:0x[a-z0-9]*]] <line:15:3, col:10>
-// CHECK-NEXT: |     `-IntegerLiteral [[ADDR_20:0x[a-z0-9]*]] <col:10> 'int' 1
-// CHECK-NEXT: |-FunctionDecl [[ADDR_21:0x[a-z0-9]*]] <line:17:1, col:14> col:5 implicit used OK_3 'int ({{.*}})'
-// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_22:0x[a-z0-9]*]] <<invalid sloc>> Implicit implementation={vendor(intel)}
-// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_23:0x[a-z0-9]*]] <col:1> 'int ({{.*}})' Function [[ADDR_24:0x[a-z0-9]*]] 'OK_3[implementation={vendor(intel)}]' 'int ({{.*}})'
-// CHECK-NEXT: |-FunctionDecl [[ADDR_24]] <col:1, line:19:1> line:17:1 OK_3[implementation={vendor(intel)}] 'int ({{.*}})'
-// CHECK-NEXT: | `-CompoundStmt [[ADDR_25:0x[a-z0-9]*]] <col:16, line:19:1>
-// CHECK-NEXT: |   `-ReturnStmt [[ADDR_26:0x[a-z0-9]*]] <line:18:3, col:10>
-// CHECK-NEXT: |     `-IntegerLiteral [[ADDR_27:0x[a-z0-9]*]] <col:10> 'int' 1
-// CHECK-NEXT: |-FunctionDecl [[ADDR_28:0x[a-z0-9]*]] prev [[ADDR_21]] <line:22:1, col:14> col:5 used OK_3 'int ({{.*}})'
-// CHECK-NEXT: | `-OMPDeclareVariantAttr [[ADDR_29:0x[a-z0-9]*]] <<invalid sloc>> Inherited Implicit implementation={vendor(intel)}
-// CHECK-NEXT: |   `-DeclRefExpr [[ADDR_23]] <line:17:1> 'int ({{.*}})' Function [[ADDR_24]] 'OK_3[implementation={vendor(intel)}]' 'int ({{.*}})'
+// CHECK-NEXT: |-FunctionDecl [[ADDR_28:0x[a-z0-9]*]] <line:22:1, col:14> col:5 used OK_3 'int ({{.*}})'
 // CHECK-NEXT: `-FunctionDecl [[ADDR_30:0x[a-z0-9]*]] <line:24:1, line:27:1> line:24:5 test 'int ({{.*}})'
 // CHECK-NEXT:   `-CompoundStmt [[ADDR_31:0x[a-z0-9]*]] <col:16, line:27:1>
 // CHECK-NEXT:     `-ReturnStmt [[ADDR_32:0x[a-z0-9]*]] <line:26:3, col:35>
-// CHECK-NEXT:       `-BinaryOperator [[ADDR_33:0x[a-z0-9]*]] <col:10, col:35> 'int' '+'
-// CHECK-NEXT:         |-BinaryOperator [[ADDR_34:0x[a-z0-9]*]] <col:10, col:26> 'int' '+'
+// C:                `-BinaryOperator [[ADDR_33:0x[a-z0-9]*]] <col:10, col:35> 'int' '+'
+// C-NEXT:             |-BinaryOperator [[ADDR_34:0x[a-z0-9]*]] <col:10, col:26> 'int' '+'
+// CXX:              `-BinaryOperator [[ADDR_33:0x[a-z0-9]*]] <col:10, col:35> '<dependent type>' contains-errors '+'
+// CXX-NEXT:           |-BinaryOperator [[ADDR_34:0x[a-z0-9]*]] <col:10, col:26> '<dependent type>' contains-errors '+'
 // CHECK-NEXT:         | |-CallExpr [[ADDR_35:0x[a-z0-9]*]] <col:10, col:15> 'int'
 // CHECK-NEXT:         | | `-ImplicitCastExpr [[ADDR_36:0x[a-z0-9]*]] <col:10> 'int (*)({{.*}})' <FunctionToPointerDecay>
 // CHECK-NEXT:         | |   `-DeclRefExpr [[ADDR_37:0x[a-z0-9]*]] <col:10> 'int ({{.*}})' {{.*}}Function [[ADDR_0]] 'OK_1' 'int ({{.*}})'
-// CHECK-NEXT:         | `-CallExpr [[ADDR_38:0x[a-z0-9]*]] <col:19, col:26> 'int'
-// CHECK-NEXT:         |   `-ImplicitCastExpr [[ADDR_39:0x[a-z0-9]*]] <col:19> 'int (*)({{.*}})' <FunctionToPointerDecay>
-// CHECK-NEXT:         |     `-DeclRefExpr [[ADDR_40:0x[a-z0-9]*]] <col:19> 'int ({{.*}})' {{.*}}Function [[ADDR_14]] 'not_OK' 'int ({{.*}})'
+// C:                  | `-CallExpr [[ADDR_38:0x[a-z0-9]*]] <col:19, col:26> 'int'
+// C-NEXT:             |   `-ImplicitCastExpr [[ADDR_39:0x[a-z0-9]*]] <col:19> 'int (*)({{.*}})' <FunctionToPointerDecay>
+// C-NEXT:             |     `-DeclRefExpr [[ADDR_40:0x[a-z0-9]*]] <col:19> 'int ({{.*}})' {{.*}}Function {{.*}} 'not_OK' 'int ({{.*}})'
+// CXX:                | `-RecoveryExpr {{.*}} <col:19, col:26> '<dependent type>' contains-errors lvalue
+// CXX-NEXT:           |   `-UnresolvedLookupExpr {{.*}} <col:19> '<overloaded function type>' lvalue (ADL) = 'not_OK' empty
 // CHECK-NEXT:         `-CallExpr [[ADDR_41:0x[a-z0-9]*]] <col:30, col:35> 'int'
 // CHECK-NEXT:           `-ImplicitCastExpr [[ADDR_42:0x[a-z0-9]*]] <col:30> 'int (*)({{.*}})' <FunctionToPointerDecay>
 // CHECK-NEXT:             `-DeclRefExpr [[ADDR_43:0x[a-z0-9]*]] <col:30> 'int ({{.*}})' {{.*}}Function [[ADDR_28]] 'OK_3' 'int ({{.*}})'
diff --git a/clang/test/OpenMP/begin_declare_variant_elided_range_implementation.c b/clang/test/OpenMP/begin_declare_variant_elided_range_implementation.c
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp -x c -std=c99 -fms-extensions -Wno-pragma-pack %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -verify -fopenmp-simd -x c -std=c99 -fms-extensions -Wno-pragma-pack %s
+
+// expected-no-diagnostics
+
+#pragma omp begin declare variant match(implementation={vendor(llvm)})
+typedef int bar;
+void f() {}
+#pragma omp end declare variant
+
+#pragma omp begin declare variant match(implementation={vendor(ibm)})
+typedef float bar;
+void f() {}
+#pragma omp end declare variant
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPContext.h b/llvm/include/llvm/Frontend/OpenMP/OMPContext.h
@@ -180,12 +180,13 @@ struct OMPContext {
 };
 
 /// Return true if \p VMI is applicable in \p Ctx, that is, all traits required
-/// by \p VMI are available in the OpenMP context \p Ctx. If \p DeviceSetOnly is
-/// true, only the device selector set, if present, are checked. Note that we
-/// still honor extension traits provided by the user.
+/// by \p VMI are available in the OpenMP context \p Ctx. If
+/// \p DeviceOrImplementationSetOnly is true, only the device and implementation
+/// selector set, if present, are checked. Note that we still honor extension
+/// traits provided by the user.
 bool isVariantApplicableInContext(const VariantMatchInfo &VMI,
                                   const OMPContext &Ctx,
-                                  bool DeviceSetOnly = false);
+                                  bool DeviceOrImplementationSetOnly = false);
 
 /// Return the index (into \p VMIs) of the variant with the highest score
 /// from the ones applicable in \p Ctx. See llvm::isVariantApplicableInContext.
diff --git a/llvm/lib/Frontend/OpenMP/OMPContext.cpp b/llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -193,9 +193,11 @@ static bool isStrictSubset(const VariantMatchInfo &VMI0,
   return true;
 }
 
-static int isVariantApplicableInContextHelper(
-    const VariantMatchInfo &VMI, const OMPContext &Ctx,
-    SmallVectorImpl<unsigned> *ConstructMatches, bool DeviceSetOnly) {
+static int
+isVariantApplicableInContextHelper(const VariantMatchInfo &VMI,
+                                   const OMPContext &Ctx,
+                                   SmallVectorImpl<unsigned> *ConstructMatches,
+                                   bool DeviceOrImplementationSetOnly) {
 
   // The match kind determines if we need to match all traits, any of the
   // traits, or none of the traits for it to be an applicable context.
@@ -245,8 +247,10 @@ static int isVariantApplicableInContextHelper(
 
   for (unsigned Bit : VMI.RequiredTraits.set_bits()) {
     TraitProperty Property = TraitProperty(Bit);
-    if (DeviceSetOnly &&
-        getOpenMPContextTraitSetForProperty(Property) != TraitSet::device)
+    if (DeviceOrImplementationSetOnly &&
+        getOpenMPContextTraitSetForProperty(Property) != TraitSet::device &&
+        getOpenMPContextTraitSetForProperty(Property) !=
+            TraitSet::implementation)
       continue;
 
     // So far all extensions are handled elsewhere, we skip them here as they
@@ -272,7 +276,7 @@ static int isVariantApplicableInContextHelper(
       return *Result;
   }
 
-  if (!DeviceSetOnly) {
+  if (!DeviceOrImplementationSetOnly) {
     // We could use isSubset here but we also want to record the match
     // locations.
     unsigned ConstructIdx = 0, NoConstructTraits = Ctx.ConstructTraits.size();
@@ -315,11 +319,11 @@ static int isVariantApplicableInContextHelper(
   return true;
 }
 
-bool llvm::omp::isVariantApplicableInContext(const VariantMatchInfo &VMI,
-                                             const OMPContext &Ctx,
-                                             bool DeviceSetOnly) {
+bool llvm::omp::isVariantApplicableInContext(
+    const VariantMatchInfo &VMI, const OMPContext &Ctx,
+    bool DeviceOrImplementationSetOnly) {
   return isVariantApplicableInContextHelper(
-      VMI, Ctx, /* ConstructMatches */ nullptr, DeviceSetOnly);
+      VMI, Ctx, /* ConstructMatches */ nullptr, DeviceOrImplementationSetOnly);
 }
 
 static APInt getVariantMatchScore(const VariantMatchInfo &VMI,
@@ -418,8 +422,9 @@ int llvm::omp::getBestVariantMatchForContext(
 
     SmallVector<unsigned, 8> ConstructMatches;
     // If the variant is not applicable its not the best.
-    if (!isVariantApplicableInContextHelper(VMI, Ctx, &ConstructMatches,
-                                            /* DeviceSetOnly */ false))
+    if (!isVariantApplicableInContextHelper(
+            VMI, Ctx, &ConstructMatches,
+            /* DeviceOrImplementationSetOnly */ false))
       continue;
     // Check if its clearly not the best.
     APInt Score = getVariantMatchScore(VMI, Ctx, ConstructMatches);

Original file line number	Diff line number	Diff line change
`@@ -2282,7 +2282,8 @@ Parser::DeclGroupPtrTy Parser::ParseOpenMPDeclarativeDirectiveWithExtDecl(`
`2282`	`2282`	`/* ConstructTraits */ ArrayRef<llvm::omp::TraitProperty>(),`
`2283`	`2283`	`Actions.OpenMP().getOpenMPDeviceNum());`
`2284`	`2284`
`2285`		`- if (isVariantApplicableInContext(VMI, OMPCtx, /* DeviceSetOnly */ true)) {`
	`2285`	`+ if (isVariantApplicableInContext(VMI, OMPCtx,`
	`2286`	`+ /DeviceOrImplementationSetOnly=/true)) {`
`2286`	`2287`	`Actions.OpenMP().ActOnOpenMPBeginDeclareVariant(Loc, TI);`
`2287`	`2288`	`break;`
`2288`	`2289`	`}`