================
@@ -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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to