[llvm] c2fba02 - [ValueTracking] Fix bug of using wrong condition for deducing KnownBits (#124481)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 28 13:54:06 PST 2025
Author: goldsteinn
Date: 2025-01-28T15:54:00-06:00
New Revision: c2fba023475fddb893eac29dc9f34dfbdb221cd5
URL: https://github.com/llvm/llvm-project/commit/c2fba023475fddb893eac29dc9f34dfbdb221cd5
DIFF: https://github.com/llvm/llvm-project/commit/c2fba023475fddb893eac29dc9f34dfbdb221cd5.diff
LOG: [ValueTracking] Fix bug of using wrong condition for deducing KnownBits (#124481)
- **[ValueTracking] Add test for issue 124275**
- **[ValueTracking] Fix bug of using wrong condition for deducing
KnownBits**
Fixes https://github.com/llvm/llvm-project/issues/124275
Bug was introduced by https://github.com/llvm/llvm-project/pull/114689
Now that computeKnownBits supports breaking out of recursive Phi
nodes, `IncValue` can be an operand of a different Phi than `P`. This
breaks the previous assumptions we had when using the possibly
condition at `CxtI` to constrain `IncValue`.
Added:
Modified:
llvm/lib/Analysis/ValueTracking.cpp
llvm/test/Analysis/ValueTracking/phi-known-bits.ll
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index eba728c7c8c360..b63a0a07f7de29 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -593,11 +593,14 @@ static bool cmpExcludesZero(CmpInst::Predicate Pred, const Value *RHS) {
}
static void breakSelfRecursivePHI(const Use *U, const PHINode *PHI,
- Value *&ValOut, Instruction *&CtxIOut) {
+ Value *&ValOut, Instruction *&CtxIOut,
+ const PHINode **PhiOut = nullptr) {
ValOut = U->get();
if (ValOut == PHI)
return;
CtxIOut = PHI->getIncomingBlock(*U)->getTerminator();
+ if (PhiOut)
+ *PhiOut = PHI;
Value *V;
// If the Use is a select of this phi, compute analysis on other arm to break
// recursion.
@@ -610,11 +613,13 @@ static void breakSelfRecursivePHI(const Use *U, const PHINode *PHI,
// incoming value to break recursion.
// TODO: We could handle any number of incoming edges as long as we only have
// two unique values.
- else if (auto *IncPhi = dyn_cast<PHINode>(ValOut);
- IncPhi && IncPhi->getNumIncomingValues() == 2) {
+ if (auto *IncPhi = dyn_cast<PHINode>(ValOut);
+ IncPhi && IncPhi->getNumIncomingValues() == 2) {
for (int Idx = 0; Idx < 2; ++Idx) {
if (IncPhi->getIncomingValue(Idx) == PHI) {
ValOut = IncPhi->getIncomingValue(1 - Idx);
+ if (PhiOut)
+ *PhiOut = IncPhi;
CtxIOut = IncPhi->getIncomingBlock(1 - Idx)->getTerminator();
break;
}
@@ -1673,8 +1678,9 @@ static void computeKnownBitsFromOperator(const Operator *I,
Known.One.setAllBits();
for (const Use &U : P->operands()) {
Value *IncValue;
+ const PHINode *CxtPhi;
Instruction *CxtI;
- breakSelfRecursivePHI(&U, P, IncValue, CxtI);
+ breakSelfRecursivePHI(&U, P, IncValue, CxtI, &CxtPhi);
// Skip direct self references.
if (IncValue == P)
continue;
@@ -1705,9 +1711,10 @@ static void computeKnownBitsFromOperator(const Operator *I,
m_Br(m_c_ICmp(Pred, m_Specific(IncValue), m_APInt(RHSC)),
m_BasicBlock(TrueSucc), m_BasicBlock(FalseSucc)))) {
// Check for cases of duplicate successors.
- if ((TrueSucc == P->getParent()) != (FalseSucc == P->getParent())) {
+ if ((TrueSucc == CxtPhi->getParent()) !=
+ (FalseSucc == CxtPhi->getParent())) {
// If we're using the false successor, invert the predicate.
- if (FalseSucc == P->getParent())
+ if (FalseSucc == CxtPhi->getParent())
Pred = CmpInst::getInversePredicate(Pred);
// Get the knownbits implied by the incoming phi condition.
auto CR = ConstantRange::makeExactICmpRegion(Pred, *RHSC);
diff --git a/llvm/test/Analysis/ValueTracking/phi-known-bits.ll b/llvm/test/Analysis/ValueTracking/phi-known-bits.ll
index 84220832f068f1..436aadbc25de6f 100644
--- a/llvm/test/Analysis/ValueTracking/phi-known-bits.ll
+++ b/llvm/test/Analysis/ValueTracking/phi-known-bits.ll
@@ -1112,3 +1112,41 @@ cleanup:
%retval.0 = phi i1 [ %cmp, %if.then ], [ %tobool, %if.end4 ]
ret i1 %retval.0
}
+
+
+define i32 @issue_124275_wrong_br_direction(i32 noundef %inp) {
+; CHECK-LABEL: @issue_124275_wrong_br_direction(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[TMP0:%.*]] = xor i32 [[INP:%.*]], -2
+; CHECK-NEXT: [[XOR_INP_NEG:%.*]] = add i32 [[TMP0]], 1
+; CHECK-NEXT: [[CMP_NE_NOT:%.*]] = icmp eq i32 [[XOR_INP_NEG]], 0
+; CHECK-NEXT: br i1 [[CMP_NE_NOT]], label [[B1:%.*]], label [[B0:%.*]]
+; CHECK: B0:
+; CHECK-NEXT: [[PHI_B0:%.*]] = phi i32 [ [[PHI_B1:%.*]], [[B1]] ], [ [[XOR_INP_NEG]], [[ENTRY:%.*]] ]
+; CHECK-NEXT: br label [[B1]]
+; CHECK: B1:
+; CHECK-NEXT: [[PHI_B1]] = phi i32 [ [[PHI_B0]], [[B0]] ], [ 0, [[ENTRY]] ]
+; CHECK-NEXT: [[CMP_NE_B1_NOT:%.*]] = icmp eq i32 [[PHI_B1]], 0
+; CHECK-NEXT: br i1 [[CMP_NE_B1_NOT]], label [[B0]], label [[END:%.*]]
+; CHECK: end:
+; CHECK-NEXT: ret i32 0
+;
+entry:
+ %xor_inp = xor i32 %inp, 1
+ %sub = sub i32 0, %xor_inp
+ %cmp_ne = icmp ne i32 %sub, 0
+ br i1 %cmp_ne, label %B0, label %B1
+
+B0:
+ %phi_B0 = phi i32 [ %phi_B1, %B1 ], [ %sub, %entry ]
+ br label %B1
+
+B1:
+ %phi_B1 = phi i32 [ %phi_B0, %B0 ], [ 0, %entry ]
+ %cmp_ne_B1 = icmp ne i32 %phi_B1, 0
+ %cmp_eq_B1 = xor i1 %cmp_ne_B1, true
+ br i1 %cmp_eq_B1, label %B0, label %end
+
+end:
+ ret i32 0
+}
More information about the llvm-commits
mailing list