[llvm] [CVP] Guard against undef in common phi value transform (#198836) (PR #203238)
via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 11 03:40:30 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-analysis
Author: lijinpei-amd
<details>
<summary>Changes</summary>
simplifyCommonValuePhi() replaces a phi's constant incomings with a
non-constant "common value" when LVI reports the common value equals each
constant along the corresponding edge. But getConstantOnEdge() derives such
facts only up to undef:
merge(undef, constant) -> constant
So, for
bb3:
%u = phi i1 [ undef, %bb1 ], [ false, %dead2 ]
exit:
%p = phi i1 [ %u, %bb3 ], [ false, %bb4 ]
%p is replaced with %u, and when %dead2 is proven dead later, %u is replaced
with undef, creating a miscompile.
Fix by gating the transformation with %u known non-undef.
Fixes https://github.com/llvm/llvm-project/issues/198836
---
Full diff: https://github.com/llvm/llvm-project/pull/203238.diff
8 Files Affected:
- (modified) llvm/include/llvm/Analysis/LazyValueInfo.h (+7)
- (modified) llvm/include/llvm/Analysis/ValueLattice.h (+49-16)
- (modified) llvm/lib/Analysis/LazyValueInfo.cpp (+17)
- (modified) llvm/lib/Analysis/ValueLattice.cpp (+2)
- (modified) llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp (+2-1)
- (modified) llvm/lib/Transforms/Utils/SCCPSolver.cpp (+5-6)
- (modified) llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll (+45)
- (modified) llvm/unittests/Analysis/ValueLatticeTest.cpp (+37)
``````````diff
diff --git a/llvm/include/llvm/Analysis/LazyValueInfo.h b/llvm/include/llvm/Analysis/LazyValueInfo.h
index 7b0e6c07c00f4..a879c43295037 100644
--- a/llvm/include/llvm/Analysis/LazyValueInfo.h
+++ b/llvm/include/llvm/Analysis/LazyValueInfo.h
@@ -105,6 +105,13 @@ namespace llvm {
BasicBlock *ToBB,
Instruction *CxtI = nullptr);
+ /// Like getConstantOnEdge, but rejects results that may be undef on the
+ /// edge.
+ LLVM_ABI Constant *
+ getKnownNonUndefConstantOnEdge(Value *V, BasicBlock *FromBB,
+ BasicBlock *ToBB,
+ Instruction *CxtI = nullptr);
+
/// Return the ConstantRage constraint that is known to hold for the
/// specified value on the specified edge. This may be only be called
/// on integer-typed Values.
diff --git a/llvm/include/llvm/Analysis/ValueLattice.h b/llvm/include/llvm/Analysis/ValueLattice.h
index 262ff58f07dfd..b58e5ec9627b8 100644
--- a/llvm/include/llvm/Analysis/ValueLattice.h
+++ b/llvm/include/llvm/Analysis/ValueLattice.h
@@ -44,12 +44,20 @@ class ValueLatticeElement {
/// This Value has a specific constant value. The constant cannot be undef.
/// (For constant integers, constantrange is used instead. Integer typed
- /// constantexprs can appear as constant.) Note that the constant state
- /// can be reached by merging undef & constant states.
+ /// constantexprs can appear as constant.)
/// Transition allowed to the following states:
+ /// constant_including_undef
/// overdefined
constant,
+ /// This Value has a specific constant value, but may also be undef. Merging
+ /// it with a different constant value results in overdefined. (For constant
+ /// integers, constantrange_including_undef is used instead.) This state is
+ /// reached by merging undef & constant states.
+ /// Transition allowed to the following states:
+ /// overdefined
+ constant_including_undef,
+
/// This Value is known to not have the specified value. (For constant
/// integers, constantrange is used instead. As above, integer typed
/// constantexprs can appear here.)
@@ -95,6 +103,7 @@ class ValueLatticeElement {
case unknown:
case undef:
case constant:
+ case constant_including_undef:
case notconstant:
break;
case constantrange_including_undef:
@@ -156,6 +165,7 @@ class ValueLatticeElement {
NumRangeExtensions = Other.NumRangeExtensions;
break;
case constant:
+ case constant_including_undef:
case notconstant:
ConstVal = Other.ConstVal;
break;
@@ -175,6 +185,7 @@ class ValueLatticeElement {
NumRangeExtensions = Other.NumRangeExtensions;
break;
case constant:
+ case constant_including_undef:
case notconstant:
ConstVal = Other.ConstVal;
break;
@@ -235,7 +246,12 @@ class ValueLatticeElement {
bool isUndef() const { return Tag == undef; }
bool isUnknown() const { return Tag == unknown; }
bool isUnknownOrUndef() const { return Tag == unknown || Tag == undef; }
- bool isConstant() const { return Tag == constant; }
+ bool isConstant(bool UndefAllowed = true) const {
+ return Tag == constant || (Tag == constant_including_undef && UndefAllowed);
+ }
+ bool isConstantIncludingUndef() const {
+ return Tag == constant_including_undef;
+ }
bool isNotConstant() const { return Tag == notconstant; }
bool isConstantRangeIncludingUndef() const {
return Tag == constantrange_including_undef;
@@ -250,8 +266,9 @@ class ValueLatticeElement {
}
bool isOverdefined() const { return Tag == overdefined; }
- Constant *getConstant() const {
- assert(isConstant() && "Cannot get the constant of a non-constant!");
+ Constant *getConstant(bool UndefAllowed = true) const {
+ assert(isConstant(UndefAllowed) &&
+ "Cannot get the constant of a non-constant!");
return ConstVal;
}
@@ -282,7 +299,7 @@ class ValueLatticeElement {
ConstantRange asConstantRange(unsigned BW, bool UndefAllowed = false) const {
if (isConstantRange(UndefAllowed))
return getConstantRange();
- if (isConstant())
+ if (isConstant(UndefAllowed))
return getConstant()->toConstantRange();
if (isUnknown())
return ConstantRange::getEmpty(BW);
@@ -315,18 +332,27 @@ class ValueLatticeElement {
if (isa<UndefValue>(V))
return markUndef();
- if (isConstant()) {
- assert(getConstant() == V && "Marking constant with different value");
- return false;
- }
-
if (ConstantInt *CI = dyn_cast<ConstantInt>(V))
return markConstantRange(
ConstantRange(CI->getValue()),
MergeOptions().setMayIncludeUndef(MayIncludeUndef));
+ // Non-integer constant. Mirror constantrange /
+ // constantrange_including_undef by tracking whether the constant may also
+ // be undef.
+ ValueLatticeElementTy NewTag =
+ (isUndef() || isConstantIncludingUndef() || MayIncludeUndef)
+ ? constant_including_undef
+ : constant;
+ if (isConstant()) {
+ assert(getConstant() == V && "Marking constant with different value");
+ bool Changed = Tag != NewTag;
+ Tag = NewTag;
+ return Changed;
+ }
+
assert(isUnknown() || isUndef());
- Tag = constant;
+ Tag = NewTag;
ConstVal = V;
return true;
}
@@ -425,10 +451,15 @@ class ValueLatticeElement {
}
if (isConstant()) {
- if (RHS.isConstant() && getConstant() == RHS.getConstant())
+ if (RHS.isConstant() && getConstant() == RHS.getConstant()) {
+ // Same constant value; the result may include undef if either input
+ // may.
+ if (RHS.isConstantIncludingUndef())
+ return markConstant(getConstant(), /*MayIncludeUndef=*/true);
return false;
+ }
if (RHS.isUndef())
- return false;
+ return markConstant(getConstant(), /*MayIncludeUndef=*/true);
// If the constant is a vector of integers, try to treat it as a range.
if (getConstant()->getType()->isVectorTy() &&
getConstant()->getType()->getScalarType()->isIntegerTy()) {
@@ -437,7 +468,8 @@ class ValueLatticeElement {
RHS.asConstantRange(L.getBitWidth(), /*UndefAllowed=*/true));
return markConstantRange(
std::move(NewR),
- Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef()));
+ Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef() ||
+ isConstantIncludingUndef()));
}
markOverdefined();
return true;
@@ -462,7 +494,8 @@ class ValueLatticeElement {
RHS.asConstantRange(L.getBitWidth(), /*UndefAllowed=*/true));
return markConstantRange(
std::move(NewR),
- Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef()));
+ Opts.setMayIncludeUndef(RHS.isConstantRangeIncludingUndef() ||
+ RHS.isConstantIncludingUndef()));
}
// Compares this symbolic value with Other using Pred and returns either
diff --git a/llvm/lib/Analysis/LazyValueInfo.cpp b/llvm/lib/Analysis/LazyValueInfo.cpp
index b052352b51eec..edf7a6cea9bec 100644
--- a/llvm/lib/Analysis/LazyValueInfo.cpp
+++ b/llvm/lib/Analysis/LazyValueInfo.cpp
@@ -2044,6 +2044,23 @@ Constant *LazyValueInfo::getConstantOnEdge(Value *V, BasicBlock *FromBB,
return nullptr;
}
+Constant *LazyValueInfo::getKnownNonUndefConstantOnEdge(Value *V,
+ BasicBlock *FromBB,
+ BasicBlock *ToBB,
+ Instruction *CxtI) {
+ ValueLatticeElement Result =
+ getOrCreateImpl().getValueOnEdge(V, FromBB, ToBB, CxtI);
+
+ if (Result.isConstant(/*UndefAllowed=*/false))
+ return Result.getConstant();
+ if (Result.isConstantRange() && !Result.isConstantRangeIncludingUndef()) {
+ const ConstantRange &CR = Result.getConstantRange();
+ if (const APInt *SingleVal = CR.getSingleElement())
+ return ConstantInt::get(V->getType(), *SingleVal);
+ }
+ return nullptr;
+}
+
ConstantRange LazyValueInfo::getConstantRangeOnEdge(Value *V,
BasicBlock *FromBB,
BasicBlock *ToBB,
diff --git a/llvm/lib/Analysis/ValueLattice.cpp b/llvm/lib/Analysis/ValueLattice.cpp
index 03810f1c554e5..dfefd12d04ee8 100644
--- a/llvm/lib/Analysis/ValueLattice.cpp
+++ b/llvm/lib/Analysis/ValueLattice.cpp
@@ -128,6 +128,8 @@ raw_ostream &operator<<(raw_ostream &OS, const ValueLatticeElement &Val) {
if (Val.isConstantRange())
return OS << "constantrange<" << Val.getConstantRange().getLower() << ", "
<< Val.getConstantRange().getUpper() << ">";
+ if (Val.isConstantIncludingUndef())
+ return OS << "constant incl. undef <" << *Val.getConstant() << ">";
return OS << "constant<" << *Val.getConstant() << ">";
}
} // end namespace llvm
diff --git a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
index ff0b70b51e5f7..8d8f647a27a22 100644
--- a/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
+++ b/llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
@@ -187,7 +187,8 @@ static bool simplifyCommonValuePhi(PHINode *P, LazyValueInfo *LVI,
for (auto &IncomingConstant : IncomingConstants) {
Constant *C = IncomingConstant.first;
BasicBlock *IncomingBB = P->getIncomingBlock(IncomingConstant.second);
- if (C != LVI->getConstantOnEdge(CommonValue, IncomingBB, ToBB, P))
+ if (C !=
+ LVI->getKnownNonUndefConstantOnEdge(CommonValue, IncomingBB, ToBB, P))
return false;
}
diff --git a/llvm/lib/Transforms/Utils/SCCPSolver.cpp b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
index 4cae040f96fc6..65605faa92ba2 100644
--- a/llvm/lib/Transforms/Utils/SCCPSolver.cpp
+++ b/llvm/lib/Transforms/Utils/SCCPSolver.cpp
@@ -1734,13 +1734,12 @@ void SCCPInstVisitor::visitBinaryOperator(Instruction &I) {
Value *R = simplifyBinOp(I.getOpcode(), V1, V2, SimplifyQuery(DL, &I));
auto *C = dyn_cast_or_null<Constant>(R);
if (C) {
- // Conservatively assume that the result may be based on operands that may
- // be undef. Note that we use mergeInValue to combine the constant with
- // the existing lattice value for I, as different constants might be found
- // after one of the operands go to overdefined, e.g. due to one operand
- // being a special floating value.
+ bool MayIncludeUndef = V1State.isConstantRangeIncludingUndef() ||
+ V1State.isConstantIncludingUndef() ||
+ V2State.isConstantRangeIncludingUndef() ||
+ V2State.isConstantIncludingUndef();
ValueLatticeElement NewV;
- NewV.markConstant(C, /*MayIncludeUndef=*/true);
+ NewV.markConstant(C, MayIncludeUndef);
return (void)mergeInValue(ValueState[&I], &I, NewV);
}
}
diff --git a/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll b/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
index 7e712947e37ad..64c9d40803502 100644
--- a/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
+++ b/llvm/test/Transforms/CorrelatedValuePropagation/phi-common-val.ll
@@ -216,3 +216,48 @@ join2:
%phi = phi i8 [ 0, %join1 ], [ %y, %else2 ]
ret i8 %phi
}
+
+; original: https://github.com/llvm/llvm-project/issues/198836
+define void @common_value_undef(i32 noundef %a) {
+; CHECK-LABEL: @common_value_undef(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[HEADER:%.*]]
+; CHECK: header:
+; CHECK-NEXT: [[U:%.*]] = phi i1 [ undef, [[ENTRY:%.*]] ], [ false, [[BB:%.*]] ]
+; CHECK-NEXT: [[CMP0:%.*]] = icmp eq i32 [[A:%.*]], 0
+; CHECK-NEXT: br i1 [[CMP0]], label [[BB]], label [[EXIT:%.*]]
+; CHECK: bb:
+; CHECK-NEXT: call void @llvm.assume(i1 false)
+; CHECK-NEXT: br label [[HEADER]]
+; CHECK: dead:
+; CHECK-NEXT: br label [[EXIT]]
+; CHECK: exit:
+; CHECK-NEXT: [[P:%.*]] = phi i1 [ [[U]], [[DEAD:%.*]] ], [ false, [[HEADER]] ]
+; CHECK-NEXT: [[NOT:%.*]] = xor i1 [[P]], true
+; CHECK-NEXT: call void @llvm.assume(i1 [[NOT]])
+; CHECK-NEXT: ret void
+;
+entry:
+ br label %header
+
+header:
+ %u = phi i1 [ undef, %entry ], [ %cmp1, %bb ]
+ %cmp0 = icmp eq i32 %a, 0
+ br i1 %cmp0, label %bb, label %exit
+
+bb: ; reached only when %a == 0; UB via assume(false)
+ %cmp1 = icmp eq i32 %a, 1
+ call void @llvm.assume(i1 false)
+ br label %header
+
+dead: ; unreachable; feeds %u into %p
+ br label %exit
+
+exit:
+ %p = phi i1 [ %u, %dead ], [ false, %header ]
+ %not = xor i1 %p, true
+ call void @llvm.assume(i1 %not)
+ ret void
+}
+
+declare void @llvm.assume(i1)
diff --git a/llvm/unittests/Analysis/ValueLatticeTest.cpp b/llvm/unittests/Analysis/ValueLatticeTest.cpp
index 3570bd0f3f498..0cfe87634ea48 100644
--- a/llvm/unittests/Analysis/ValueLatticeTest.cpp
+++ b/llvm/unittests/Analysis/ValueLatticeTest.cpp
@@ -99,6 +99,43 @@ TEST_F(ValueLatticeTest, MergeIn) {
EXPECT_TRUE(LV1.isOverdefined());
}
+TEST_F(ValueLatticeTest, MergeNonIntegerConstantWithUndef) {
+ // Non-integer constants use the constant / constant_including_undef states
+ // (integers use constantrange* instead). Merging undef with such a constant
+ // must remember that the value may be undef, so it cannot be substituted for
+ // a well-defined value.
+ auto FloatTy = Type::getFloatTy(Context);
+ auto *C1 = ConstantFP::get(FloatTy, 1.1);
+
+ // undef merged with a constant -> constant_including_undef.
+ ValueLatticeElement LV1;
+ EXPECT_TRUE(LV1.markUndef());
+ EXPECT_TRUE(LV1.mergeIn(ValueLatticeElement::get(C1)));
+ EXPECT_TRUE(LV1.isConstantIncludingUndef());
+ EXPECT_TRUE(LV1.isConstant(/*UndefAllowed=*/true));
+ EXPECT_FALSE(LV1.isConstant(/*UndefAllowed=*/false));
+ EXPECT_EQ(LV1.getConstant(), C1);
+
+ // The symmetric merge order yields the same result.
+ auto LV2 = ValueLatticeElement::get(C1);
+ EXPECT_FALSE(LV2.isConstantIncludingUndef());
+ EXPECT_TRUE(LV2.isConstant(/*UndefAllowed=*/false));
+ ValueLatticeElement Undef;
+ EXPECT_TRUE(Undef.markUndef());
+ EXPECT_TRUE(LV2.mergeIn(Undef));
+ EXPECT_TRUE(LV2.isConstantIncludingUndef());
+ EXPECT_FALSE(LV2.isConstant(/*UndefAllowed=*/false));
+
+ // Merging the same constant value (without undef) keeps the undef flag.
+ EXPECT_FALSE(LV2.mergeIn(ValueLatticeElement::get(C1)));
+ EXPECT_TRUE(LV2.isConstantIncludingUndef());
+
+ // Merging a different constant goes to overdefined.
+ auto *C2 = ConstantFP::get(FloatTy, 2.2);
+ EXPECT_TRUE(LV2.mergeIn(ValueLatticeElement::get(C2)));
+ EXPECT_TRUE(LV2.isOverdefined());
+}
+
TEST_F(ValueLatticeTest, getCompareIntegers) {
auto *I32Ty = IntegerType::get(Context, 32);
auto *I1Ty = IntegerType::get(Context, 1);
``````````
</details>
https://github.com/llvm/llvm-project/pull/203238
More information about the llvm-commits
mailing list