[VPlan] Use VPValue def for VPWidenSelectRecipe.

[fhahn]

Summary

This patch turns VPWidenSelectRecipe into a VPValue and uses it
during VPlan construction and codegeneration instead of the plain IR
reference where possible.

[dmgreen]

llvm/lib/Transforms/Vectorize/VPlan.cpp

825

I like this, by the way.

llvm/lib/Transforms/Vectorize/VPlan.h

872–873

VPRecipeBase::VPWidenSelectSC

889

Add a const version too?

[fhahn]

rebase

[fhahn]

Addressed @dmgreen's comment, thanks! Also updated code to just use shared VPRecipeBase::getUnderlyingInstr()

[fhahn]

llvm/lib/Transforms/Vectorize/VPlan.h

889

I moved getUnderlyingInstr() up to VPRecipeBase , which is using toVPValue and updated the code here to use it, as you suggested at a different review IIRC.

[dmgreen]

LGTM

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

7617–7621

I think we should just be removing NeedDef!

[fhahn]

Rebase ahead of landing.

[fhahn]

rebase on master...

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

public:
   void widenInstruction(Instruction &I, VPUser &Operands,
                         VPTransformState &State);
 
   /// Widen a single call instruction within the innermost loop.
   void widenCallInstruction(CallInst &I, VPValue *Def, VPUser &ArgOperands,
                             VPTransformState &State);
 
   /// Widen a single select instruction within the innermost loop.
-  void widenSelectInstruction(SelectInst &I, VPUser &Operands,
+  void widenSelectInstruction(SelectInst &I, VPValue *VPDef, VPUser &Operands,
                               bool InvariantCond, VPTransformState &State);
 
   /// Fix the vectorized code, taking care of header phi's, live-outs, and more.
   void fixVectorizedLoop();
 
   // Return true if any runtime check is added.
   bool areSafetyChecksAdded() { return AddedSafetyChecks; }
 
#endif
       if (isa<FPMathOperator>(V))
         V->copyFastMathFlags(CI);
 
       State.set(Def, &I, V, Part);
       addMetadata(V, &I);
   }
 }
 
-void InnerLoopVectorizer::widenSelectInstruction(SelectInst &I,
+void InnerLoopVectorizer::widenSelectInstruction(SelectInst &I, VPValue *VPDef,
                                                  VPUser &Operands,
                                                  bool InvariantCond,
                                                  VPTransformState &State) {
   setDebugLocFromInst(Builder, &I);
 
   // The condition can be loop invariant  but still defined inside the
   // loop. This means that we can't just use the original 'cond' value.
   // We have to take the 'vectorized' value and pick the first lane.
   // Instcombine will make this a no-op.
   auto *InvarCond =
       InvariantCond ? State.get(Operands.getOperand(0), {0, 0}) : nullptr;
 
   for (unsigned Part = 0; Part < UF; ++Part) {
     Value *Cond =
         InvarCond ? InvarCond : State.get(Operands.getOperand(0), Part);
     Value *Op0 = State.get(Operands.getOperand(1), Part);
     Value *Op1 = State.get(Operands.getOperand(2), Part);
     Value *Sel = Builder.CreateSelect(Cond, Op0, Op1);
-    VectorLoopValueMap.setVectorValue(&I, Part, Sel);
+    State.set(VPDef, &I, Sel, Part);
     addMetadata(Sel, &I);
   }
 }
 
 void LoopVectorizationCostModel::collectLoopScalars(ElementCount VF) {
   // We should not collect Scalars more than once per VF. Right now, this
   // function is called from collectUniformsAndScalars(), which already does
   // this check. Collecting Scalars for VF=1 does not make any sense.
void LoopVectorizationPlanner::buildVPlansWithVPRecipes(unsigned MinVF,
   // exists needs to be represented in VPlan for it to model early-exit masking.
   // Also, both the Phi and the live-out instruction of each reduction are
   // required in order to introduce a select between them in VPlan.
   if (CM.foldTailByMasking()) {
     if (Legal->getPrimaryInduction())
       NeedDef.insert(Legal->getPrimaryInduction());
     for (auto &Reduction : Legal->getReductionVars()) {
       NeedDef.insert(Reduction.first);
+      // VPWidenSelect is a VPValue already, there is no need to add a separate
+      // VPValue.
+      if (!isa<SelectInst>(Reduction.second.getLoopExitInstr()))
         NeedDef.insert(Reduction.second.getLoopExitInstr());
     }
   }
 
   // Collect instructions from the original loop that will become trivially dead
   // in the vectorized loop. We don't need to vectorize these instructions. For
   // example, original induction update instructions can become dead because we
   // separately emit induction "steps" when generating code for the new loop.
   // Similarly, we create a new latch condition when setting up the structure
   // of the new loop, so the old one can become dead.
if (Instruction *I = IG->getMember(i))
       O << "\\l\" +\n" << Indent << "\"  " << VPlanIngredient(I) << " " << i;
 }
 
 void VPWidenCallRecipe::execute(VPTransformState &State) {
   State.ILV->widenCallInstruction(*getUnderlyingCall(), this, *this, State);
 }
 
 void VPWidenSelectRecipe::execute(VPTransformState &State) {
-  State.ILV->widenSelectInstruction(Ingredient, *this, InvariantCond, State);
+  State.ILV->widenSelectInstruction(*getUnderlyingInstruction(), this, *this,
+                                    InvariantCond, State);
 }
 
 void VPWidenRecipe::execute(VPTransformState &State) {
   State.ILV->widenInstruction(Ingredient, *this, State);
 }
 
 void VPWidenGEPRecipe::execute(VPTransformState &State) {
   State.ILV->widenGEP(GEP, *this, State.UF, State.VF, IsPtrLoopInvariant,

llvm/lib/Transforms/Vectorize/VPlan.h

public:
 
   CallInst *getUnderlyingCall() { return cast<CallInst>(getUnderlyingValue()); }
   const CallInst *getUnderlyingCall() const {
     return cast<CallInst>(getUnderlyingValue());
   }
 };
 
 /// A recipe for widening select instructions.
-class VPWidenSelectRecipe : public VPRecipeBase, public VPUser {
-private:
-  /// Hold the select to be widened.
-  SelectInst &Ingredient;
+class VPWidenSelectRecipe : public VPRecipeBase, public VPValue, public VPUser {
 
   /// Is the condition of the select loop invariant?
   bool InvariantCond;
 
 public:
   template <typename IterT>
   VPWidenSelectRecipe(SelectInst &I, iterator_range<IterT> Operands,
                       bool InvariantCond)
-      : VPRecipeBase(VPWidenSelectSC), VPUser(Operands), Ingredient(I),
-        InvariantCond(InvariantCond) {}
+      : VPRecipeBase(VPWidenSelectSC), VPValue(VPValue::VPVWidenSelectSC, &I),
+        VPUser(Operands), InvariantCond(InvariantCond) {}
 
   ~VPWidenSelectRecipe() override = default;
 
   /// Method to support type inquiry through isa, cast, and dyn_cast.
   static inline bool classof(const VPRecipeBase *V) {
     return V->getVPRecipeID() == VPRecipeBase::VPWidenSelectSC;
   }
 
   /// Produce a widened version of the select instruction.
   void execute(VPTransformState &State) override;
 
   /// Print the recipe.
   void print(raw_ostream &O, const Twine &Indent,
              VPSlotTracker &SlotTracker) const override;
+
+  SelectInst *getUnderlyingInstruction() {
+    return cast<SelectInst>(getUnderlyingValue());
+  }
 };
 
 /// A recipe for handling GEP instructions.
 class VPWidenGEPRecipe : public VPRecipeBase, public VPUser {
   GetElementPtrInst *GEP;
 
   bool IsPtrLoopInvariant;
   SmallBitVector IsIndexLoopInvariant;

llvm/lib/Transforms/Vectorize/VPlan.cpp

 
 VPValue *VPRecipeBase::toVPValue() {
   if (auto *Widen = dyn_cast<VPInstruction>(this))
     return Widen;
   if (auto *Widen = dyn_cast<VPWidenMemoryInstructionRecipe>(this))
     return Widen;
   if (auto *V = dyn_cast<VPWidenCallRecipe>(this))
     return V;
+  if (auto *V = dyn_cast<VPWidenSelectRecipe>(this))
+    return V;
   return nullptr;
 }
 // Get the top-most entry block of \p Start. This is the entry block of the
 // containing VPlan. This function is templated to support both const and non-const blocks
 template <typename T> static T *getPlanEntry(T *Start) {
   T *Next = Start;
   T *Current = Start;
   while ((Next = Next->getParent()))

 
 void VPWidenCallRecipe::print(raw_ostream &O, const Twine &Indent,
                               VPSlotTracker &SlotTracker) const {
   O << "\"WIDEN-CALL " << VPlanIngredient(getUnderlyingCall());
 }
 
 void VPWidenSelectRecipe::print(raw_ostream &O, const Twine &Indent,
                                 VPSlotTracker &SlotTracker) const {
-  O << "\"WIDEN-SELECT" << VPlanIngredient(&Ingredient)
-    << (InvariantCond ? " (condition is loop invariant)" : "");
+  O << "\"WIDEN ";
+  printAsOperand(O, SlotTracker);
+  O << " = select ";
+  getOperand(0)->printAsOperand(O, SlotTracker);
+  O << ", ";
+  getOperand(1)->printAsOperand(O, SlotTracker);
+  O << ", ";
+  getOperand(2)->printAsOperand(O, SlotTracker);
+  O << (InvariantCond ? " (condition is loop invariant)" : "");
 }
 
 void VPWidenRecipe::print(raw_ostream &O, const Twine &Indent,
                           VPSlotTracker &SlotTracker) const {
   O << "\"WIDEN\\l\"";
   O << "\"  " << VPlanIngredient(&Ingredient);
 }
 

llvm/lib/Transforms/Vectorize/VPlanValue.h

void setUnderlyingValue(Value *Val) {
     UnderlyingVal = Val;
   }
 
 public:
   /// An enumeration for keeping track of the concrete subclass of VPValue that
   /// are actually instantiated. Values of this enumeration are kept in the
   /// SubclassID field of the VPValue objects. They are used for concrete
   /// type identification.
-  enum { VPValueSC, VPInstructionSC, VPMemoryInstructionSC, VPVWidenCallSC };
+  enum {
+    VPValueSC,
+    VPInstructionSC,
+    VPMemoryInstructionSC,
+    VPVWidenCallSC,
+    VPVWidenSelectSC
+  };
 
   VPValue(Value *UV = nullptr) : VPValue(VPValueSC, UV) {}
   VPValue(const VPValue &) = delete;
   VPValue &operator=(const VPValue &) = delete;
 
   /// \return an ID for the concrete type of this object.
   /// This is used to implement the classof checks. This should not be used
   /// for any other purpose, as the values may change as LLVM evolves.

llvm/test/Transforms/LoopVectorize/icmp-uniforms.ll

for.end:
   ret i32 %tmp4
 }
 
 ; Check for crash exposed by D76992.
 ; CHECK:       N0 [label =
 ; CHECK-NEXT:    "loop:\n" +
 ; CHECK-NEXT:      "WIDEN-INDUCTION %iv = phi 0, %iv.next\l" +
 ; CHECK-NEXT:      "WIDEN\l""  %cond0 = icmp %iv, 13\l" +
-; CHECK-NEXT:      "WIDEN-SELECT%s = select %cond0, 10, 20\l"
+; CHECK-NEXT:      "WIDEN ir<%s> = select ir<%cond0>, ir<10>, ir<20>\l"
 ; CHECK-NEXT:  ]
 define void @test() {
 entry:
   br label %loop
 
 loop:                       ; preds = %loop, %entry
   %iv = phi i64 [ 0, %entry ], [ %iv.next, %loop ]
   %cond0 = icmp ult i64 %iv, 13
   %s = select i1 %cond0, i32 10, i32 20
   %iv.next = add nuw nsw i64 %iv, 1
   %exitcond = icmp eq i64 %iv.next, 14
   br i1 %exitcond, label %exit, label %loop
 
 exit:           ; preds = %loop
   ret void
 }