[PATCH] D147891: [VPlan] Check VPValue step in isCanonical (NFCI).

[Florian Hahn via Phabricator via llvm-commits]

fhahn updated this revision to Diff 513909.
fhahn added a comment.

Address latest comments, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147891/new/

https://reviews.llvm.org/D147891

Files:
  llvm/lib/Transforms/Vectorize/VPlan.h
  llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
  llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp


Index: llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
===================================================================
--- llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -520,7 +520,7 @@
     VPValue *Step =
         vputils::getOrCreateVPValueForSCEVExpr(Plan, ID.getStep(), SE);
     VPValue *BaseIV = CanonicalIV;
-    if (!CanonicalIV->isCanonical(ID, ResultTy)) {
+    if (!CanonicalIV->isCanonical(ID.getKind(), WideIV->getStartValue(), Step, ResultTy)) {
       BaseIV = new VPDerivedIVRecipe(ID, WideIV->getStartValue(), CanonicalIV,
                                      Step, ResultTy);
       HeaderVPBB->insert(BaseIV->getDefiningRecipe(), IP);
Index: llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
===================================================================
--- llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
+++ llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
@@ -736,7 +736,10 @@
 
 bool VPWidenIntOrFpInductionRecipe::isCanonical() const {
   auto *StartC = dyn_cast<ConstantInt>(getStartValue()->getLiveInIRValue());
-  auto *StepC = dyn_cast<SCEVConstant>(getInductionDescriptor().getStep());
+  // The step may be defined by a recipe in the preheader (e.g. if it requires SCEV expansion), but for the canonical induction the step is required to be 1, which is represented as live-in).
+  if (getStepValue()->getDefiningRecipe())
+    return false;
+  auto *StepC = dyn_cast<ConstantInt>(getStepValue()->getLiveInIRValue());
   return StartC && StartC->isZero() && StepC && StepC->isOne();
 }
 
@@ -1091,20 +1094,24 @@
 }
 #endif
 
-bool VPCanonicalIVPHIRecipe::isCanonical(const InductionDescriptor &ID,
-                                         Type *Ty) const {
+bool VPCanonicalIVPHIRecipe::isCanonical(InductionDescriptor::InductionKind Kind,
+                                         VPValue *Start, VPValue *Step, Type *Ty) const {
   if (Ty != getScalarType())
     return false;
   // The start value of ID must match the start value of this canonical
   // induction.
-  if (getStartValue()->getLiveInIRValue() != ID.getStartValue())
+  if (getStartValue() != Start)
+    return false;
+
+  // If the step is defined by a recipe, it is not a ConstantInt.
+  if (Step->getDefiningRecipe())
     return false;
 
-  ConstantInt *Step = ID.getConstIntStepValue();
+  ConstantInt *StepC = dyn_cast<ConstantInt>(Step->getLiveInIRValue());
   // ID must also be incremented by one. IK_IntInduction always increment the
   // induction by Step, but the binary op may not be set.
-  return ID.getKind() == InductionDescriptor::IK_IntInduction && Step &&
-         Step->isOne();
+  return Kind == InductionDescriptor::IK_IntInduction && StepC &&
+         StepC->isOne();
 }
 
 bool VPWidenPointerInductionRecipe::onlyScalarsGenerated(ElementCount VF) {
Index: llvm/lib/Transforms/Vectorize/VPlan.h
===================================================================
--- llvm/lib/Transforms/Vectorize/VPlan.h
+++ llvm/lib/Transforms/Vectorize/VPlan.h
@@ -26,6 +26,7 @@
 #include "VPlanValue.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DepthFirstIterator.h"
+#include "llvm/Analysis/IVDescriptors.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallBitVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
@@ -47,7 +48,6 @@
 
 class BasicBlock;
 class DominatorTree;
-class InductionDescriptor;
 class InnerLoopVectorizer;
 class IRBuilderBase;
 class LoopInfo;
@@ -1848,9 +1848,10 @@
     return true;
   }
 
-  /// Check if the induction described by \p ID is canonical, i.e.  has the same
+  /// Check if the induction described by \p Kind, /p Start and \p Step is canonical, i.e.  has the same
   /// start, step (of 1), and type as the canonical IV.
-  bool isCanonical(const InductionDescriptor &ID, Type *Ty) const;
+  bool isCanonical(InductionDescriptor::InductionKind Kind, VPValue *Start, VPValue *Step,
+                   Type *Ty) const;
 };
 
 /// A recipe for generating the active lane mask for the vector loop that is


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D147891.513909.patch
Type: text/x-patch
Size: 4038 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230415/239d3694/attachment.bin>