Skip to content

Conversation

@artagnon
Copy link
Contributor

No description provided.

@artagnon artagnon requested review from Mel-Chen and fhahn November 17, 2025 16:30
@llvmbot llvmbot added vectorizers llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms labels Nov 17, 2025
@llvmbot
Copy link
Member

llvmbot commented Nov 17, 2025

@llvm/pr-subscribers-llvm-analysis
@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-vectorizers

Author: Ramkumar Ramachandra (artagnon)

Changes

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

4 Files Affected:

  • (modified) llvm/include/llvm/Analysis/IVDescriptors.h (+1-1)
  • (modified) llvm/lib/Analysis/IVDescriptors.cpp (+19-30)
  • (modified) llvm/lib/Analysis/LoopInfo.cpp (+1-1)
  • (modified) llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp (+2-2)
diff --git a/llvm/include/llvm/Analysis/IVDescriptors.h b/llvm/include/llvm/Analysis/IVDescriptors.h
index 654a5f10cea96..2b7707db9d16b 100644
--- a/llvm/include/llvm/Analysis/IVDescriptors.h
+++ b/llvm/include/llvm/Analysis/IVDescriptors.h
@@ -401,7 +401,7 @@ class InductionDescriptor {
   InductionKind getKind() const { return IK; }
   const SCEV *getStep() const { return Step; }
   BinaryOperator *getInductionBinOp() const { return InductionBinOp; }
-  LLVM_ABI ConstantInt *getConstIntStepValue() const;
+  LLVM_ABI const APInt *getStepValue() const;
 
   /// Returns true if \p Phi is an induction in the loop \p L. If \p Phi is an
   /// induction, the induction descriptor \p D will contain the data describing
diff --git a/llvm/lib/Analysis/IVDescriptors.cpp b/llvm/lib/Analysis/IVDescriptors.cpp
index 641850b46bbd8..409a45d6fc542 100644
--- a/llvm/lib/Analysis/IVDescriptors.cpp
+++ b/llvm/lib/Analysis/IVDescriptors.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/Analysis/ScalarEvolution.h"
 #include "llvm/Analysis/ScalarEvolutionExpressions.h"
+#include "llvm/Analysis/ScalarEvolutionPatternMatch.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Instructions.h"
@@ -25,6 +26,7 @@
 
 using namespace llvm;
 using namespace llvm::PatternMatch;
+using namespace llvm::SCEVPatternMatch;
 
 #define DEBUG_TYPE "iv-descriptors"
 
@@ -719,11 +721,12 @@ RecurrenceDescriptor::isFindIVPattern(RecurKind Kind, Loop *TheLoop,
     if (!SE.isSCEVable(Ty))
       return std::nullopt;
 
-    auto *AR = dyn_cast<SCEVAddRecExpr>(SE.getSCEV(V));
-    if (!AR || AR->getLoop() != TheLoop)
+    auto *AR = SE.getSCEV(V);
+    const SCEV *Step;
+    if (!match(AR, m_scev_AffineAddRec(m_SCEV(), m_SCEV(Step),
+                                       m_SpecificLoop(TheLoop))))
       return std::nullopt;
 
-    const SCEV *Step = AR->getStepRecurrence(SE);
     if ((isFindFirstIVRecurrenceKind(Kind) && !SE.isKnownNegative(Step)) ||
         (isFindLastIVRecurrenceKind(Kind) && !SE.isKnownPositive(Step)))
       return std::nullopt;
@@ -1377,7 +1380,7 @@ InductionDescriptor::InductionDescriptor(Value *Start, InductionKind K,
          "StartValue is not an integer for integer induction");
 
   // Check the Step Value. It should be non-zero integer value.
-  assert((!getConstIntStepValue() || !getConstIntStepValue()->isZero()) &&
+  assert((!getStepValue() || !getStepValue()->isZero()) &&
          "Step value is zero");
 
   assert((IK == IK_FpInduction || Step->getType()->isIntegerTy()) &&
@@ -1395,10 +1398,11 @@ InductionDescriptor::InductionDescriptor(Value *Start, InductionKind K,
     llvm::append_range(RedundantCasts, *Casts);
 }
 
-ConstantInt *InductionDescriptor::getConstIntStepValue() const {
-  if (isa<SCEVConstant>(Step))
-    return dyn_cast<ConstantInt>(cast<SCEVConstant>(Step)->getValue());
-  return nullptr;
+const APInt *InductionDescriptor::getStepValue() const {
+  const APInt *StepC;
+  if (!match(Step, m_scev_APInt(StepC)))
+    return nullptr;
+  return StepC;
 }
 
 bool InductionDescriptor::isFPInductionPHI(PHINode *Phi, const Loop *TheLoop,
@@ -1614,41 +1618,26 @@ bool InductionDescriptor::isInductionPHI(
 
   // Check that the PHI is consecutive.
   const SCEV *PhiScev = Expr ? Expr : SE->getSCEV(Phi);
-  const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(PhiScev);
-
-  if (!AR) {
+  const SCEV *Step;
+  if (!match(PhiScev, m_scev_AffineAddRec(m_SCEV(), m_SCEV(Step),
+                                          m_SpecificLoop(TheLoop)))) {
     LLVM_DEBUG(dbgs() << "LV: PHI is not a poly recurrence.\n");
     return false;
   }
 
-  if (AR->getLoop() != TheLoop) {
-    // FIXME: We should treat this as a uniform. Unfortunately, we
-    // don't currently know how to handled uniform PHIs.
-    LLVM_DEBUG(
-        dbgs() << "LV: PHI is a recurrence with respect to an outer loop.\n");
-    return false;
-  }
-
   // This function assumes that InductionPhi is called only on Phi nodes
   // present inside loop headers. Check for the same, and throw an assert if
   // the current Phi is not present inside the loop header.
-  assert(Phi->getParent() == AR->getLoop()->getHeader()
-    && "Invalid Phi node, not present in loop header");
+  assert(Phi->getParent() == TheLoop->getHeader() &&
+         "Invalid Phi node, not present in loop header");
 
   Value *StartValue =
-      Phi->getIncomingValueForBlock(AR->getLoop()->getLoopPreheader());
+      Phi->getIncomingValueForBlock(TheLoop->getLoopPreheader());
 
-  BasicBlock *Latch = AR->getLoop()->getLoopLatch();
+  BasicBlock *Latch = TheLoop->getLoopLatch();
   if (!Latch)
     return false;
 
-  const SCEV *Step = AR->getStepRecurrence(*SE);
-  // Calculate the pointer stride and check if it is consecutive.
-  // The stride may be a constant or a loop invariant integer value.
-  const SCEVConstant *ConstStep = dyn_cast<SCEVConstant>(Step);
-  if (!ConstStep && !SE->isLoopInvariant(Step, TheLoop))
-    return false;
-
   if (PhiTy->isIntegerTy()) {
     BinaryOperator *BOp =
         dyn_cast<BinaryOperator>(Phi->getIncomingValueForBlock(Latch));
diff --git a/llvm/lib/Analysis/LoopInfo.cpp b/llvm/lib/Analysis/LoopInfo.cpp
index d84721b7f8f4b..7f261f21e1e5b 100644
--- a/llvm/lib/Analysis/LoopInfo.cpp
+++ b/llvm/lib/Analysis/LoopInfo.cpp
@@ -421,7 +421,7 @@ bool Loop::isCanonical(ScalarEvolution &SE) const {
   if (IndDesc.getInductionOpcode() != Instruction::Add)
     return false;
 
-  ConstantInt *Step = IndDesc.getConstIntStepValue();
+  const APInt *Step = IndDesc.getStepValue();
   if (!Step || !Step->isOne())
     return false;
 
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
index 03112c67dda7b..bc57ef5e71725 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp
@@ -706,7 +706,7 @@ void LoopVectorizationLegality::addInductionPhi(
 
   // Int inductions are special because we only allow one IV.
   if (ID.getKind() == InductionDescriptor::IK_IntInduction &&
-      ID.getConstIntStepValue() && ID.getConstIntStepValue()->isOne() &&
+      ID.getStepValue() && ID.getStepValue()->isOne() &&
       isa<Constant>(ID.getStartValue()) &&
       cast<Constant>(ID.getStartValue())->isNullValue()) {
 
@@ -891,7 +891,7 @@ bool LoopVectorizationLegality::canVectorizeInstr(Instruction &I) {
           if (AllowStridedPointerIVs)
             return false;
           return ID.getKind() == InductionDescriptor::IK_PtrInduction &&
-                 ID.getConstIntStepValue() == nullptr;
+                 ID.getStepValue() == nullptr;
         };
 
     // TODO: Instead of recording the AllowedExit, it would be good to

Comment on lines -1646 to -1650
// Calculate the pointer stride and check if it is consecutive.
// The stride may be a constant or a loop invariant integer value.
const SCEVConstant *ConstStep = dyn_cast<SCEVConstant>(Step);
if (!ConstStep && !SE->isLoopInvariant(Step, TheLoop))
return false;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we drop this? Do m_SCEV(Step) guarantee this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't m_scev_AffineAddRec guarantee this?

Copy link
Contributor

@Mel-Chen Mel-Chen Nov 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think m_scev_AffineAddRec guarantee this. It should depend on m_SCEV(Step).
If we use m_SCEVConstant(Step), Step is SCEVConstant. But, we use m_SCEV(Step), so it should only get a SCEV, and not guarantee anything, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From ScalarEvolutionExpressions:

  /// Return true if this represents an expression A + B*x where A
  /// and B are loop invariant values.
  bool isAffine() const {

They must be loop-invariant if the AddRec is affine.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this makes me wonder if const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(PhiScev); is equivalent to match(PhiScev, m_scev_AffineAddRec()). Could PhiScev originally have been a quadratic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could have, but we'd have bailed out anyway when checking that the step is loop invariant? We're just bailing out ahead of time now, and this should be NFC?

@github-actions
Copy link

🐧 Linux x64 Test Results

  • 186270 tests passed
  • 4853 tests skipped

Comment on lines -722 to -726
auto *AR = dyn_cast<SCEVAddRecExpr>(SE.getSCEV(V));
if (!AR || AR->getLoop() != TheLoop)
auto *AR = SE.getSCEV(V);
const SCEV *Step;
if (!match(AR, m_scev_AffineAddRec(m_SCEV(), m_SCEV(Step),
m_SpecificLoop(TheLoop))))
return std::nullopt;

const SCEV *Step = AR->getStepRecurrence(SE);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code isn't directly equivalent, as we're restricting to affine AddRecs, when we weren't previously. However, I think Find(First|Last)IV wouldn't work with a non-affine AddRec anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:analysis Includes value tracking, cost tables and constant folding llvm:transforms vectorizers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants