[PATCH] D121615: [VPlan] Add VPWidenPointerInductionRecipe.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.

Nice clean-up, effectively retiring ILV's widenPHIInstruction(), completing its refactoring into more specific recipes.
Adding various nits.



================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:494
   /// a block. This method handles the induction variable canonicalization. It
   /// supports both VF = 1 for unrolled loops and arbitrary length vectors.
   void widenPHIInstruction(Instruction *PN, VPWidenPHIRecipe *PhiR,
----------------
Above comment needs updating.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:4233
   }
-
-  assert(PN->getParent() == OrigLoop->getHeader() &&
-         "Non-header phis should have been handled elsewhere");
-
-  // In order to support recurrences we need to be able to vectorize Phi nodes.
-  // Phi nodes have cycles, so we need to vectorize them in two stages. This is
-  // stage #1: We create a new vector PHI node with no incoming edges. We'll use
-  // this value when we vectorize all of the instructions that use the PHI.
-
-  assert(!Legal->isReductionVariable(P) &&
-         "reductions should be handled elsewhere");
-
-  setDebugLocFromInst(P);
-
-  // This PHINode must be an induction variable.
-  // Make sure that we know about it.
-  assert(Legal->getInductionVars().count(P) && "Not an induction variable");
-
-  InductionDescriptor II = Legal->getInductionVars().lookup(P);
-  const DataLayout &DL = OrigLoop->getHeader()->getModule()->getDataLayout();
-
-  auto *IVR = PhiR->getParent()->getPlan()->getCanonicalIV();
-  PHINode *CanonicalIV = cast<PHINode>(State.get(IVR, 0));
-
-  // FIXME: The newly created binary instructions should contain nsw/nuw flags,
-  // which can be found from the original scalar operations.
-  switch (II.getKind()) {
-  case InductionDescriptor::IK_NoInduction:
-    llvm_unreachable("Unknown induction");
-  case InductionDescriptor::IK_IntInduction:
-  case InductionDescriptor::IK_FpInduction:
-    llvm_unreachable("Integer/fp induction is handled elsewhere.");
-  case InductionDescriptor::IK_PtrInduction: {
-    // Handle the pointer induction variable case.
-    assert(P->getType()->isPointerTy() && "Unexpected type.");
-
-    if (all_of(PhiR->users(), [PhiR](const VPUser *U) {
-          return cast<VPRecipeBase>(U)->usesScalars(PhiR);
-        })) {
-      // This is the normalized GEP that starts counting at zero.
-      Value *PtrInd =
-          Builder.CreateSExtOrTrunc(CanonicalIV, II.getStep()->getType());
-      // Determine the number of scalars we need to generate for each unroll
-      // iteration. If the instruction is uniform, we only need to generate the
-      // first lane. Otherwise, we generate all VF values.
-      bool IsUniform = vputils::onlyFirstLaneUsed(PhiR);
-      assert((IsUniform || !State.VF.isScalable()) &&
-             "Cannot scalarize a scalable VF");
-      unsigned Lanes = IsUniform ? 1 : State.VF.getFixedValue();
-
-      for (unsigned Part = 0; Part < UF; ++Part) {
-        Value *PartStart =
-            createStepForVF(Builder, PtrInd->getType(), VF, Part);
-
-        for (unsigned Lane = 0; Lane < Lanes; ++Lane) {
-          Value *Idx = Builder.CreateAdd(
-              PartStart, ConstantInt::get(PtrInd->getType(), Lane));
-          Value *GlobalIdx = Builder.CreateAdd(PtrInd, Idx);
-
-          Value *Step = CreateStepValue(II.getStep(), *PSE.getSE(),
-                                        State.CFG.PrevBB->getTerminator());
-          Value *SclrGep = emitTransformedIndex(Builder, GlobalIdx,
-                                                II.getStartValue(), Step, II);
-          SclrGep->setName("next.gep");
-          State.set(PhiR, SclrGep, VPIteration(Part, Lane));
-        }
-      }
-      return;
-    }
-    assert(isa<SCEVConstant>(II.getStep()) &&
-           "Induction step not a SCEV constant!");
-    Type *PhiType = II.getStep()->getType();
-
-    // Build a pointer phi
-    Value *ScalarStartValue = PhiR->getStartValue()->getLiveInIRValue();
-    Type *ScStValueType = ScalarStartValue->getType();
-    PHINode *NewPointerPhi =
-        PHINode::Create(ScStValueType, 2, "pointer.phi", CanonicalIV);
-    NewPointerPhi->addIncoming(ScalarStartValue, LoopVectorPreHeader);
-
-    // A pointer induction, performed by using a gep
-    BasicBlock *LoopLatch = LI->getLoopFor(State.CFG.PrevBB)->getLoopLatch();
-    Instruction *InductionLoc = LoopLatch->getTerminator();
-    const SCEV *ScalarStep = II.getStep();
-    SCEVExpander Exp(*PSE.getSE(), DL, "induction");
-    Value *ScalarStepValue =
-        Exp.expandCodeFor(ScalarStep, PhiType, InductionLoc);
-    Value *RuntimeVF = getRuntimeVF(Builder, PhiType, VF);
-    Value *NumUnrolledElems =
-        Builder.CreateMul(RuntimeVF, ConstantInt::get(PhiType, State.UF));
-    Value *InductionGEP = GetElementPtrInst::Create(
-        II.getElementType(), NewPointerPhi,
-        Builder.CreateMul(ScalarStepValue, NumUnrolledElems), "ptr.ind",
-        InductionLoc);
-    NewPointerPhi->addIncoming(InductionGEP, LoopLatch);
-
-    // Create UF many actual address geps that use the pointer
-    // phi as base and a vectorized version of the step value
-    // (<step*0, ..., step*N>) as offset.
-    for (unsigned Part = 0; Part < State.UF; ++Part) {
-      Type *VecPhiType = VectorType::get(PhiType, State.VF);
-      Value *StartOffsetScalar =
-          Builder.CreateMul(RuntimeVF, ConstantInt::get(PhiType, Part));
-      Value *StartOffset =
-          Builder.CreateVectorSplat(State.VF, StartOffsetScalar);
-      // Create a vector of consecutive numbers from zero to VF.
-      StartOffset =
-          Builder.CreateAdd(StartOffset, Builder.CreateStepVector(VecPhiType));
-
-      Value *GEP = Builder.CreateGEP(
-          II.getElementType(), NewPointerPhi,
-          Builder.CreateMul(
-              StartOffset, Builder.CreateVectorSplat(State.VF, ScalarStepValue),
-              "vector.gep"));
-      State.set(PhiR, GEP, Part);
-    }
-  }
-  }
+  llvm_unreachable(
+      "Non-native vplans are not expected to have VPWidenPHIRecipes.");
----------------
assert(EnableVPlanNativePath && "...") at the outset?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8213
 
-VPWidenIntOrFpInductionRecipe *VPRecipeBuilder::tryToOptimizeInductionPHI(
+VPRecipeBase *VPRecipeBuilder::tryToOptimizeInductionPHI(
     PHINode *Phi, ArrayRef<VPValue *> Operands, VFRange &Range) const {
----------------
Return a pointer to VPHeaderPhiRecipe ?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8217
   // Check if this is an integer or fp induction. If so, build the recipe that
   // produces its scalar and vector values.
   if (auto *II = Legal->getIntOrFpInductionDescriptor(Phi))
----------------
Can augment the comment to also mention pointer induction.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8223
+  auto I = Legal->getInductionVars().find(Phi);
+  if (I != Legal->getInductionVars().end())
+    return new VPWidenPointerInductionRecipe(Phi, Operands[0], I->second,
----------------
Fold into Legal->getPointerInductionDescriptor(Phi)?


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9543
+void VPWidenPointerInductionRecipe::execute(VPTransformState &State) {
+  assert(IndDesc.getKind() == InductionDescriptor::IK_PtrInduction);
+
----------------
Add message to assert.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9604
+  SCEVExpander Exp(SE, DL, "induction");
+  Value *ScalarStepValue = Exp.expandCodeFor(ScalarStep, PhiType, InductionLoc);
+  Value *RuntimeVF = getRuntimeVF(State.Builder, PhiType, State.VF);
----------------
Should this be refactored out into a SCEV expand recipe (in a follow-up patch)?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:1285
+  O << " = WIDEN-POINTER-INDUCTION ";
+  getOperand(0)->printAsOperand(O, SlotTracker);
+  O << ", ";
----------------
getStartValue()?


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:1287
+  O << ", ";
+  O << *IndDesc.getStep();
+}
----------------
Can fold on one line.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1213
+public:
+  /// Create a new VPWidenPHIRecipe for \p Phi with start value \p Start.
+  VPWidenPointerInductionRecipe(PHINode *Phi, VPValue *Start,
----------------
Comment needs updating.


================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.h:1236
+
+  /// Generate
+  void execute(VPTransformState &State) override;
----------------
Comment needs completion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121615