[flang-commits] [clang] [flang] [llvm] [clang][flang][OpenMP] Fix context selector matching and scoring (PR #224431)

[via flang-commits]

================
@@ -157,24 +155,46 @@ template <typename T> static bool isSubset(ArrayRef<T> C0, ArrayRef<T> C1) {
       ++It1;
       continue;
     }
-    ++It0;
+    ++It1;
   }
   return true;
 }
 
 static bool isStrictSubset(const VariantMatchInfo &VMI0,
                            const VariantMatchInfo &VMI1) {
-  // If all required traits are a strict subset and the ordered vectors storing
-  // the construct traits, we say it is a strict subset. Note that the latter
-  // relation is not required to be strict.
-  if (VMI0.RequiredTraits.count() >= VMI1.RequiredTraits.count())
+  // kind(any) is equivalent to omitting the kind selector.
+  BitVector Traits0 = VMI0.RequiredTraits, Traits1 = VMI1.RequiredTraits;
+  for (TraitProperty Property : {TraitProperty::device_kind_any,
+                                 TraitProperty::target_device_kind_any}) {
+    Traits0.reset(unsigned(Property));
+    Traits1.reset(unsigned(Property));
+  }
+  size_t TraitCount0 = Traits0.count() + VMI0.UnknownTraits.size();
+  size_t TraitCount1 = Traits1.count() + VMI1.UnknownTraits.size();
+  if (TraitCount0 > TraitCount1)
     return false;
-  for (unsigned Bit : VMI0.RequiredTraits.set_bits())
-    if (!VMI1.RequiredTraits.test(Bit))
+  for (unsigned Bit : Traits0.set_bits())
+    if (!Traits1.test(Bit))
+      return false;
+  for (const auto &Trait : VMI0.UnknownTraits)
+    if (!llvm::is_contained(VMI1.UnknownTraits, Trait))
       return false;
-  if (!isSubset<TraitProperty>(VMI0.ConstructTraits, VMI1.ConstructTraits))
+  for (const auto &Trait : VMI0.ISATraits)
+    if (!llvm::is_contained(VMI1.ISATraits, Trait))
+      return false;
+  bool HasAdditionalISATrait =
+      llvm::any_of(VMI1.ISATraits, [&](const auto &Trait) {
+        return !llvm::is_contained(VMI0.ISATraits, Trait);
+      });
+  if (!VMI0.UserCondition.empty() && VMI0.UserCondition != VMI1.UserCondition)
----------------
MattPD wrote:

The Clang producer `OMPTraitInfo::getAsVariantMatchInfo` passes the literal `"<condition>"` as the raw string for every folded condition, so every Clang candidate with a `condition` selector receives the same `UserCondition` identity. Under the new rule that zeroes a candidate whose selector set is a strict subset of another compatible candidate's, two candidates with distinct conditions are compared as though they shared one condition. The smaller selector set then scores zero even when its explicit score is higher. `DistinctUserConditionsAreNotSubsets` asserts the intended semantics, but it constructs the `VariantMatchInfo` directly and bypasses the Clang producer.

You can reproduce this by saving the following to `cond.c` and running `clang -fopenmp -fopenmp-version=52 -S -emit-llvm -o - cond.c`:

```c
#pragma omp begin declare target
void high_variant(void);
void low_variant(void);
#pragma omp declare variant(high_variant) match(implementation={vendor(score(100): llvm)}, user={condition(1)})
#pragma omp declare variant(low_variant) match(implementation={vendor(score(1): llvm)}, device={kind(cpu)}, user={condition(2)})
void base(void);
#pragma omp end declare target
void test(void) { base(); }
```

At the merge base (6eba5341) `test` calls `high_variant` (101 against 3). With this PR it calls `low_variant`. With `condition(1)` in both candidates, `low_variant` is the intended result, since the first candidate's selector set is then a genuine strict subset. Could the Clang producer carry each condition expression's identity into `VariantMatchInfo`, with C and C++ tests for a distinct pair and an identical pair?

https://github.com/llvm/llvm-project/pull/224431