[VPlan] Unify value/recipe printing after VPDef transition.

This patch unifies the way recipes and VPValues are printed after the
transition to VPDef.

VPSlotTracker has been updated to iterate over all recipes and all
their defined values to number those. There is no need to number
values in Value2VPValue.

It also updates a few places that only used slot numbers for
VPInstruction. All recipes now can produce numbered VPValues.

lib/Transforms/Vectorize/VPlan.cpp

@@ -71,22 +71,24 @@ VPValue::~VPValue() {
 }
 
 void VPValue::print(raw_ostream &OS, VPSlotTracker &SlotTracker) const {
-  if (const VPInstruction *Instr = dyn_cast<VPInstruction>(this))
-    Instr->print(OS, SlotTracker);
+  if (const VPRecipeBase *R = dyn_cast_or_null<VPRecipeBase>(Def))
+    R->print(OS, "", SlotTracker);
   else
     printAsOperand(OS, SlotTracker);
 }
 
 void VPValue::dump() const {
-  const VPInstruction *Instr = dyn_cast<VPInstruction>(this);
+  const VPRecipeBase *Instr = dyn_cast_or_null<VPRecipeBase>(this->Def);
   VPSlotTracker SlotTracker(
       (Instr && Instr->getParent()) ? Instr->getParent()->getPlan() : nullptr);
   print(dbgs(), SlotTracker);
   dbgs() << "\n";
 }
 
-void VPRecipeBase::dump() const {
-  VPSlotTracker SlotTracker(nullptr);
+void VPDef::dump() const {
+  const VPRecipeBase *Instr = dyn_cast_or_null<VPRecipeBase>(this);
+  VPSlotTracker SlotTracker(
+      (Instr && Instr->getParent()) ? Instr->getParent()->getPlan() : nullptr);
   print(dbgs(), "", SlotTracker);
   dbgs() << "\n";
 }
@@ -500,18 +502,15 @@ void VPInstruction::execute(VPTransformState &State) {
     generateInstruction(State, Part);
 }
 
+void VPInstruction::dump() const {
+  VPSlotTracker SlotTracker(getParent()->getPlan());
+  print(dbgs(), "", SlotTracker);
+}
+
 void VPInstruction::print(raw_ostream &O, const Twine &Indent,
                           VPSlotTracker &SlotTracker) const {
   O << "EMIT ";
-  print(O, SlotTracker);
-}
 
-void VPInstruction::print(raw_ostream &O) const {
-  VPSlotTracker SlotTracker(getParent()->getPlan());
-  print(O, SlotTracker);
-}
-
-void VPInstruction::print(raw_ostream &O, VPSlotTracker &SlotTracker) const {
   if (hasResult()) {
     printAsOperand(O, SlotTracker);
     O << " = ";
@@ -1087,13 +1086,6 @@ VPInterleavedAccessInfo::VPInterleavedAccessInfo(VPlan &Plan,
 
 void VPSlotTracker::assignSlot(const VPValue *V) {
   assert(Slots.find(V) == Slots.end() && "VPValue already has a slot!");
-  const Value *UV = V->getUnderlyingValue();
-  if (UV)
-    return;
-  const auto *VPI = dyn_cast<VPInstruction>(V);
-  if (VPI && !VPI->hasResult())
-    return;
-
   Slots[V] = NextSlot++;
 }
 
@@ -1112,10 +1104,8 @@ void VPSlotTracker::assignSlots(const VPRegionBlock *Region) {
 
 void VPSlotTracker::assignSlots(const VPBasicBlock *VPBB) {
   for (const VPRecipeBase &Recipe : *VPBB) {
-    if (const auto *VPI = dyn_cast<VPInstruction>(&Recipe))
-      assignSlot(VPI);
-    else if (const auto *VPIV = dyn_cast<VPWidenCanonicalIVRecipe>(&Recipe))
-      assignSlot(VPIV->getVPValue());
+    for (VPValue *Def : Recipe.definedValues())
+      assignSlot(Def);
   }
 }
 
@@ -1124,10 +1114,6 @@ void VPSlotTracker::assignSlots(const VPlan &Plan) {
   for (const VPValue *V : Plan.VPExternalDefs)
     assignSlot(V);
 
-  for (auto &E : Plan.Value2VPValue)
-    if (!isa<VPInstruction>(E.second))
-      assignSlot(E.second);
-
   for (const VPValue *V : Plan.VPCBVs)
     assignSlot(V);
 

lib/Transforms/Vectorize/VPlan.h

@@ -645,13 +645,6 @@ public:
   /// this VPRecipe, thereby "executing" the VPlan.
   virtual void execute(struct VPTransformState &State) = 0;
 
-  /// Each recipe prints itself.
-  virtual void print(raw_ostream &O, const Twine &Indent,
-                     VPSlotTracker &SlotTracker) const = 0;
-
-  /// Dump the recipe to stderr (for debugging).
-  void dump() const;
-
   /// Insert an unlinked recipe into a basic block immediately before
   /// the specified recipe.
   void insertBefore(VPRecipeBase *InsertPos);
@@ -777,13 +770,12 @@ public:
   /// provided.
   void execute(VPTransformState &State) override;
 
-  /// Print the Recipe.
+  /// Print the VPInstruction to \p O.
   void print(raw_ostream &O, const Twine &Indent,
              VPSlotTracker &SlotTracker) const override;
 
-  /// Print the VPInstruction.
-  void print(raw_ostream &O) const;
-  void print(raw_ostream &O, VPSlotTracker &SlotTracker) const;
+  /// Print the VPInstruction to dbgs() (for debugging).
+  void dump() const;
 
   /// Return true if this instruction may modify memory.
   bool mayWriteToMemory() const {
@@ -1244,7 +1236,7 @@ public:
              VPSlotTracker &SlotTracker) const override {
     O << " +\n" << Indent << "\"BRANCH-ON-MASK ";
     if (VPValue *Mask = getMask())
-      Mask->print(O, SlotTracker);
+      Mask->printAsOperand(O, SlotTracker);
     else
       O << " All-One";
     O << "\\l\"";

lib/Transforms/Vectorize/VPlanSLP.cpp

@@ -466,8 +466,8 @@ VPInstruction *VPlanSlp::buildGraph(ArrayRef<VPValue *> Values) {
   auto *VPI = new VPInstruction(Opcode, CombinedOperands);
   VPI->setUnderlyingInstr(cast<VPInstruction>(Values[0])->getUnderlyingInstr());
 
-  LLVM_DEBUG(dbgs() << "Create VPInstruction "; VPI->print(dbgs());
-             cast<VPInstruction>(Values[0])->print(dbgs()); dbgs() << "\n");
+  LLVM_DEBUG(dbgs() << "Create VPInstruction " << *VPI << " "
+                    << *cast<VPInstruction>(Values[0]) << "\n");
   addCombined(Values, VPI);
   return VPI;
 }

lib/Transforms/Vectorize/VPlanValue.h

@@ -327,6 +327,8 @@ public:
 
   /// Returns an ArrayRef of the values defined by the VPDef.
   ArrayRef<VPValue *> definedValues() { return DefinedValues; }
+  /// Returns an ArrayRef of the values defined by the VPDef.
+  ArrayRef<VPValue *> definedValues() const { return DefinedValues; }
 
   /// Returns the number of values defined by the VPDef.
   unsigned getNumDefinedValues() const { return DefinedValues.size(); }
@@ -335,6 +337,13 @@ public:
   /// 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.
   unsigned getVPDefID() const { return SubclassID; }
+
+  /// Dump the VPDef to stderr (for debugging).
+  void dump() const;
+
+  /// Each concrete VPDef prints itself.
+  virtual void print(raw_ostream &O, const Twine &Indent,
+                     VPSlotTracker &SlotTracker) const = 0;
 };
 
 class VPlan;
@@ -356,7 +365,7 @@ class VPSlotTracker {
   void assignSlots(const VPlan &Plan);
 
 public:
-  VPSlotTracker(const VPlan *Plan) {
+  VPSlotTracker(const VPlan *Plan = nullptr) {
     if (Plan)
       assignSlots(*Plan);
   }

test/Transforms/LoopVectorize/vplan-printing.ll

@@ -123,8 +123,8 @@ define void @print_replicate_predicated_phi(i64 %n, i64* %x) {
 ;
 ; CHECK:       N7 [label =
 ; CHECK-NEXT:    "for.inc:\n" +
-; CHECK-NEXT:      "EMIT vp<%0> = not ir<%cmp>\l" +
-; CHECK-NEXT:      "BLEND %d = ir<0>/vp<%0> ir<%tmp4>/ir<%cmp>\l" +
+; CHECK-NEXT:      "EMIT vp<%4> = not ir<%cmp>\l" +
+; CHECK-NEXT:      "BLEND %d = ir<0>/vp<%4> ir<%tmp4>/ir<%cmp>\l" +
 ; CHECK-NEXT:      "CLONE ir<%idx> = getelementptr ir<%x>, ir<%i>\l" +
 ; CHECK-NEXT:      "WIDEN store ir<%idx>, ir<%d>\l"
 ; CHECK-NEXT:  ]

unittests/Transforms/Vectorize/VPlanTest.cpp

@@ -346,9 +346,10 @@ TEST(VPBasicBlockTest, print) {
   {
     std::string I3Dump;
     raw_string_ostream OS(I3Dump);
-    I3->print(OS);
+    VPSlotTracker SlotTracker;
+    I3->print(OS, "", SlotTracker);
     OS.flush();
-    EXPECT_EQ("br <badref> <badref>", I3Dump);
+    EXPECT_EQ("EMIT br <badref> <badref>", I3Dump);
   }
 
   VPlan Plan;
@@ -370,8 +371,8 @@ compound=true
   N0 -> N1 [ label=""]
   N1 [label =
     ":\n" +
-      "EMIT vp<%2> = mul vp<%1> vp<%0>\l" +
-      "EMIT ret vp<%2>\l"
+      "EMIT vp<%3> = mul vp<%1> vp<%0>\l" +
+      "EMIT ret vp<%3>\l"
   ]
 }
 )";
@@ -380,9 +381,10 @@ compound=true
   {
     std::string I3Dump;
     raw_string_ostream OS(I3Dump);
-    I3->print(OS);
+    VPSlotTracker SlotTracker(&Plan);
+    I3->print(OS, "", SlotTracker);
     OS.flush();
-    EXPECT_EQ("br vp<%0> vp<%1>", I3Dump);
+    EXPECT_EQ("EMIT br vp<%0> vp<%1>", I3Dump);
   }
 
   {
@@ -390,7 +392,7 @@ compound=true
     raw_string_ostream OS(I4Dump);
     OS << *I4;
     OS.flush();
-    EXPECT_EQ("vp<%2> = mul vp<%1> vp<%0>", I4Dump);
+    EXPECT_EQ("EMIT vp<%3> = mul vp<%1> vp<%0>", I4Dump);
   }
 }
 
@@ -579,6 +581,62 @@ TEST(VPRecipeTest, CastVPWidenMemoryInstructionRecipeToVPUserAndVPDef) {
   delete Load;
 }
 
+TEST(VPRecipeTest, dump) {
+  VPlan Plan;
+  VPBasicBlock *VPBB1 = new VPBasicBlock();
+  Plan.setEntry(VPBB1);
+
+  LLVMContext C;
+
+  IntegerType *Int32 = IntegerType::get(C, 32);
+  auto *AI =
+      BinaryOperator::CreateAdd(UndefValue::get(Int32), UndefValue::get(Int32));
+  AI->setName("a");
+  SmallVector<VPValue *, 2> Args;
+  VPValue *ExtVPV1 = new VPValue();
+  VPValue *ExtVPV2 = new VPValue();
+  Plan.addExternalDef(ExtVPV1);
+  Plan.addExternalDef(ExtVPV2);
+  Args.push_back(ExtVPV1);
+  Args.push_back(ExtVPV2);
+  VPWidenRecipe *WidenR =
+      new VPWidenRecipe(*AI, make_range(Args.begin(), Args.end()));
+  VPBB1->appendRecipe(WidenR);
+
+  {
+    // Use EXPECT_EXIT to capture stderr and compare against expected output.
+    //
+    // Test VPValue::dump().
+    VPValue *VPV = WidenR;
+    EXPECT_EXIT(
+        {
+          VPV->dump();
+          exit(0);
+        },
+        testing::ExitedWithCode(0), "WIDEN ir<%a> = add vp<%0>, vp<%1>");
+
+    // Test VPRecipeBase::dump().
+    VPRecipeBase *R = WidenR;
+    EXPECT_EXIT(
+        {
+          R->dump();
+          exit(0);
+        },
+        testing::ExitedWithCode(0), "WIDEN ir<%a> = add vp<%0>, vp<%1>");
+
+    // Test VPDef::dump().
+    VPDef *D = WidenR;
+    EXPECT_EXIT(
+        {
+          D->dump();
+          exit(0);
+        },
+        testing::ExitedWithCode(0), "WIDEN ir<%a> = add vp<%0>, vp<%1>");
+  }
+
+  delete AI;
+}
+
 TEST(VPRecipeTest, CastVPReductionRecipeToVPUser) {
   LLVMContext C;