[clang] 3c71e66 - [clang][OpenMP] Match device and target_device ISA names separately (#232996)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 21 23:52:32 PDT 2026
Author: Chi-Chun, Chen
Date: 2026-10-22T01:52:25-05:00
New Revision: 3c71e661b96c89a82338163128cc9be4578f7f1a
URL: https://github.com/llvm/llvm-project/commit/3c71e661b96c89a82338163128cc9be4578f7f1a
DIFF: https://github.com/llvm/llvm-project/commit/3c71e661b96c89a82338163128cc9be4578f7f1a.diff
LOG: [clang][OpenMP] Match device and target_device ISA names separately (#232996)
A context selector can name ISAs in both `device={isa(...)}` and
`target_device={isa(...)}`. These are separate selectors, but their ISA
names were checked together, so an ISA that the target does not support
made both selectors fail. The default match_all needs both selectors
anyway, but the `match_any` and `match_none` extensions selected the
wrong variant. For example, on x86-64:
```c
#pragma omp declare variant(variant) \
match(device = {isa(sse2)}, target_device = {isa(bogus_isa)}, \
implementation = {extension(match_any)})
```
did not select variant, although `device={isa(sse2)}` matches. Likewise,
`match_none` accepted a variant whose `target_device` ISA matched.
This patch records which selector each ISA name belongs to and check
each selector against its own names only. Because begin declare variant
only looks at the device and implementation sets, a `target_device` ISA
name no longer causes a begin declare variant block to be elided.
Fixes https://github.com/llvm/llvm-project/issues/232990.
Assisted-by: GitHub Copilot (Claude Opus 5.5)
Added:
clang/test/OpenMP/declare_variant_isa_selectors.c
Modified:
llvm/include/llvm/Frontend/OpenMP/OMPContext.h
llvm/lib/Frontend/OpenMP/OMPContext.cpp
llvm/unittests/Frontend/OpenMPContextTest.cpp
Removed:
################################################################################
diff --git a/clang/test/OpenMP/declare_variant_isa_selectors.c b/clang/test/OpenMP/declare_variant_isa_selectors.c
new file mode 100644
index 0000000000000..0b022638a538a
--- /dev/null
+++ b/clang/test/OpenMP/declare_variant_isa_selectors.c
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -fopenmp -triple x86_64-unknown-linux -emit-llvm %s \
+// RUN: -Wno-source-uses-openmp -o - | FileCheck %s
+
+// Device and target_device ISA selectors are matched independently.
+
+int variant(void);
+
+// MATCH_ANY is satisfied by the supported device ISA.
+#pragma omp declare variant(variant) \
+ match(device = {isa(sse2)}, target_device = {isa(bogus_isa)}, \
+ implementation = {extension(match_any)})
+int any_base(void);
+
+// CHECK-LABEL: define{{.*}} i32 @any_caller(
+// CHECK: call i32 @variant()
+int any_caller(void) { return any_base(); }
+
+// MATCH_NONE is not satisfied because the target_device ISA is supported.
+#pragma omp declare variant(variant) \
+ match(device = {isa(bogus_isa)}, target_device = {isa(sse2)}, \
+ implementation = {extension(match_none)})
+int none_base(void);
+
+// CHECK-LABEL: define{{.*}} i32 @none_caller(
+// CHECK: call i32 @none_base()
+int none_caller(void) { return none_base(); }
+
+// BEGIN DECLARE VARIANT ignores target_device traits, so the unsupported
+// target_device ISA does not elide the block.
+int mixed(void) { return 0; }
+#pragma omp begin declare variant \
+ match(device = {isa(sse2)}, target_device = {isa(bogus_isa)})
+// CHECK: define{{.*}} i32 @"mixed$ompvariant${{.*}}"()
+int mixed(void) { return 1; }
+#pragma omp end declare variant
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPContext.h b/llvm/include/llvm/Frontend/OpenMP/OMPContext.h
index 00837c661b54d..1231fa1b9c1ca 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPContext.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPContext.h
@@ -122,6 +122,15 @@ LLVM_ABI bool isValidTraitPropertyForTraitSetAndSelector(TraitProperty Property,
/// scored (via the ScoresMap). In addition, the required consturct nesting is
/// decribed as well.
struct VariantMatchInfo {
+ /// A device or target_device isa property and the ISA name it requires.
+ struct ISATrait {
+ TraitProperty Property;
+ StringRef Name;
+ bool operator==(const ISATrait &Other) const {
+ return Property == Other.Property && Name == Other.Name;
+ }
+ };
+
/// Add the trait \p Property to the required trait set. \p RawString is the
/// string we parsed and derived \p Property from. If \p Score is not null, it
/// recorded as well. If \p Property is in the `construct` set it is recorded
@@ -142,10 +151,9 @@ struct VariantMatchInfo {
// Special handling for `device={isa(...)}` as we do not match the enum but
// the raw string.
- if (Property == TraitProperty::device_isa___ANY)
- ISATraits.push_back(RawString);
- if (Property == TraitProperty::target_device_isa___ANY)
- ISATraits.push_back(RawString);
+ if (Property == TraitProperty::device_isa___ANY ||
+ Property == TraitProperty::target_device_isa___ANY)
+ ISATraits.push_back({Property, RawString});
RequiredTraits.set(unsigned(Property));
if (Set == TraitSet::construct)
@@ -153,7 +161,7 @@ struct VariantMatchInfo {
}
BitVector RequiredTraits = BitVector(unsigned(TraitProperty::Last) + 1);
- SmallVector<StringRef, 8> ISATraits;
+ SmallVector<ISATrait, 8> ISATraits;
SmallVector<TraitProperty, 8> ConstructTraits;
SmallDenseMap<TraitProperty, APInt> ScoreMap;
};
diff --git a/llvm/lib/Frontend/OpenMP/OMPContext.cpp b/llvm/lib/Frontend/OpenMP/OMPContext.cpp
index e77e48913fdd0..c7b689aee50ef 100644
--- a/llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -182,9 +182,10 @@ static bool isStrictSubset(const VariantMatchInfo &VMI0,
TraitProperty::user_condition_unknown})
if (VMI0.RequiredTraits.test(unsigned(Property)))
return false;
- return llvm::all_of(VMI0.ISATraits, [&](StringRef Name) {
- return llvm::is_contained(VMI1.ISATraits, Name);
- });
+ return llvm::all_of(VMI0.ISATraits,
+ [&](const VariantMatchInfo::ISATrait &Trait) {
+ return llvm::is_contained(VMI1.ISATraits, Trait);
+ });
}
static int
@@ -257,13 +258,10 @@ isVariantApplicableInContextHelper(const VariantMatchInfo &VMI,
// We overwrite the isa trait as it is actually up to the OMPContext hook to
// check the raw string(s).
- if (Property == TraitProperty::device_isa___ANY)
- IsActiveTrait = llvm::all_of(VMI.ISATraits, [&](StringRef RawString) {
- return Ctx.matchesISATrait(RawString);
- });
- if (Property == TraitProperty::target_device_isa___ANY)
- IsActiveTrait = llvm::all_of(VMI.ISATraits, [&](StringRef RawString) {
- return Ctx.matchesISATrait(RawString);
+ if (Property == TraitProperty::device_isa___ANY ||
+ Property == TraitProperty::target_device_isa___ANY)
+ IsActiveTrait = llvm::all_of(VMI.ISATraits, [&](const auto &Trait) {
+ return Trait.Property != Property || Ctx.matchesISATrait(Trait.Name);
});
if (std::optional<bool> Result = HandleTrait(Property, IsActiveTrait))
diff --git a/llvm/unittests/Frontend/OpenMPContextTest.cpp b/llvm/unittests/Frontend/OpenMPContextTest.cpp
index ddba513e5330c..5227229a56173 100644
--- a/llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -313,6 +313,38 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) {
}
}
+TEST_F(OpenMPContextTest, ApplicabilityISATraits) {
+ struct SSE2Context : OMPContext {
+ using OMPContext::OMPContext;
+ bool matchesISATrait(StringRef ISA) const override { return ISA == "sse2"; }
+ };
+ SSE2Context Context(false, Triple("x86_64-unknown-linux"), Triple(), -1);
+
+ // An unsupported ISA in one selector must not fail the other selector.
+ for (auto [Supported, Unsupported] :
+ {std::pair(TraitProperty::device_isa___ANY,
+ TraitProperty::target_device_isa___ANY),
+ std::pair(TraitProperty::target_device_isa___ANY,
+ TraitProperty::device_isa___ANY)}) {
+ VariantMatchInfo VMI;
+ VMI.addTrait(Supported, "sse2");
+ VMI.addTrait(Unsupported, "bogus_isa");
+ EXPECT_FALSE(isVariantApplicableInContext(VMI, Context));
+ VariantMatchInfo Any = VMI, None = VMI;
+ Any.addTrait(TraitProperty::implementation_extension_match_any, "");
+ EXPECT_TRUE(isVariantApplicableInContext(Any, Context));
+ None.addTrait(TraitProperty::implementation_extension_match_none, "");
+ EXPECT_FALSE(isVariantApplicableInContext(None, Context));
+ }
+
+ // Every name in one selector is still required.
+ VariantMatchInfo TwoNames;
+ TwoNames.addTrait(TraitProperty::device_isa___ANY, "sse2");
+ TwoNames.addTrait(TraitProperty::device_isa___ANY, "bogus_isa");
+ TwoNames.addTrait(TraitProperty::implementation_extension_match_any, "");
+ EXPECT_FALSE(isVariantApplicableInContext(TwoNames, Context));
+}
+
TEST_F(OpenMPContextTest, ScoringSimple) {
// TODO: Add scoring tests (via getBestVariantMatchForContext).
}
@@ -391,7 +423,7 @@ TEST_F(OpenMPContextTest, RepeatedConstructWithConditionOrISA) {
// Each pair ties and requires the same traits, but the conditions or ISA
// names may
diff er, so clause order decides.
- APInt Four(64, 4), Twenty(64, 20);
+ APInt Four(64, 4), Twenty(64, 20), ThirtySix(64, 36);
for (auto [Property, Score] :
{std::pair(TraitProperty::user_condition_true, &Four),
std::pair(TraitProperty::device_isa___ANY, &Twenty)}) {
@@ -419,6 +451,17 @@ TEST_F(OpenMPContextTest, RepeatedConstructWithConditionOrISA) {
EXPECT_EQ(getBestVariantMatchForContext(Candidates, Context), 0);
std::swap(Candidates[0], Candidates[1]);
EXPECT_EQ(getBestVariantMatchForContext(Candidates, Context), 1);
+
+ // The same ISA names in swapped selectors are not subsets of each other.
+ ParallelFor.addTrait(TraitProperty::implementation_vendor_llvm, "",
+ &ThirtySix);
+ ParallelFor.addTrait(TraitProperty::target_device_isa___ANY, "sse2");
+ ParallelForParallel.addTrait(TraitProperty::target_device_isa___ANY,
+ "sse");
+ Candidates = {ParallelFor, ParallelForParallel};
+ EXPECT_EQ(getBestVariantMatchForContext(Candidates, Context), 0);
+ std::swap(Candidates[0], Candidates[1]);
+ EXPECT_EQ(getBestVariantMatchForContext(Candidates, Context), 0);
}
}
}
More information about the cfe-commits
mailing list