[llvm] c8460a9 - [ValueTracking] Clarify KnownBits recurrence code (#222266)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 9 06:47:37 PDT 2026
Author: Nikita Popov
Date: 2026-09-09T15:47:31+02:00
New Revision: c8460a91f629145012bd1adfc05b08aedfbc7b4d
URL: https://github.com/llvm/llvm-project/commit/c8460a91f629145012bd1adfc05b08aedfbc7b4d
DIFF: https://github.com/llvm/llvm-project/commit/c8460a91f629145012bd1adfc05b08aedfbc7b4d.diff
LOG: [ValueTracking] Clarify KnownBits recurrence code (#222266)
While reviewing a related PR, I found the R/L variable naming here very
confusing. Use Start and Step instead, matching the parameter names of
matchSimpleRecurrence().
Also clarify why the context adjustment for the step is necessary, and
add a test that would miscompile if it isn't performed.
Added:
Modified:
llvm/lib/Analysis/ValueTracking.cpp
llvm/test/Transforms/InstCombine/recurrence.ll
Removed:
################################################################################
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 950b125028b99..c881ecc8d2d35 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -1819,8 +1819,9 @@ static void computeKnownBitsFromOperator(const Operator *I,
case Instruction::PHI: {
const PHINode *P = cast<PHINode>(I);
BinaryOperator *BO = nullptr;
- Value *R = nullptr, *L = nullptr;
- if (matchSimpleRecurrence(P, BO, R, L)) {
+ Value *Start = nullptr, *Step = nullptr;
+ KnownBits &KnownStart = Known2;
+ if (matchSimpleRecurrence(P, BO, Start, Step)) {
// Handle the case of a simple two-predecessor recurrence PHI.
// There's a lot more that could theoretically be done here, but
// this is sufficient to catch some interesting cases.
@@ -1853,23 +1854,23 @@ static void computeKnownBitsFromOperator(const Operator *I,
// add sufficient tests to cover.
SimplifyQuery RecQ = Q.getWithoutCondContext();
RecQ.CxtI = P;
- computeKnownBits(R, DemandedElts, Known2, RecQ, Depth + 1);
+ computeKnownBits(Start, DemandedElts, KnownStart, RecQ, Depth + 1);
switch (Opcode) {
case Instruction::Shl:
// A shl recurrence will only increase the tailing zeros
- Known.Zero.setLowBits(Known2.countMinTrailingZeros());
+ Known.Zero.setLowBits(KnownStart.countMinTrailingZeros());
break;
case Instruction::LShr:
case Instruction::UDiv:
case Instruction::URem:
// lshr, udiv, and urem recurrences will preserve the leading zeros of
// the start value.
- Known.Zero.setHighBits(Known2.countMinLeadingZeros());
+ Known.Zero.setHighBits(KnownStart.countMinLeadingZeros());
break;
case Instruction::AShr:
// An ashr recurrence will extend the initial sign bit
- Known.Zero.setHighBits(Known2.countMinLeadingZeros());
- Known.One.setHighBits(Known2.countMinLeadingOnes());
+ Known.Zero.setHighBits(KnownStart.countMinLeadingZeros());
+ Known.One.setHighBits(KnownStart.countMinLeadingOnes());
break;
}
break;
@@ -1889,22 +1890,25 @@ static void computeKnownBitsFromOperator(const Operator *I,
// D69571).
SimplifyQuery RecQ = Q.getWithoutCondContext();
- unsigned OpNum = P->getOperand(0) == R ? 0 : 1;
- Instruction *RInst = P->getIncomingBlock(OpNum)->getTerminator();
- Instruction *LInst = P->getIncomingBlock(1 - OpNum)->getTerminator();
+ unsigned OpNum = P->getOperand(0) == Start ? 0 : 1;
+ Instruction *StartTerm = P->getIncomingBlock(OpNum)->getTerminator();
+ Instruction *LatchTerm =
+ P->getIncomingBlock(1 - OpNum)->getTerminator();
- // Ok, we have a PHI of the form L op= R. Check for low
+ // Ok, we have a recurrence of the form {Start,op,Step}. Check for low
// zero bits.
- RecQ.CxtI = RInst;
- computeKnownBits(R, DemandedElts, Known2, RecQ, Depth + 1);
+ RecQ.CxtI = StartTerm;
+ computeKnownBits(Start, DemandedElts, KnownStart, RecQ, Depth + 1);
- // We need to take the minimum number of known bits
- KnownBits Known3(BitWidth);
- RecQ.CxtI = LInst;
- computeKnownBits(L, DemandedElts, Known3, RecQ, Depth + 1);
+ // We need to take the minimum number of known bits.
+ // The step may be loop-variant, so make sure we don't make use of
+ // any conditions that only hold on the last iteration.
+ KnownBits KnownStep(BitWidth);
+ RecQ.CxtI = LatchTerm;
+ computeKnownBits(Step, DemandedElts, KnownStep, RecQ, Depth + 1);
- Known.Zero.setLowBits(std::min(Known2.countMinTrailingZeros(),
- Known3.countMinTrailingZeros()));
+ Known.Zero.setLowBits(std::min(KnownStart.countMinTrailingZeros(),
+ KnownStep.countMinTrailingZeros()));
auto *OverflowOp = dyn_cast<OverflowingBinaryOperator>(BO);
if (!OverflowOp || !Q.IIQ.hasNoSignedWrap(OverflowOp))
@@ -1921,9 +1925,9 @@ static void computeKnownBitsFromOperator(const Operator *I,
// (add non-negative, non-negative) --> non-negative
// (add negative, negative) --> negative
case Instruction::Add: {
- if (Known2.isNonNegative() && Known3.isNonNegative())
+ if (KnownStart.isNonNegative() && KnownStep.isNonNegative())
Known.makeNonNegative();
- else if (Known2.isNegative() && Known3.isNegative())
+ else if (KnownStart.isNegative() && KnownStep.isNegative())
Known.makeNegative();
break;
}
@@ -1933,16 +1937,16 @@ static void computeKnownBitsFromOperator(const Operator *I,
case Instruction::Sub: {
if (BO->getOperand(0) != I)
break;
- if (Known2.isNonNegative() && Known3.isNegative())
+ if (KnownStart.isNonNegative() && KnownStep.isNegative())
Known.makeNonNegative();
- else if (Known2.isNegative() && Known3.isNonNegative())
+ else if (KnownStart.isNegative() && KnownStep.isNonNegative())
Known.makeNegative();
break;
}
// (mul nsw non-negative, non-negative) --> non-negative
case Instruction::Mul:
- if (Known2.isNonNegative() && Known3.isNonNegative())
+ if (KnownStart.isNonNegative() && KnownStep.isNonNegative())
Known.makeNonNegative();
break;
diff --git a/llvm/test/Transforms/InstCombine/recurrence.ll b/llvm/test/Transforms/InstCombine/recurrence.ll
index 6207009b531d5..456b20a187519 100644
--- a/llvm/test/Transforms/InstCombine/recurrence.ll
+++ b/llvm/test/Transforms/InstCombine/recurrence.ll
@@ -162,4 +162,40 @@ loop: ; preds = %loop, %entry
br label %loop
}
+declare i32 @get_step()
+
+define i1 @test_loop_variant_step_with_condition() {
+; CHECK-LABEL: @test_loop_variant_step_with_condition(
+; CHECK-NEXT: entry:
+; CHECK-NEXT: br label [[LOOP:%.*]]
+; CHECK: loop:
+; CHECK-NEXT: [[IV:%.*]] = phi i32 [ 0, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LATCH:%.*]] ]
+; CHECK-NEXT: [[STEP:%.*]] = call i32 @get_step()
+; CHECK-NEXT: [[C:%.*]] = icmp sgt i32 [[STEP]], -1
+; CHECK-NEXT: br i1 [[C]], label [[EXIT:%.*]], label [[LATCH]]
+; CHECK: latch:
+; CHECK-NEXT: [[IV_NEXT]] = add nsw i32 [[IV]], [[STEP]]
+; CHECK-NEXT: br label [[LOOP]]
+; CHECK: exit:
+; CHECK-NEXT: [[RESULT:%.*]] = icmp sgt i32 [[IV]], -1
+; CHECK-NEXT: ret i1 [[RESULT]]
+;
+entry:
+ br label %loop
+
+loop:
+ %iv = phi i32 [ 0, %entry ], [ %iv.next, %latch ]
+ %step = call i32 @get_step()
+ %iv.next = add nsw i32 %iv, %step
+ %c = icmp sge i32 %step, 0
+ br i1 %c, label %exit, label %latch
+
+latch:
+ br label %loop
+
+exit:
+ %result = icmp sge i32 %iv, 0
+ ret i1 %result
+}
+
declare void @use(i64)
More information about the llvm-commits
mailing list