[flang-commits] [flang] [llvm] [flang][OpenMP] Don't treat different runtime conditions as subsets (PR #233493)
via flang-commits
flang-commits at lists.llvm.org
Mon Oct 19 09:47:43 PDT 2026
https://github.com/chichunchen created https://github.com/llvm/llvm-project/pull/233493
When two METADIRECTIVE candidates have the same score and one selector is a strict subset of the other, the superset wins. Flang ranks every runtime condition with the same property, so isStrictSubset saw different conditions as the same:
!$omp metadirective &
!$omp& when(user={condition(a)}, &
!$omp& implementation={vendor(score(4): llvm)}: taskyield) &
!$omp& when(user={condition(b)}, device={kind(host)}, &
!$omp& implementation={vendor(score(3): llvm)}: taskwait)
Both candidates score 5, and `b` was tested first, so TASKWAIT ran when both conditions were true. Neither selector is a subset of the other, so clause order should decide.
This patch adds a UserCondition key to VariantMatchInfo, and isStrictSubset requires equal keys when the first selector has a condition. Flang sets the key from a non-constant condition, normalized as in the existing check for repeated conditions, so condition(a) and condition((a)) still form a strict subset. Constant conditions, including all of Clang's, leave the key empty and are still compared by truth value. A constant condition and a runtime condition are now different.
Fixes #233038.
Assisted-by: GitHub Copilot (Claude Opus 5.5)
>From 99121aa070c5b156aa904bcb96f11429ebb1c27e Mon Sep 17 00:00:00 2001
From: "Chi Chun, Chen" <chichun.chen at hpe.com>
Date: Sat, 17 Oct 2026 22:32:07 -0500
Subject: [PATCH] [flang][OpenMP] Don't treat different runtime conditions as
subsets
When two METADIRECTIVE candidates have the same score and one selector is
a strict subset of the other, the superset wins. Flang ranks every
runtime condition with the same property, so isStrictSubset saw
different conditions as the same:
!$omp metadirective &
!$omp& when(user={condition(a)}, &
!$omp& implementation={vendor(score(4): llvm)}: taskyield) &
!$omp& when(user={condition(b)}, device={kind(host)}, &
!$omp& implementation={vendor(score(3): llvm)}: taskwait)
Both candidates score 5, and `b` was tested first, so TASKWAIT ran when
both conditions were true. Neither selector is a subset of the other, so
clause order should decide.
This patch adds a UserCondition key to VariantMatchInfo, and
isStrictSubset requires equal keys when the first selector has a
condition. Flang sets the key from a non-constant condition, normalized
as in the existing check for repeated conditions, so condition(a) and
condition((a)) still form a strict subset. Constant conditions, including
all of Clang's, leave the key empty and are still compared by truth
value. A constant condition and a runtime condition are now different.
Fixes #233038.
Assisted-by: GitHub Copilot (Claude Opus 5.5)
---
flang/lib/Semantics/openmp-utils.cpp | 16 +++-
.../test/Lower/OpenMP/metadirective-user.f90 | 77 +++++++++++++++++++
.../include/llvm/Frontend/OpenMP/OMPContext.h | 4 +
llvm/lib/Frontend/OpenMP/OMPContext.cpp | 7 ++
llvm/unittests/Frontend/OpenMPContextTest.cpp | 37 +++++++++
5 files changed, 138 insertions(+), 3 deletions(-)
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 46fef418fe18b..92e8be5988f35 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -2473,6 +2473,8 @@ static void AppendConstructTraitsForDirective(
add(llvm::omp::TraitProperty::construct_dispatch_dispatch);
}
+static SomeExpr NormalizeMetadirectiveCondition(const SomeExpr &expr);
+
static void AddTraitPropertiesFromSelector(llvm::omp::TraitSet set,
const parser::OmpTraitSelector &selector, llvm::omp::VariantMatchInfo &vmi,
SemanticsContext &semaCtx,
@@ -2509,6 +2511,9 @@ static void AddTraitPropertiesFromSelector(llvm::omp::TraitSet set,
if (!dynamicCond) {
dynamicCond = DynamicUserCondition{scalarExpr, prop.source};
}
+ if (const auto *expr{GetExpr(semaCtx, *scalarExpr)}) {
+ vmi.UserCondition = NormalizeMetadirectiveCondition(*expr).AsFortran();
+ }
vmi.addTrait(set, llvm::omp::TraitProperty::user_condition_unknown,
"<condition>", scorePtr);
}
@@ -2780,6 +2785,12 @@ bool isRepeatableMetadirectiveCondition(const SomeExpr &expr) {
}
} // namespace
+static SomeExpr NormalizeMetadirectiveCondition(const SomeExpr &expr) {
+ MetadirectiveConditionNormalizer normalizer;
+ evaluate::rewrite::Mutator normalize{normalizer};
+ return normalize(expr);
+}
+
bool IsRepeatableMetadirectiveCondition(
const parser::ScalarExpr &condition, SemanticsContext &context) {
const SomeExpr *expr{GetExpr(context, condition)};
@@ -2795,9 +2806,8 @@ bool AreSameRepeatableMetadirectiveCondition(const parser::ScalarExpr &left,
!isRepeatableMetadirectiveCondition(*rightExpr))
return false;
- MetadirectiveConditionNormalizer normalizer;
- evaluate::rewrite::Mutator normalize{normalizer};
- return normalize(*leftExpr) == normalize(*rightExpr);
+ return NormalizeMetadirectiveCondition(*leftExpr) ==
+ NormalizeMetadirectiveCondition(*rightExpr);
}
llvm::SmallVector<unsigned, 4> GetMetadirectiveElsePathCandidates(
diff --git a/flang/test/Lower/OpenMP/metadirective-user.f90 b/flang/test/Lower/OpenMP/metadirective-user.f90
index cdfbddd4151a0..f84e2f8be5286 100644
--- a/flang/test/Lower/OpenMP/metadirective-user.f90
+++ b/flang/test/Lower/OpenMP/metadirective-user.f90
@@ -544,3 +544,80 @@ subroutine test_multi_dynamic_multi_static(a, b)
!$omp & default(nothing)
#endif
end subroutine
+
+! Both candidates score 5. The conditions differ, so neither selector is a
+! subset of the other and `a` is tested first.
+! CHECK-LABEL: func.func @_QPtest_distinct_conditions(
+! CHECK-SAME: %[[ARG0:[^,]*]]: !fir.ref<!fir.logical<4>>
+! CHECK-SAME: %[[ARG1:.*]]: !fir.ref<!fir.logical<4>>
+! CHECK: %[[DECLA:.*]]:2 = hlfir.declare %[[ARG0]]
+! CHECK: %[[DECLB:.*]]:2 = hlfir.declare %[[ARG1]]
+! CHECK: %[[LOADA:.*]] = fir.load %[[DECLA]]#0
+! CHECK: %[[CONDA:.*]] = fir.convert %[[LOADA]] : (!fir.logical<4>) -> i1
+! CHECK: fir.if %[[CONDA]] {
+! CHECK-NEXT: omp.taskyield
+! CHECK-NEXT: } else {
+! CHECK: %[[LOADB:.*]] = fir.load %[[DECLB]]#0
+! CHECK: %[[CONDB:.*]] = fir.convert %[[LOADB]] : (!fir.logical<4>) -> i1
+! CHECK: fir.if %[[CONDB]] {
+! CHECK-NEXT: omp.taskwait
+! CHECK: return
+subroutine test_distinct_conditions(a, b)
+ logical, intent(in) :: a, b
+ !$omp metadirective &
+ !$omp & when(user={condition(a)}, &
+ !$omp & implementation={vendor(score(4): llvm)}: taskyield) &
+ !$omp & when(user={condition(b)}, device={kind(host)}, &
+ !$omp & implementation={vendor(score(3): llvm)}: taskwait)
+end subroutine
+
+! With the same condition, the first selector is a strict subset of the second.
+! CHECK-LABEL: func.func @_QPtest_same_condition(
+! CHECK: fir.if
+! CHECK-NEXT: omp.taskwait
+! CHECK-NOT: omp.taskyield
+! CHECK: return
+subroutine test_same_condition(a)
+ logical, intent(in) :: a
+ !$omp metadirective &
+ !$omp & when(user={condition(a)}, &
+ !$omp & implementation={vendor(score(4): llvm)}: taskyield) &
+ !$omp & when(user={condition(a)}, device={kind(host)}, &
+ !$omp & implementation={vendor(score(3): llvm)}: taskwait)
+end subroutine
+
+! Outer parentheses don't change the condition.
+! CHECK-LABEL: func.func @_QPtest_parenthesized_condition(
+! CHECK: fir.if
+! CHECK-NEXT: omp.taskwait
+! CHECK-NOT: omp.taskyield
+! CHECK: return
+subroutine test_parenthesized_condition(a)
+ logical, intent(in) :: a
+ !$omp metadirective &
+ !$omp & when(user={condition((a))}, &
+ !$omp & implementation={vendor(score(4): llvm)}: taskyield) &
+ !$omp & when(user={condition(a)}, device={kind(host)}, &
+ !$omp & implementation={vendor(score(3): llvm)}: taskwait)
+end subroutine
+
+! Under match_none, both candidates score 3 and `a` is tested first.
+! CHECK-LABEL: func.func @_QPtest_distinct_conditions_match_none(
+! CHECK-SAME: %[[ARG0:[^,]*]]: !fir.ref<!fir.logical<4>>
+! CHECK: %[[DECLA:.*]]:2 = hlfir.declare %[[ARG0]]
+! CHECK: %[[LOADA:.*]] = fir.load %[[DECLA]]#0
+! CHECK: %[[CONDA:.*]] = fir.convert %[[LOADA]] : (!fir.logical<4>) -> i1
+! CHECK: %[[NOTA:.*]] = arith.xori %[[CONDA]]
+! CHECK: fir.if %[[NOTA]] {
+! CHECK-NEXT: omp.taskyield
+! CHECK-NEXT: } else {
+! CHECK: omp.taskwait
+! CHECK: return
+subroutine test_distinct_conditions_match_none(a, b)
+ logical, intent(in) :: a, b
+ !$omp metadirective &
+ !$omp & when(user={condition(score(2): a)}, &
+ !$omp & implementation={extension(match_none)}: taskyield) &
+ !$omp & when(user={condition(b)}, device={arch(nvptx)}, &
+ !$omp & implementation={extension(match_none)}: taskwait)
+end subroutine
diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPContext.h b/llvm/include/llvm/Frontend/OpenMP/OMPContext.h
index 7849d32665994..c8c69823d59ed 100644
--- a/llvm/include/llvm/Frontend/OpenMP/OMPContext.h
+++ b/llvm/include/llvm/Frontend/OpenMP/OMPContext.h
@@ -22,6 +22,8 @@
#include "llvm/Frontend/OpenMP/OMPConstants.h"
#include "llvm/Support/Compiler.h"
+#include <string>
+
namespace llvm {
class Triple;
namespace omp {
@@ -156,6 +158,8 @@ struct VariantMatchInfo {
SmallVector<StringRef, 8> ISATraits;
SmallVector<TraitProperty, 8> ConstructTraits;
SmallDenseMap<TraitProperty, APInt> ScoreMap;
+ /// Identifies a runtime user condition; empty for a constant condition.
+ std::string UserCondition;
};
/// The context for a source location is made up of active property traits,
diff --git a/llvm/lib/Frontend/OpenMP/OMPContext.cpp b/llvm/lib/Frontend/OpenMP/OMPContext.cpp
index f2cdd9bbaf5e4..dca1b6c63af39 100644
--- a/llvm/lib/Frontend/OpenMP/OMPContext.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPContext.cpp
@@ -172,6 +172,13 @@ static bool isStrictSubset(const VariantMatchInfo &VMI0,
for (unsigned Bit : VMI0.RequiredTraits.set_bits())
if (!VMI1.RequiredTraits.test(Bit))
return false;
+ // Different runtime conditions share a trait, so compare them as well.
+ for (TraitProperty Property :
+ {TraitProperty::user_condition_true, TraitProperty::user_condition_false,
+ TraitProperty::user_condition_unknown})
+ if (VMI0.RequiredTraits.test(unsigned(Property)) &&
+ VMI0.UserCondition != VMI1.UserCondition)
+ return false;
if (!isSubset<TraitProperty>(VMI0.ConstructTraits, VMI1.ConstructTraits))
return false;
return true;
diff --git a/llvm/unittests/Frontend/OpenMPContextTest.cpp b/llvm/unittests/Frontend/OpenMPContextTest.cpp
index f9683ae56e933..cdf39813fe0f1 100644
--- a/llvm/unittests/Frontend/OpenMPContextTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPContextTest.cpp
@@ -317,4 +317,41 @@ TEST_F(OpenMPContextTest, ScoringSimple) {
// TODO: Add scoring tests (via getBestVariantMatchForContext).
}
+TEST_F(OpenMPContextTest, UserConditionInStrictSubset) {
+ OMPContext Context(false, Triple("x86_64-unknown-linux"), Triple(), -1);
+ APInt Three(64, 3), Four(64, 4), Five(64, 5);
+ VariantMatchInfo A, B;
+ A.addTrait(TraitProperty::user_condition_true, "");
+ A.addTrait(TraitProperty::implementation_vendor_llvm, "", &Four);
+ B.addTrait(TraitProperty::user_condition_true, "");
+ B.addTrait(TraitProperty::implementation_vendor_llvm, "", &Three);
+ B.addTrait(TraitProperty::device_kind_host, "");
+ auto Best = [&](const VariantMatchInfo &First,
+ const VariantMatchInfo &Second) {
+ return getBestVariantMatchForContext(
+ SmallVector<VariantMatchInfo, 2>{First, Second}, Context);
+ };
+
+ // Both score 5. Constant conditions are compared by their trait alone.
+ EXPECT_EQ(Best(A, B), 1);
+ EXPECT_EQ(Best(B, A), 0);
+
+ // Different runtime conditions keep clause order.
+ A.UserCondition = "a";
+ B.UserCondition = "b";
+ EXPECT_EQ(Best(A, B), 0);
+ EXPECT_EQ(Best(B, A), 0);
+
+ // The same runtime condition still makes A a strict subset of B.
+ B.UserCondition = "a";
+ EXPECT_EQ(Best(A, B), 1);
+ EXPECT_EQ(Best(B, A), 0);
+
+ // A higher score wins regardless of the conditions.
+ B.UserCondition = "b";
+ A.ScoreMap[TraitProperty::implementation_vendor_llvm] = Five;
+ EXPECT_EQ(Best(A, B), 0);
+ EXPECT_EQ(Best(B, A), 1);
+}
+
} // namespace
More information about the flang-commits
mailing list