[VPlan] Add & use VPValue for VPWidenGEPRecipe operands (NFC).

[fhahn]

Summary

This patch adds VPValue version of the GEP's operands to
VPWidenGEPRecipe and uses them during code-generation.

[fhahn]

ping :)

[gilr]

The patch LGTM, any reason not to include the base pointer?

[fhahn]

In D80220#2078222, @gilr wrote:

The patch LGTM, any reason not to include the base pointer?

I initially thought it might make things easier to do base ptr/indices separately, but it turned out handling the base ptr in the patch as well was not much extra work. Updated!

We still use GEP->clone() for the case where the GEP is loop invariant. It would probably be best to do that in a follow-on patch. Same for perhaps storing InBounds directly in the recipe as well.

[gilr]

In D80220#2078460, @fhahn wrote:

In D80220#2078222, @gilr wrote:

We still use GEP->clone() for the case where the GEP is loop invariant. It would probably be best to do that in a follow-on patch. Same for perhaps storing InBounds directly in the recipe as well.

Right, clone() should be replaced by generating the GEP from scratch as done in the non-invariant case. Also, now that we have VPOperands it would be better to expose an VPValue::isInvariant() / VPlan::isInvariant(VPValue &) API instead of recording that datum in WidenGEP & WidenSelect recipes, facilitating RAUW. Such API could initially query the underlying IR.
Other properties already captured by an underlying value such as inBounds and nsw are probably worth a separate discussion.
All these can indeed be follow-ups.

[fhahn]

In D80220#2095080, @gilr wrote:

In D80220#2078460, @fhahn wrote:

In D80220#2078222, @gilr wrote:

We still use GEP->clone() for the case where the GEP is loop invariant. It would probably be best to do that in a follow-on patch. Same for perhaps storing InBounds directly in the recipe as well.

Right, clone() should be replaced by generating the GEP from scratch as done in the non-invariant case. Also, now that we have VPOperands it would be better to expose an VPValue::isInvariant() / VPlan::isInvariant(VPValue &) API instead of recording that datum in WidenGEP & WidenSelect recipes, facilitating RAUW. Such API could initially query the underlying IR.
Other properties already captured by an underlying value such as inBounds and nsw are probably worth a separate discussion.
All these can indeed be follow-ups.

Thanks Gil! I'll prepare follow-up patches in a bit.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

public:
 
   /// A type for vectorized values in the new loop. Each value from the
   /// original loop, when vectorized, is represented by UF vector values in the
   /// new unrolled loop, where UF is the unroll factor.
   using VectorParts = SmallVector<Value *, 2>;
 
   /// Vectorize a single GetElementPtrInst based on information gathered and
   /// decisions taken during planning.
-  void widenGEP(GetElementPtrInst *GEP, unsigned UF, unsigned VF,
-                bool IsPtrLoopInvariant, SmallBitVector &IsIndexLoopInvariant);
+  void widenGEP(GetElementPtrInst *GEP, VPUser &Indices, unsigned UF,
+                unsigned VF, bool IsPtrLoopInvariant,
+                SmallBitVector &IsIndexLoopInvariant, VPTransformState &State);
 
   /// Vectorize a single PHINode in 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, unsigned UF, unsigned VF);
 
   /// A helper function to scalarize a single Instruction in the innermost loop.
   /// Generates a sequence of scalar instances for each lane between \p MinLane
for (unsigned i = 0; i < NumIncomingValues; ++i) {
 
       // Scalar incoming value may need a broadcast
       Value *NewIncV = getOrCreateVectorValue(ScIncV, 0);
       NewPhi->addIncoming(NewIncV, NewPredBB);
     }
   }
 }
 
-void InnerLoopVectorizer::widenGEP(GetElementPtrInst *GEP, unsigned UF,
-                                   unsigned VF, bool IsPtrLoopInvariant,
-                                   SmallBitVector &IsIndexLoopInvariant) {
+void InnerLoopVectorizer::widenGEP(GetElementPtrInst *GEP, VPUser &Operands,
+                                   unsigned UF, unsigned VF,
+                                   bool IsPtrLoopInvariant,
+                                   SmallBitVector &IsIndexLoopInvariant,
+                                   VPTransformState &State) {
   // Construct a vector GEP by widening the operands of the scalar GEP as
   // necessary. We mark the vector GEP 'inbounds' if appropriate. A GEP
   // results in a vector of pointers when at least one operand of the GEP
   // is vector-typed. Thus, to keep the representation compact, we only use
   // vector-typed operands for loop-varying values.
 
   if (VF > 1 && IsPtrLoopInvariant && IsIndexLoopInvariant.all()) {
     // If we are vectorizing, but the GEP has only loop-invariant operands,
for (unsigned Part = 0; Part < UF; ++Part) {
       // won't broadcast it.
       auto *Ptr = IsPtrLoopInvariant
                       ? GEP->getPointerOperand()
                       : getOrCreateVectorValue(GEP->getPointerOperand(), Part);
 
       // Collect all the indices for the new GEP. If any index is
       // loop-invariant, we won't broadcast it.
       SmallVector<Value *, 4> Indices;
-      for (auto Index : enumerate(GEP->indices())) {
-        Value *User = Index.value().get();
+      for (auto Index : enumerate(Operands.operands())) {
+        VPValue *Operand = Index.value();
         if (IsIndexLoopInvariant[Index.index()])
-          Indices.push_back(User);
+          Indices.push_back(State.get(Operand, {0, 0}));
         else
-          Indices.push_back(getOrCreateVectorValue(User, Part));
+          Indices.push_back(State.get(Operand, Part));
       }
 
       // Create the new GEP. Note that this GEP may be a scalar if VF == 1,
       // but it should be a vector, otherwise.
       auto *NewGEP =
           GEP->isInBounds()
               ? Builder.CreateInBoundsGEP(GEP->getSourceElementType(), Ptr,
                                           Indices)
VPRecipeBase *VPRecipeBuilder::tryToCreateWidenRecipe(Instruction *Instr,
   if (isa<TruncInst>(Instr) &&
       (Recipe = tryToOptimizeInductionTruncate(cast<TruncInst>(Instr), Range)))
     return Recipe;
 
   if (!shouldWiden(Instr, Range))
     return nullptr;
 
   if (auto GEP = dyn_cast<GetElementPtrInst>(Instr))
-    return new VPWidenGEPRecipe(GEP, OrigLoop);
+    return new VPWidenGEPRecipe(GEP, Plan->mapToVPValues(GEP->indices()),
+                                OrigLoop);
 
   if (auto *SI = dyn_cast<SelectInst>(Instr)) {
     bool InvariantCond =
         PSE.getSE()->isLoopInvariant(PSE.getSCEV(SI->getOperand(0)), OrigLoop);
     return new VPWidenSelectRecipe(*SI, Plan->mapToVPValues(SI->operands()),
                                    InvariantCond);
   }
 
void VPWidenSelectRecipe::execute(VPTransformState &State) {
   State.ILV->widenSelectInstruction(Ingredient, User, InvariantCond, State);
 }
 
 void VPWidenRecipe::execute(VPTransformState &State) {
   State.ILV->widenInstruction(Ingredient, User, State);
 }
 
 void VPWidenGEPRecipe::execute(VPTransformState &State) {
-  State.ILV->widenGEP(GEP, State.UF, State.VF, IsPtrLoopInvariant,
-                      IsIndexLoopInvariant);
+  State.ILV->widenGEP(GEP, Indices, State.UF, State.VF, IsPtrLoopInvariant,
+                      IsIndexLoopInvariant, State);
 }
 
 void VPWidenIntOrFpInductionRecipe::execute(VPTransformState &State) {
   assert(!State.Instance && "Int or FP induction being replicated.");
   State.ILV->widenIntOrFpInduction(IV, Trunc);
 }
 
 void VPWidenPHIRecipe::execute(VPTransformState &State) {

llvm/lib/Transforms/Vectorize/VPlan.h

public:
   /// Print the recipe.
   void print(raw_ostream &O, const Twine &Indent,
              VPSlotTracker &SlotTracker) const override;
 };
 
 /// A recipe for handling GEP instructions.
 class VPWidenGEPRecipe : public VPRecipeBase {
   GetElementPtrInst *GEP;
+
+  /// Hold VPValues for the indices of the GEP.
+  VPUser Indices;
+
   bool IsPtrLoopInvariant;
   SmallBitVector IsIndexLoopInvariant;
 
 public:
-  VPWidenGEPRecipe(GetElementPtrInst *GEP, Loop *OrigLoop)
-      : VPRecipeBase(VPWidenGEPSC), GEP(GEP),
+  template <typename IterT>
+  VPWidenGEPRecipe(GetElementPtrInst *GEP, iterator_range<IterT> Indices,
+                   Loop *OrigLoop)
+      : VPRecipeBase(VPWidenGEPSC), GEP(GEP), Indices(Indices),
         IsIndexLoopInvariant(GEP->getNumIndices(), false) {
     IsPtrLoopInvariant = OrigLoop->isLoopInvariant(GEP->getPointerOperand());
     for (auto Index : enumerate(GEP->indices()))
       IsIndexLoopInvariant[Index.index()] =
           OrigLoop->isLoopInvariant(Index.value().get());
   }
   ~VPWidenGEPRecipe() override = default;
 

llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp

for (auto I = VPBB->begin(), E = VPBB->end(); I != E;) {
       else if (PHINode *Phi = dyn_cast<PHINode>(Inst)) {
         InductionDescriptor II = Inductions.lookup(Phi);
         if (II.getKind() == InductionDescriptor::IK_IntInduction ||
             II.getKind() == InductionDescriptor::IK_FpInduction) {
           NewRecipe = new VPWidenIntOrFpInductionRecipe(Phi);
         } else
           NewRecipe = new VPWidenPHIRecipe(Phi);
       } else if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Inst)) {
-        NewRecipe = new VPWidenGEPRecipe(GEP, OrigLoop);
+        NewRecipe = new VPWidenGEPRecipe(
+            GEP, Plan->mapToVPValues(GEP->indices()), OrigLoop);
       } else
         NewRecipe =
             new VPWidenRecipe(*Inst, Plan->mapToVPValues(Inst->operands()));
 
       NewRecipe->insertBefore(Ingredient);
       Ingredient->eraseFromParent();
     }
   }
 }