Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 9, 2025

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);

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
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