[VPlan] Use underlying value for printing, if available.

[fhahn]

Summary

When the an underlying value is available, we can use its name for
printing, as discussed in D73078.

[Ayal]

llvm/lib/Transforms/Vectorize/VPlan.cpp

590

If now every VPValue that wraps a Value clearly says so in its name, does it hopefully make this "%vpA := %irX" part of "where:" redundant?

820

Even if UV does not have a name, could we still indicate that VPValue wraps it; possibly by calling UV->printAsOperand()?

[fhahn]

Updated to use Value::printAsOperand for all VPValues with underlying values.

The new format uses '%vp.ir.<underlying value name>' for non-constant underlying values. It seems we have to do a bit of extra work to drop the % from the printed name. getName() would just return the string, but we also want to include implicitly numbered IR values.

For constant values, it prints 'ir constant <constant value>'. Not sure if calling out that it's an IR constant is required though.

It also adds a hasResult() method to VPInstruction, because printAsOperand does not make sense on IR values of type void. The method is subsequently used to avoid printing a result name for VPInstruction with underlying values of void ty.

[fhahn]

llvm/lib/Transforms/Vectorize/VPlan.cpp

590

I can drop that in a follow-up, if that's ok?

820

Done! It required to add special handling for constants and also a bit of code to drop the % printed by printAsOperand, but now all underlying IR values should be printed consistently. A full list of the required changes can be found in the update comment.

[Ayal]

llvm/lib/Transforms/Vectorize/VPlan.cpp

408

Better call printAsOperand(O, SlotTracker) only when needed, i.e.,

if (hasResult()) {
  printAsOperand(O, SlotTracker);
  O << " = ";
}

590

Sure, by all means.

820

Thanks!
Could this be simplified, by always printing "ir<X>" where X is whatever printAsOperand provides, which could be ir<%tmp>, ir<%5>, ir<7>, ir<void>, letting the caller worry about not printing void?

And for VPValues w/o ingredients, print "vp<%x>" where x is the number assigned by SlotTracker? Currently this should be only BackedgeTakenCount, which ought to be assigned a meaningful name...

889

Suffice to check now instead if (!UV)

llvm/lib/Transforms/Vectorize/VPlan.h

739

Better look at the VPInstruction's Opcode to determine if it has a result?

1491

Better record the underlying ingredient to be used when printing, by doing new VPValue(V) instead.

[fhahn]

Address Ayal's comments, thanks!

[fhahn]

llvm/lib/Transforms/Vectorize/VPlan.cpp

820

Yes that format simplifies things. I think it looks OK as well.

889

If V is a VPInstruction, we should also only number instructions with results. Should be fixed

llvm/lib/Transforms/Vectorize/VPlan.h

739

I wanted to avoid to have such a check here initially, but it does not look too bad. Unless I missed an instruction, I think calls should be the only instruction where the opcode does not determine if it has a result or not. But just assuming calls have results should be fine.

Plenty of instructions without result are not likely to be VPInstructions, but I guess it does not hurt to include them here.

1491

Also better as a follow up?

[Ayal]

This looks good to me, thanks!
Added some minor final comments.

llvm/lib/Transforms/Vectorize/VPlan.cpp

890

Right. May be easier to read if reversed into an early-exiting

if (UV || (VPI && !VPI->hasResult()))
  return;

At some point it would be good to name VPValues (w/o underlying Values). These would need SlotTracker's attention, to break name reuses.

llvm/lib/Transforms/Vectorize/VPlan.h

739

Sure. Some of these cannot be processed by LV.

Would be good to switch back to something like "!getType()->isVoidTy()", once [VP]Types are introduced into VPlan/VPValue.

1491

Very well.

[fhahn]

Thanks. I'll prepare the follow-ups shortly.

llvm/lib/Transforms/Vectorize/VPlan.h

739

Sounds good!

llvm/lib/Transforms/Vectorize/VPlan.h

public:
 
   /// Return true if this instruction may modify memory.
   bool mayWriteToMemory() const {
     // TODO: we can use attributes of the called function to rule out memory
     //       modifications.
     return Opcode == Instruction::Store || Opcode == Instruction::Call ||
            Opcode == Instruction::Invoke || Opcode == SLPStore;
   }
+
+  bool hasResult() const {
+    // CallInst may or may not have a result, depending on the called function.
+    // Conservatively return calls have results for now.
+    switch (getOpcode()) {
+    case Instruction::Ret:
+    case Instruction::Br:
+    case Instruction::Store:
+    case Instruction::Switch:
+    case Instruction::IndirectBr:
+    case Instruction::Resume:
+    case Instruction::CatchRet:
+    case Instruction::Unreachable:
+    case Instruction::Fence:
+    case Instruction::AtomicRMW:
+      return false;
+    default:
+      return true;
+    }
+  }
 };
 
 /// VPWidenRecipe is a recipe for producing a copy of vector type for each
 /// Instruction in its ingredients independently, in order. This recipe covers
 /// most of the traditional vectorization cases where each ingredient transforms
 /// into a vectorized version of itself.
 class VPWidenRecipe : public VPRecipeBase {
 private:
public:
   /// Add \p CBV to the vector of condition bit values.
   void addCBV(VPValue *CBV) {
     VPCBVs.push_back(CBV);
   }
 
   void addVPValue(Value *V) {
     assert(V && "Trying to add a null Value to VPlan");
     assert(!Value2VPValue.count(V) && "Value already exists in VPlan");
     Value2VPValue[V] = new VPValue();
   }
 
   VPValue *getVPValue(Value *V) {
     assert(V && "Trying to get the VPValue of a null Value");
     assert(Value2VPValue.count(V) && "Value does not exist in VPlan");
     return Value2VPValue[V];
   }
 

llvm/lib/Transforms/Vectorize/VPlan.cpp

 }
 
 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 << " = ";
+  }
 
   switch (getOpcode()) {
   case VPInstruction::Not:
     O << "not";
     break;
   case VPInstruction::ICmpULE:
     O << "icmp ule";
     break;
if (!Plan.getName().empty())
     OS << "\\n" << DOT::EscapeString(Plan.getName());
   if (!Plan.Value2VPValue.empty() || Plan.BackedgeTakenCount) {
     OS << ", where:";
     if (Plan.BackedgeTakenCount) {
       OS << "\\n";
       Plan.BackedgeTakenCount->print(OS, SlotTracker);
       OS << " := BackedgeTakenCount";
     }
     for (auto Entry : Plan.Value2VPValue) {
       OS << "\\n";
       Entry.second->print(OS, SlotTracker);
       OS << DOT::EscapeString(" := ");
       Entry.first->printAsOperand(OS, false);
     }
   }
   OS << "\"]\n";
   OS << "node [shape=rect, fontname=Courier, fontsize=30]\n";

 void VPValue::replaceAllUsesWith(VPValue *New) {
   for (VPUser *User : users())
     for (unsigned I = 0, E = User->getNumOperands(); I < E; ++I)
       if (User->getOperand(I) == this)
         User->setOperand(I, New);
 }
 
 void VPValue::printAsOperand(raw_ostream &OS, VPSlotTracker &Tracker) const {
+  if (const Value *UV = getUnderlyingValue()) {
+    OS << "ir<";
+    UV->printAsOperand(OS, false);
+    OS << ">";
+    return;
+  }
+
   unsigned Slot = Tracker.getSlot(this);
   if (Slot == unsigned(-1))
     OS << "<badref>";
   else
-    OS << "%vp" << Tracker.getSlot(this);
+    OS << "vp<%" << Tracker.getSlot(this) << ">";
 }
 
 void VPInterleavedAccessInfo::visitRegion(VPRegionBlock *Region,
                                           Old2NewTy &Old2New,
                                           InterleavedAccessInfo &IAI) {
   ReversePostOrderTraversal<VPBlockBase *> RPOT(Region->getEntry());
   for (VPBlockBase *Base : RPOT) {
     visitBlock(Base, Old2New, IAI);

 VPInterleavedAccessInfo::VPInterleavedAccessInfo(VPlan &Plan,
                                                  InterleavedAccessInfo &IAI) {
   Old2NewTy Old2New;
   visitRegion(cast<VPRegionBlock>(Plan.getEntry()), Old2New, IAI);
 }
 
 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++;
 }
 
 void VPSlotTracker::assignSlots(const VPBlockBase *VPBB) {
   if (auto *Region = dyn_cast<VPRegionBlock>(VPBB))
     assignSlots(Region);
   else
     assignSlots(cast<VPBasicBlock>(VPBB));
 }
 
 void VPSlotTracker::assignSlots(const VPRegionBlock *Region) {

llvm/lib/Transforms/Vectorize/VPlanValue.h

 // flow into, within and out of the VPlan. VPValues can stand for live-ins
 // coming from the input IR, instructions which VPlan will generate if executed
 // and live-outs which the VPlan will need to fix accordingly.
 class VPValue {
   friend class VPBuilder;
   friend class VPlanTransforms;
   friend class VPBasicBlock;
   friend class VPInterleavedAccessInfo;
+  friend class VPSlotTracker;
 
 private:
   const unsigned char SubclassID; ///< Subclass identifier (for isa/dyn_cast).
 
   SmallVector<VPUser *, 1> Users;
 
 protected:
   // Hold the underlying Value, if any, attached to this VPValue.
   Value *UnderlyingVal;
 
   VPValue(const unsigned char SC, Value *UV = nullptr)
       : SubclassID(SC), UnderlyingVal(UV) {}
 
   // DESIGN PRINCIPLE: Access to the underlying IR must be strictly limited to
   // the front-end and back-end of VPlan so that the middle-end is as
   // independent as possible of the underlying IR. We grant access to the
   // underlying IR using friendship. In that way, we should be able to use VPlan
   // for multiple underlying IRs (Polly?) by providing a new VPlan front-end,
   // back-end and analysis information for the new IR.
 
   /// Return the underlying Value attached to this VPValue.
   Value *getUnderlyingValue() { return UnderlyingVal; }
+  const Value *getUnderlyingValue() const { return UnderlyingVal; }
 
   // Set \p Val as the underlying Value of this VPValue.
   void setUnderlyingValue(Value *Val) {
     assert(!UnderlyingVal && "Underlying Value is already set.");
     UnderlyingVal = Val;
   }
 
 public:

llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp

 //===- llvm/unittest/Transforms/Vectorize/VPlanHCFGTest.cpp ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "../lib/Transforms/Vectorize/VPlan.h"
 #include "../lib/Transforms/Vectorize/VPlanTransforms.h"
 #include "VPlanTestBase.h"
 #include "gtest/gtest.h"
+#include <string>
 
 namespace llvm {
 namespace {
 
 class VPlanHCFGTest : public VPlanTestBase {};
 
 TEST_F(VPlanHCFGTest, testBuildHCFGInnerLoop) {
   const char *ModuleString =
TEST_F(VPlanHCFGTest, testBuildHCFGInnerLoop) {
   EXPECT_EQ(Phi, IndvarAdd->getOperand(0));
 
   VPInstruction *ICmp = dyn_cast<VPInstruction>(&*Iter++);
   EXPECT_EQ(Instruction::ICmp, ICmp->getOpcode());
   EXPECT_EQ(2u, ICmp->getNumOperands());
   EXPECT_EQ(IndvarAdd, ICmp->getOperand(0));
   EXPECT_EQ(VecBB->getCondBit(), ICmp);
 
+  std::string FullDump;
+  raw_string_ostream(FullDump) << *Plan;
+  EXPECT_EQ(R"(digraph VPlan {
+graph [labelloc=t, fontsize=30; label="Vectorization Plan"]
+node [shape=rect, fontname=Courier, fontsize=30]
+edge [fontname=Courier, fontsize=30]
+compound=true
+  subgraph cluster_N0 {
+    fontname=Courier
+    label="\<x1\> TopRegion"
+    N1 [label =
+      "entry:\n"
+    ]
+    N1 -> N2 [ label=""]
+    N2 [label =
+      "for.body:\n" +
+        "EMIT ir<%indvars.iv> = phi ir<0> ir<%indvars.iv.next>\l" +
+        "EMIT ir<%arr.idx> = getelementptr ir<%A> ir<%indvars.iv>\l" +
+        "EMIT ir<%l1> = load ir<%arr.idx>\l" +
+        "EMIT ir<%res> = add ir<%l1> ir<10>\l" +
+        "EMIT store ir<%res> ir<%arr.idx>\l" +
+        "EMIT ir<%indvars.iv.next> = add ir<%indvars.iv> ir<1>\l" +
+        "EMIT ir<%exitcond> = icmp ir<%indvars.iv.next> ir<%N>\l" +
+         "CondBit: ir<%exitcond> (for.body)\l"
+    ]
+    N2 -> N2 [ label="T"]
+    N2 -> N3 [ label="F"]
+    N3 [label =
+      "for.end:\n" +
+        "EMIT ret\l"
+    ]
+  }
+}
+)",
+            FullDump);
+
   LoopVectorizationLegality::InductionList Inductions;
   SmallPtrSet<Instruction *, 1> DeadInstructions;
   VPlanTransforms::VPInstructionsToVPRecipes(LI->getLoopFor(LoopHeader), Plan,
                                              Inductions, DeadInstructions);
 }
 
 TEST_F(VPlanHCFGTest, testVPInstructionToVPRecipesInner) {
   const char *ModuleString =

llvm/unittests/Transforms/Vectorize/VPlanTest.cpp

TEST(VPBasicBlockTest, getPlan) {
     EXPECT_EQ(&Plan, R2->getPlan());
     EXPECT_EQ(&Plan, R2BB1->getPlan());
     EXPECT_EQ(&Plan, R2BB2->getPlan());
     EXPECT_EQ(&Plan, VPBB2->getPlan());
   }
 }
 
 TEST(VPBasicBlockTest, print) {
-  VPInstruction *I1 = new VPInstruction(10, {});
-  VPInstruction *I2 = new VPInstruction(1, {I1});
-  VPInstruction *I3 = new VPInstruction(2, {I1, I2});
+  VPInstruction *I1 = new VPInstruction(Instruction::Add, {});
+  VPInstruction *I2 = new VPInstruction(Instruction::Sub, {I1});
+  VPInstruction *I3 = new VPInstruction(Instruction::Br, {I1, I2});
 
   VPBasicBlock *VPBB1 = new VPBasicBlock();
   VPBB1->appendRecipe(I1);
   VPBB1->appendRecipe(I2);
   VPBB1->appendRecipe(I3);
 
-  VPInstruction *I4 = new VPInstruction(4, {I3, I2});
-  VPInstruction *I5 = new VPInstruction(5, {I1});
+  VPInstruction *I4 = new VPInstruction(Instruction::Mul, {I2, I1});
+  VPInstruction *I5 = new VPInstruction(Instruction::Ret, {I4});
   VPBasicBlock *VPBB2 = new VPBasicBlock();
   VPBB2->appendRecipe(I4);
   VPBB2->appendRecipe(I5);
 
   VPBlockUtils::connectBlocks(VPBB1, VPBB2);
 
   // Check printing an instruction without associated VPlan.
   {
     std::string I3Dump;
     raw_string_ostream OS(I3Dump);
     I3->print(OS);
     OS.flush();
-    EXPECT_EQ("<badref> = br <badref> <badref>", I3Dump);
+    EXPECT_EQ("br <badref> <badref>", I3Dump);
   }
 
   VPlan Plan;
   Plan.setEntry(VPBB1);
   std::string FullDump;
   raw_string_ostream(FullDump) << Plan;
 
   EXPECT_EQ(R"(digraph VPlan {
 graph [labelloc=t, fontsize=30; label="Vectorization Plan"]
 node [shape=rect, fontname=Courier, fontsize=30]
 edge [fontname=Courier, fontsize=30]
 compound=true
   N0 [label =
     ":\n" +
-      "EMIT %vp0 = catchswitch\l" +
-      "EMIT %vp1 = ret %vp0\l" +
-      "EMIT %vp2 = br %vp0 %vp1\l"
+      "EMIT vp<%0> = add\l" +
+      "EMIT vp<%1> = sub vp<%0>\l" +
+      "EMIT br vp<%0> vp<%1>\l"
   ]
   N0 -> N1 [ label=""]
   N1 [label =
     ":\n" +
-      "EMIT %vp3 = indirectbr %vp2 %vp1\l" +
-      "EMIT %vp4 = invoke %vp0\l"
+      "EMIT vp<%2> = mul vp<%1> vp<%0>\l" +
+      "EMIT ret vp<%2>\l"
   ]
 }
 )",
             FullDump);
 
   {
     std::string I3Dump;
     raw_string_ostream OS(I3Dump);
     I3->print(OS);
     OS.flush();
-    EXPECT_EQ("%vp2 = br %vp0 %vp1", I3Dump);
+    EXPECT_EQ("br vp<%0> vp<%1>", I3Dump);
   }
 
   {
-    std::string I2Dump;
-    raw_string_ostream OS(I2Dump);
-    OS << *I2;
+    std::string I4Dump;
+    raw_string_ostream OS(I4Dump);
+    OS << *I4;
     OS.flush();
-    EXPECT_EQ("%vp1 = ret %vp0", I2Dump);
+    EXPECT_EQ("vp<%2> = mul vp<%1> vp<%0>", I4Dump);
   }
 }
 
 } // namespace
 } // namespace llvm