[llvm] [ValueTracking] Clarify KnownBits recurrence code (PR #222266)

[via llvm-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-llvm-analysis

Author: Nikita Popov (nikic)

<details>
<summary>Changes</summary>

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().

While doing that, I also found some context instruction adjustment that does not make sense to me. We need to adjust the context for the start value to match the start terminator, as it comes from the phi node. But the step operand has no relation to the phi, so I don't think there is any reason why this needs to adjust context to the loop latch terminator.

---
Full diff: https://github.com/llvm/llvm-project/pull/222266.diff


1 Files Affected:

- (modified) llvm/lib/Analysis/ValueTracking.cpp (+16-18) 


``````````diff
diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 950b125028b99..422a7d60b615b 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -1819,8 +1819,8 @@ 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;
+    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,7 +1853,7 @@ 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, Known2, RecQ, Depth + 1);
         switch (Opcode) {
         case Instruction::Shl:
           // A shl recurrence will only increase the tailing zeros
@@ -1889,22 +1889,20 @@ 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();
 
-        // 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, Known2, 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);
+        KnownBits KnownStep(BitWidth);
+        computeKnownBits(Step, DemandedElts, KnownStep, Q, Depth + 1);
 
         Known.Zero.setLowBits(std::min(Known2.countMinTrailingZeros(),
-                                       Known3.countMinTrailingZeros()));
+                                       KnownStep.countMinTrailingZeros()));
 
         auto *OverflowOp = dyn_cast<OverflowingBinaryOperator>(BO);
         if (!OverflowOp || !Q.IIQ.hasNoSignedWrap(OverflowOp))
@@ -1921,9 +1919,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 (Known2.isNonNegative() && KnownStep.isNonNegative())
             Known.makeNonNegative();
-          else if (Known2.isNegative() && Known3.isNegative())
+          else if (Known2.isNegative() && KnownStep.isNegative())
             Known.makeNegative();
           break;
         }
@@ -1933,16 +1931,16 @@ static void computeKnownBitsFromOperator(const Operator *I,
         case Instruction::Sub: {
           if (BO->getOperand(0) != I)
             break;
-          if (Known2.isNonNegative() && Known3.isNegative())
+          if (Known2.isNonNegative() && KnownStep.isNegative())
             Known.makeNonNegative();
-          else if (Known2.isNegative() && Known3.isNonNegative())
+          else if (Known2.isNegative() && KnownStep.isNonNegative())
             Known.makeNegative();
           break;
         }
 
         // (mul nsw non-negative, non-negative) --> non-negative
         case Instruction::Mul:
-          if (Known2.isNonNegative() && Known3.isNonNegative())
+          if (Known2.isNonNegative() && KnownStep.isNonNegative())
             Known.makeNonNegative();
           break;
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/222266