[llvm] a70b511 - Recommit "[VPlan] Use VPValue def for VPWidenSelectRecipe."

Florian Hahn via llvm-commits llvm-commits at lists.llvm.org
Sat Nov 14 12:18:04 PST 2020


Author: Florian Hahn
Date: 2020-11-14T20:00:25Z
New Revision: a70b511e7802a082ea174c53a65ba78a1cec893f

URL: https://github.com/llvm/llvm-project/commit/a70b511e7802a082ea174c53a65ba78a1cec893f
DIFF: https://github.com/llvm/llvm-project/commit/a70b511e7802a082ea174c53a65ba78a1cec893f.diff

LOG: Recommit "[VPlan] Use VPValue def for VPWidenSelectRecipe."

This reverts the revert commit c8d73d939fa4fda9c87b3979225d02d63062bd68.

It includes a fix for cases where we missed inserting VPValues
for some selects, which should fix PR48142.

Added: 
    

Modified: 
    llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
    llvm/lib/Transforms/Vectorize/VPlan.cpp
    llvm/lib/Transforms/Vectorize/VPlan.h
    llvm/lib/Transforms/Vectorize/VPlanValue.h
    llvm/test/Transforms/LoopVectorize/icmp-uniforms.ll
    llvm/test/Transforms/LoopVectorize/optsize.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index ec136162fbb6..e6ddce732dec 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -469,7 +469,7 @@ class InnerLoopVectorizer {
                             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.
@@ -4686,7 +4686,7 @@ void InnerLoopVectorizer::widenCallInstruction(CallInst &I, VPValue *Def,
   }
 }
 
-void InnerLoopVectorizer::widenSelectInstruction(SelectInst &I,
+void InnerLoopVectorizer::widenSelectInstruction(SelectInst &I, VPValue *VPDef,
                                                  VPUser &Operands,
                                                  bool InvariantCond,
                                                  VPTransformState &State) {
@@ -4705,7 +4705,7 @@ void InnerLoopVectorizer::widenSelectInstruction(SelectInst &I,
     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);
   }
 }
@@ -7636,16 +7636,8 @@ void LoopVectorizationPlanner::buildVPlansWithVPRecipes(ElementCount MinVF,
 
   // If the tail is to be folded by masking, the primary induction variable, if
   // 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);
-      NeedDef.insert(Reduction.second.getLoopExitInstr());
-    }
-  }
+  if (CM.foldTailByMasking() && Legal->getPrimaryInduction())
+    NeedDef.insert(Legal->getPrimaryInduction());
 
   // Collect instructions from the original loop that will become trivially dead
   // in the vectorized loop. We don't need to vectorize these instructions. For
@@ -7858,8 +7850,8 @@ VPlanPtr LoopVectorizationPlanner::buildVPlanWithVPRecipes(
     for (auto &Reduction : Legal->getReductionVars()) {
       if (CM.isInLoopReduction(Reduction.first))
         continue;
-      VPValue *Phi = Plan->getVPValue(Reduction.first);
-      VPValue *Red = Plan->getVPValue(Reduction.second.getLoopExitInstr());
+      VPValue *Phi = Plan->getOrAddVPValue(Reduction.first);
+      VPValue *Red = Plan->getOrAddVPValue(Reduction.second.getLoopExitInstr());
       Builder.createNaryOp(Instruction::Select, {Cond, Red, Phi});
     }
   }
@@ -8003,7 +7995,8 @@ void VPWidenCallRecipe::execute(VPTransformState &State) {
 }
 
 void VPWidenSelectRecipe::execute(VPTransformState &State) {
-  State.ILV->widenSelectInstruction(Ingredient, *this, InvariantCond, State);
+  State.ILV->widenSelectInstruction(*cast<SelectInst>(getUnderlyingInstr()),
+                                    this, *this, InvariantCond, State);
 }
 
 void VPWidenRecipe::execute(VPTransformState &State) {

diff  --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 0656854c3b36..46ab2eae623c 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -108,6 +108,8 @@ VPValue *VPRecipeBase::toVPValue() {
     return V;
   if (auto *V = dyn_cast<VPWidenCallRecipe>(this))
     return V;
+  if (auto *V = dyn_cast<VPWidenSelectRecipe>(this))
+    return V;
   return nullptr;
 }
 
@@ -118,6 +120,8 @@ const VPValue *VPRecipeBase::toVPValue() const {
     return V;
   if (auto *V = dyn_cast<VPWidenCallRecipe>(this))
     return V;
+  if (auto *V = dyn_cast<VPWidenSelectRecipe>(this))
+    return V;
   return nullptr;
 }
 
@@ -843,8 +847,15 @@ void VPWidenCallRecipe::print(raw_ostream &O, const Twine &Indent,
 
 void VPWidenSelectRecipe::print(raw_ostream &O, const Twine &Indent,
                                 VPSlotTracker &SlotTracker) const {
-  O << "\"WIDEN-SELECT" << VPlanIngredient(&Ingredient)
-    << (InvariantCond ? " (condition is loop invariant)" : "");
+  O << "\"WIDEN-SELECT ";
+  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,

diff  --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index c70d82bfb137..4c7982c5fedf 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -877,10 +877,7 @@ class VPWidenCallRecipe : public VPRecipeBase, public VPValue, public VPUser {
 };
 
 /// 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;
@@ -889,7 +886,8 @@ class VPWidenSelectRecipe : public VPRecipeBase, public VPUser {
   template <typename IterT>
   VPWidenSelectRecipe(SelectInst &I, iterator_range<IterT> Operands,
                       bool InvariantCond)
-      : VPRecipeBase(VPWidenSelectSC), VPUser(Operands), Ingredient(I),
+      : VPRecipeBase(VPRecipeBase::VPWidenSelectSC),
+        VPValue(VPValue::VPVWidenSelectSC, &I), VPUser(Operands),
         InvariantCond(InvariantCond) {}
 
   ~VPWidenSelectRecipe() override = default;

diff  --git a/llvm/lib/Transforms/Vectorize/VPlanValue.h b/llvm/lib/Transforms/Vectorize/VPlanValue.h
index 503abec30c66..ce3b4b1452e4 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanValue.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanValue.h
@@ -78,7 +78,13 @@ class VPValue {
   /// 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;

diff  --git a/llvm/test/Transforms/LoopVectorize/icmp-uniforms.ll b/llvm/test/Transforms/LoopVectorize/icmp-uniforms.ll
index a5b57432ff6e..e8e80d19bc75 100644
--- a/llvm/test/Transforms/LoopVectorize/icmp-uniforms.ll
+++ b/llvm/test/Transforms/LoopVectorize/icmp-uniforms.ll
@@ -39,7 +39,7 @@ for.end:
 ; 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-SELECT ir<%s> = select ir<%cond0>, ir<10>, ir<20>\l"
 ; CHECK-NEXT:  ]
 define void @test() {
 entry:

diff  --git a/llvm/test/Transforms/LoopVectorize/optsize.ll b/llvm/test/Transforms/LoopVectorize/optsize.ll
index 048e5dd40ded..ddc4747141fd 100644
--- a/llvm/test/Transforms/LoopVectorize/optsize.ll
+++ b/llvm/test/Transforms/LoopVectorize/optsize.ll
@@ -341,6 +341,29 @@ for.end:                                        ; preds = %for.body
   ret void
 }
 
+; Make sure we do not crash while building the VPlan for the loop with the
+; select below.
+define i32 @PR48142(i32* %ptr.start, i32* %ptr.end) optsize {
+; CHECK-LABEL: PR48142
+; CHECK-NOT: vector.body
+entry:
+  br label %for.body
+
+for.body:
+  %i.014 = phi i32 [ 20, %entry ], [ %cond, %for.body ]
+  %ptr.iv = phi i32* [ %ptr.start, %entry ], [ %ptr.next, %for.body ]
+  %cmp4 = icmp slt i32 %i.014, 99
+  %cond = select i1 %cmp4, i32 99, i32 %i.014
+  store i32 0, i32* %ptr.iv
+  %ptr.next = getelementptr inbounds i32, i32* %ptr.iv, i64 1
+  %cmp.not = icmp eq i32* %ptr.next, %ptr.end
+  br i1 %cmp.not, label %exit, label %for.body
+
+exit:
+  %res = phi i32 [ %cond, %for.body ]
+  ret i32 %res
+}
+
 !llvm.module.flags = !{!0}
 !0 = !{i32 1, !"ProfileSummary", !1}
 !1 = !{!2, !3, !4, !5, !6, !7, !8, !9}


        


More information about the llvm-commits mailing list