https://github.com/chichunchen created 
https://github.com/llvm/llvm-project/pull/233905

…e others

When several context selectors tie for the highest score, a selector that is a 
strict subset of another tied selector loses, and otherwise the first one wins. 
getBestVariantMatchForContext only compared each candidate with the current 
best, which isn't enough because that comparison isn't transitive:

```
  #pragma omp metadirective                                        \
      when(user={condition(1)},                                    \
           implementation={vendor(score(4): llvm)}: taskyield)     \
      when(device={kind(cpu)},                                     \
           implementation={vendor(score(3): llvm)}: taskwait)      \
      when(user={condition(1)}, device={kind(host)},               \
           implementation={vendor(score(3): llvm)}: barrier)
```

All three score 5 on the host. The first is a strict subset of the third, so 
TASKWAIT should be selected. The scan kept the first over the second, replaced 
it with the third, and never compared the third with the second, so it selected 
BARRIER.

This patch collects all variants with the highest score, drops any that is a 
strict subset of another of them, and selects the first one left. As before, a 
strict subset only loses to a variant with the same score. This affects 
metadirective and declare variant selection in Clang and Flang.

Fixes #233893.

Assisted-by: GitHub Copilot (Claude Opus 5.5)

>From 6278453680db4f0058ca12663f87b84c1b914ea4 Mon Sep 17 00:00:00 2001
From: "Chi Chun, Chen" <[email protected]>
Date: Tue, 20 Oct 2026 11:19:22 -0500
Subject: [PATCH] [OpenMP][clang][flang] Compare each tied context selector
 with all the others

When several context selectors tie for the highest score, a selector that
is a strict subset of another tied selector loses, and otherwise the first
one wins. getBestVariantMatchForContext only compared each candidate with
the current best, which isn't enough because that comparison isn't
transitive:

```
  #pragma omp metadirective                                        \
      when(user={condition(1)},                                    \
           implementation={vendor(score(4): llvm)}: taskyield)     \
      when(device={kind(cpu)},                                     \
           implementation={vendor(score(3): llvm)}: taskwait)      \
      when(user={condition(1)}, device={kind(host)},               \
           implementation={vendor(score(3): llvm)}: barrier)
```

All three score 5 on the host. The first is a strict subset of the third,
so TASKWAIT should be selected. The scan kept the first over the second,
replaced it with the third, and never compared the third with the second,
so it selected BARRIER.

This patch collects all variants with the highest score, drops any that is
a strict subset of another of them, and selects the first one left. As
before, a strict subset only loses to a variant with the same score. This
affects metadirective and declare variant selection in Clang and Flang.

Fixes #233893.

Assisted-by: GitHub Copilot (Claude Opus 5.5)
---
 .../metadirective_implementation_codegen.c    | 19 +++++++++++++
 .../test/Lower/OpenMP/metadirective-user.f90  | 15 ++++++++++
 llvm/lib/Frontend/OpenMP/OMPContext.cpp       | 28 +++++++++----------
 llvm/unittests/Frontend/OpenMPContextTest.cpp | 23 +++++++++++++++
 4 files changed, 70 insertions(+), 15 deletions(-)

diff --git a/clang/test/OpenMP/metadirective_implementation_codegen.c 
b/clang/test/OpenMP/metadirective_implementation_codegen.c
index da09b639d6d40..fd1c752670201 100644
--- a/clang/test/OpenMP/metadirective_implementation_codegen.c
+++ b/clang/test/OpenMP/metadirective_implementation_codegen.c
@@ -68,4 +68,23 @@ void foo(void) {
 // NO-CHECK: call void @__kmpc_for_static_fini
 // CHECK: ret void
 
+// All three score 5. The first is a strict subset of the third, which ties
+// with the second, so the second is selected.
+void tie(void) {
+#pragma omp metadirective                                                    \
+    when(user = {condition(1)}, implementation = {vendor(score(4) : llvm)}   \
+         : taskyield)                                                        \
+    when(device = {kind(cpu)}, implementation = {vendor(score(3) : llvm)}    \
+         : taskwait)                                                         \
+    when(user = {condition(1)}, device = {kind(host)},                       \
+         implementation = {vendor(score(3) : llvm)} : barrier)
+}
+
+// CHECK-LABEL: define {{.*}}void @tie(
+// CHECK-NOT: call {{.*}}@__kmpc_omp_taskyield(
+// CHECK-NOT: call {{.*}}@__kmpc_barrier(
+// CHECK: call {{.*}}@__kmpc_omp_taskwait(
+// CHECK-NOT: call {{.*}}@__kmpc_barrier(
+// CHECK: ret void
+
 #endif
diff --git a/flang/test/Lower/OpenMP/metadirective-user.f90 
b/flang/test/Lower/OpenMP/metadirective-user.f90
index cdfbddd4151a0..2f201cc10e982 100644
--- a/flang/test/Lower/OpenMP/metadirective-user.f90
+++ b/flang/test/Lower/OpenMP/metadirective-user.f90
@@ -51,6 +51,21 @@ subroutine test_condition_score()
 #endif
 end subroutine
 
+! All three score 5. The first is a strict subset of the third, which ties
+! with the second, so the second is selected.
+! CHECK-LABEL: func.func @_QPtest_condition_strict_subset_tie()
+! CHECK-NOT:     omp.taskyield
+! CHECK-NOT:     omp.barrier
+! CHECK:         omp.taskwait
+! CHECK-NOT:     omp.barrier
+! CHECK:         return
+subroutine test_condition_strict_subset_tie()
+  !$omp metadirective &
+  !$omp & when(user={condition(score(4): .true.)}: taskyield) &
+  !$omp & when(device={kind(host)}, implementation={vendor(score(3): llvm)}: 
taskwait) &
+  !$omp & when(user={condition(score(3): .true.)}, device={kind(host)}: 
barrier)
+end subroutine
+
 ! CHECK-LABEL: func.func @_QPtest_begin_condition_true()
 ! CHECK:         omp.parallel
 ! CHECK:           omp.terminator
diff --git a/llvm/lib/Frontend/OpenMP/OMPContext.cpp 
b/llvm/lib/Frontend/OpenMP/OMPContext.cpp
index f2cdd9bbaf5e4..ff346855b109e 100644
--- a/llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -398,8 +398,7 @@ int llvm::omp::getBestVariantMatchForContext(
     const SmallVectorImpl<VariantMatchInfo> &VMIs, const OMPContext &Ctx) {
 
   APInt BestScore(64, 0);
-  int BestVMIIdx = -1;
-  const VariantMatchInfo *BestVMI = nullptr;
+  SmallVector<unsigned, 4> BestVMIIdxs;
 
   for (unsigned u = 0, e = VMIs.size(); u < e; ++u) {
     const VariantMatchInfo &VMI = VMIs[u];
@@ -414,22 +413,21 @@ int llvm::omp::getBestVariantMatchForContext(
     APInt Score = getVariantMatchScore(VMI, Ctx, ConstructMatches);
     if (Score.ult(BestScore))
       continue;
-    // Equal score need subset checks.
-    if (Score.eq(BestScore)) {
-      // Strict subset are never best.
-      if (isStrictSubset(VMI, *BestVMI))
-        continue;
-      // Same score and the current best is no strict subset so we keep it.
-      if (!isStrictSubset(*BestVMI, VMI))
-        continue;
+    if (Score.ugt(BestScore)) {
+      BestScore = Score;
+      BestVMIIdxs.clear();
     }
-    // New best found.
-    BestVMI = &VMI;
-    BestVMIIdx = u;
-    BestScore = Score;
+    BestVMIIdxs.push_back(u);
   }
 
-  return BestVMIIdx;
+  // A strict subset of another tied variant is never best; otherwise the first
+  // one wins.
+  for (unsigned Idx : BestVMIIdxs)
+    if (llvm::none_of(BestVMIIdxs, [&](unsigned OtherIdx) {
+          return isStrictSubset(VMIs[Idx], VMIs[OtherIdx]);
+        }))
+      return Idx;
+  return -1;
 }
 
 TraitSet llvm::omp::getOpenMPContextTraitSetKind(StringRef S) {
diff --git a/llvm/unittests/Frontend/OpenMPContextTest.cpp 
b/llvm/unittests/Frontend/OpenMPContextTest.cpp
index f9683ae56e933..7f6ac92c6259c 100644
--- a/llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -317,4 +317,27 @@ TEST_F(OpenMPContextTest, ScoringSimple) {
   // TODO: Add scoring tests (via getBestVariantMatchForContext).
 }
 
+TEST_F(OpenMPContextTest, StrictSubsetOfLaterTiedVariant) {
+  OMPContext HostLinux(false, Triple("x86_64-unknown-linux"), Triple(), -1);
+  APInt Three(64, 3), Four(64, 4);
+  VariantMatchInfo Subset, Other, Superset;
+  Subset.addTrait(TraitProperty::user_condition_true, "");
+  Subset.addTrait(TraitProperty::implementation_vendor_llvm, "", &Four);
+  Other.addTrait(TraitProperty::device_kind_cpu, "");
+  Other.addTrait(TraitProperty::implementation_vendor_llvm, "", &Three);
+  Superset.addTrait(TraitProperty::user_condition_true, "");
+  Superset.addTrait(TraitProperty::device_kind_host, "");
+  Superset.addTrait(TraitProperty::implementation_vendor_llvm, "", &Three);
+
+  auto Best = [&](std::initializer_list<VariantMatchInfo> VMIs) {
+    return getBestVariantMatchForContext(SmallVector<VariantMatchInfo, 
3>(VMIs),
+                                         HostLinux);
+  };
+
+  // All three score 5. Subset is a strict subset of Superset, and the first of
+  // the other two wins.
+  EXPECT_EQ(Best({Subset, Other, Superset}), 1);
+  EXPECT_EQ(Best({Subset, Superset, Other}), 1);
+}
+
 } // namespace

_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to