Skip to content

Conversation

@jdoerfert
Copy link
Member

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.

@jdoerfert jdoerfert added openmp flang:openmp clang:openmp OpenMP related changes to Clang labels May 9, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 9, 2025
@llvmbot
Copy link
Member

llvmbot commented May 9, 2025

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-openmp

Author: Johannes Doerfert (jdoerfert)

Changes

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.


Full diff: https://github.com/llvm/llvm-project/pull/139287.diff

4 Files Affected:

  • (modified) clang/lib/Parse/ParseOpenMP.cpp (+1-1)
  • (added) clang/test/OpenMP/begin_declare_variant_elided_range_implementation.c (+14)
  • (modified) llvm/include/llvm/Frontend/OpenMP/OMPContext.h (+5-4)
  • (modified) llvm/lib/Frontend/OpenMP/OMPContext.cpp (+17-12)
diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 8d8698e61216f..81f813134bfd1 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2270,7 +2270,7 @@ 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/OpenMP/begin_declare_variant_elided_range_implementation.c b/clang/test/OpenMP/begin_declare_variant_elided_range_implementation.c
new file mode 100644
index 0000000000000..e50c0c4ca7573
--- /dev/null
+++ 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
index 26163fdb4b63d..9942cbd08aa43 100644
--- 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
index 2edfd786c5c23..7c56233cfc9bc 100644
--- 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);

@github-actions
Copy link

github-actions bot commented May 9, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@jdoerfert
Copy link
Member Author

I'll look at the test failures and formatting before I commit this.

@jdoerfert jdoerfert force-pushed the begin_decl_variant_impl_set branch from cde3fb4 to 4ae732b Compare May 9, 2025 21:23
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.
@jdoerfert jdoerfert force-pushed the begin_decl_variant_impl_set branch from 4ae732b to ab057a2 Compare May 9, 2025 21:24
@jdoerfert jdoerfert merged commit 73165de into llvm:main May 9, 2025
8 of 10 checks passed
@jdoerfert jdoerfert deleted the begin_decl_variant_impl_set branch June 4, 2025 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category flang:openmp openmp

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants