Skip to content

Commit 4a16816

Browse files
committed
[ValueTracking] Fix bug of using wrong condition for deducing KnownBits
Fixes llvm#124275 Bug was introduced by llvm#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`.
1 parent be7ef32 commit 4a16816

File tree

2 files changed

+19
-7
lines changed

2 files changed

+19
-7
lines changed

llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -593,11 +593,14 @@ static bool cmpExcludesZero(CmpInst::Predicate Pred, const Value *RHS) {
593593
}
594594

595595
static void breakSelfRecursivePHI(const Use *U, const PHINode *PHI,
596-
Value *&ValOut, Instruction *&CtxIOut) {
596+
Value *&ValOut, Instruction *&CtxIOut,
597+
const PHINode **PhiOut = nullptr) {
597598
ValOut = U->get();
598599
if (ValOut == PHI)
599600
return;
600601
CtxIOut = PHI->getIncomingBlock(*U)->getTerminator();
602+
if (PhiOut)
603+
*PhiOut = PHI;
601604
Value *V;
602605
// If the Use is a select of this phi, compute analysis on other arm to break
603606
// recursion.
@@ -610,11 +613,13 @@ static void breakSelfRecursivePHI(const Use *U, const PHINode *PHI,
610613
// incoming value to break recursion.
611614
// TODO: We could handle any number of incoming edges as long as we only have
612615
// two unique values.
613-
else if (auto *IncPhi = dyn_cast<PHINode>(ValOut);
616+
if (auto *IncPhi = dyn_cast<PHINode>(ValOut);
614617
IncPhi && IncPhi->getNumIncomingValues() == 2) {
615618
for (int Idx = 0; Idx < 2; ++Idx) {
616619
if (IncPhi->getIncomingValue(Idx) == PHI) {
617620
ValOut = IncPhi->getIncomingValue(1 - Idx);
621+
if (PhiOut)
622+
*PhiOut = IncPhi;
618623
CtxIOut = IncPhi->getIncomingBlock(1 - Idx)->getTerminator();
619624
break;
620625
}
@@ -1673,8 +1678,9 @@ static void computeKnownBitsFromOperator(const Operator *I,
16731678
Known.One.setAllBits();
16741679
for (const Use &U : P->operands()) {
16751680
Value *IncValue;
1681+
const PHINode *CxtPhi;
16761682
Instruction *CxtI;
1677-
breakSelfRecursivePHI(&U, P, IncValue, CxtI);
1683+
breakSelfRecursivePHI(&U, P, IncValue, CxtI, &CxtPhi);
16781684
// Skip direct self references.
16791685
if (IncValue == P)
16801686
continue;
@@ -1705,9 +1711,10 @@ static void computeKnownBitsFromOperator(const Operator *I,
17051711
m_Br(m_c_ICmp(Pred, m_Specific(IncValue), m_APInt(RHSC)),
17061712
m_BasicBlock(TrueSucc), m_BasicBlock(FalseSucc)))) {
17071713
// Check for cases of duplicate successors.
1708-
if ((TrueSucc == P->getParent()) != (FalseSucc == P->getParent())) {
1714+
if ((TrueSucc == CxtPhi->getParent()) !=
1715+
(FalseSucc == CxtPhi->getParent())) {
17091716
// If we're using the false successor, invert the predicate.
1710-
if (FalseSucc == P->getParent())
1717+
if (FalseSucc == CxtPhi->getParent())
17111718
Pred = CmpInst::getInversePredicate(Pred);
17121719
// Get the knownbits implied by the incoming phi condition.
17131720
auto CR = ConstantRange::makeExactICmpRegion(Pred, *RHSC);

llvm/test/Analysis/ValueTracking/phi-known-bits.ll

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1117,12 +1117,17 @@ cleanup:
11171117
define i32 @issue_124275_wrong_br_direction(i32 noundef %inp) {
11181118
; CHECK-LABEL: @issue_124275_wrong_br_direction(
11191119
; CHECK-NEXT: entry:
1120-
; CHECK-NEXT: [[CMP_NE_NOT:%.*]] = icmp eq i32 [[INP:%.*]], 1
1120+
; CHECK-NEXT: [[TMP0:%.*]] = xor i32 [[INP:%.*]], -2
1121+
; CHECK-NEXT: [[XOR_INP_NEG:%.*]] = add i32 [[TMP0]], 1
1122+
; CHECK-NEXT: [[CMP_NE_NOT:%.*]] = icmp eq i32 [[XOR_INP_NEG]], 0
11211123
; CHECK-NEXT: br i1 [[CMP_NE_NOT]], label [[B1:%.*]], label [[B0:%.*]]
11221124
; CHECK: B0:
1125+
; CHECK-NEXT: [[PHI_B0:%.*]] = phi i32 [ [[PHI_B1:%.*]], [[B1]] ], [ [[XOR_INP_NEG]], [[ENTRY:%.*]] ]
11231126
; CHECK-NEXT: br label [[B1]]
11241127
; CHECK: B1:
1125-
; CHECK-NEXT: br i1 true, label [[B0]], label [[END:%.*]]
1128+
; CHECK-NEXT: [[PHI_B1]] = phi i32 [ [[PHI_B0]], [[B0]] ], [ 0, [[ENTRY]] ]
1129+
; CHECK-NEXT: [[CMP_NE_B1_NOT:%.*]] = icmp eq i32 [[PHI_B1]], 0
1130+
; CHECK-NEXT: br i1 [[CMP_NE_B1_NOT]], label [[B0]], label [[END:%.*]]
11261131
; CHECK: end:
11271132
; CHECK-NEXT: ret i32 0
11281133
;

0 commit comments

Comments
 (0)