[llvm] 6254b6d - [VPlan] Version VPValue names in VPSlotTracker. (#81411)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 15 04:27:49 PDT 2024
Author: Florian Hahn
Date: 2024-04-15T12:27:45+01:00
New Revision: 6254b6dd892728306ddfe657f10eb43a3799864d
URL: https://github.com/llvm/llvm-project/commit/6254b6dd892728306ddfe657f10eb43a3799864d
DIFF: https://github.com/llvm/llvm-project/commit/6254b6dd892728306ddfe657f10eb43a3799864d.diff
LOG: [VPlan] Version VPValue names in VPSlotTracker. (#81411)
This patch restructures the way names for printing VPValues are handled.
It moves the logic to generate names for printing to VPSlotTracker.
VPSlotTracker will now version names of the same underlying value if it
is used by multiple VPValues, by adding a .V suffix to the name.
This fixes cases where at the moment the same name is printed for
different VPValues.
PR: https://github.com/llvm/llvm-project/pull/81411
Added:
Modified:
llvm/lib/Transforms/Vectorize/VPlan.cpp
llvm/lib/Transforms/Vectorize/VPlanValue.h
llvm/test/Transforms/LoopVectorize/vplan-sink-scalars-and-merge.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 3e1069d82ddad6..999236ae84898b 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -1300,18 +1300,7 @@ void VPValue::replaceUsesWithIf(
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
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 << Tracker.getOrCreateName(this);
}
void VPUser::printOperands(raw_ostream &O, VPSlotTracker &SlotTracker) const {
@@ -1373,32 +1362,88 @@ VPInterleavedAccessInfo::VPInterleavedAccessInfo(VPlan &Plan,
visitRegion(Plan.getVectorLoopRegion(), Old2New, IAI);
}
-void VPSlotTracker::assignSlot(const VPValue *V) {
- if (V->getUnderlyingValue())
+void VPSlotTracker::assignName(const VPValue *V) {
+ assert(!VPValue2Name.contains(V) && "VPValue already has a name!");
+ auto *UV = V->getUnderlyingValue();
+ if (!UV) {
+ VPValue2Name[V] = (Twine("vp<%") + Twine(NextSlot) + ">").str();
+ NextSlot++;
return;
- assert(!Slots.contains(V) && "VPValue already has a slot!");
- Slots[V] = NextSlot++;
+ }
+
+ // Use the name of the underlying Value, wrapped in "ir<>", and versioned by
+ // appending ".Number" to the name if there are multiple uses.
+ std::string Name;
+ raw_string_ostream S(Name);
+ UV->printAsOperand(S, false);
+ assert(!Name.empty() && "Name cannot be empty.");
+ std::string BaseName = (Twine("ir<") + Name + Twine(">")).str();
+
+ // First assign the base name for V.
+ const auto &[A, _] = VPValue2Name.insert({V, BaseName});
+ // Integer or FP constants with
diff erent types will result in he same string
+ // due to stripping types.
+ if (V->isLiveIn() && isa<ConstantInt, ConstantFP>(UV))
+ return;
+
+ // If it is already used by C > 0 other VPValues, increase the version counter
+ // C and use it for V.
+ const auto &[C, UseInserted] = BaseName2Version.insert({BaseName, 0});
+ if (!UseInserted) {
+ C->second++;
+ A->second = (BaseName + Twine(".") + Twine(C->second)).str();
+ }
}
-void VPSlotTracker::assignSlots(const VPlan &Plan) {
+void VPSlotTracker::assignNames(const VPlan &Plan) {
if (Plan.VFxUF.getNumUsers() > 0)
- assignSlot(&Plan.VFxUF);
- assignSlot(&Plan.VectorTripCount);
+ assignName(&Plan.VFxUF);
+ assignName(&Plan.VectorTripCount);
if (Plan.BackedgeTakenCount)
- assignSlot(Plan.BackedgeTakenCount);
- assignSlots(Plan.getPreheader());
+ assignName(Plan.BackedgeTakenCount);
+ for (VPValue *LI : Plan.VPLiveInsToFree)
+ assignName(LI);
+ assignNames(Plan.getPreheader());
ReversePostOrderTraversal<VPBlockDeepTraversalWrapper<const VPBlockBase *>>
RPOT(VPBlockDeepTraversalWrapper<const VPBlockBase *>(Plan.getEntry()));
for (const VPBasicBlock *VPBB :
VPBlockUtils::blocksOnly<const VPBasicBlock>(RPOT))
- assignSlots(VPBB);
+ assignNames(VPBB);
}
-void VPSlotTracker::assignSlots(const VPBasicBlock *VPBB) {
+void VPSlotTracker::assignNames(const VPBasicBlock *VPBB) {
for (const VPRecipeBase &Recipe : *VPBB)
for (VPValue *Def : Recipe.definedValues())
- assignSlot(Def);
+ assignName(Def);
+}
+
+std::string VPSlotTracker::getOrCreateName(const VPValue *V) const {
+ std::string Name = VPValue2Name.lookup(V);
+ if (!Name.empty())
+ return Name;
+
+ // If no name was assigned, no VPlan was provided when creating the slot
+ // tracker or it is not reachable from the provided VPlan. This can happen,
+ // e.g. when trying to print a recipe that has not been inserted into a VPlan
+ // in a debugger.
+ // TODO: Update VPSlotTracker constructor to assign names to recipes &
+ // VPValues not associated with a VPlan, instead of constructing names ad-hoc
+ // here.
+ const VPRecipeBase *DefR = V->getDefiningRecipe();
+ (void)DefR;
+ assert((!DefR || !DefR->getParent() || !DefR->getParent()->getPlan()) &&
+ "VPValue defined by a recipe in a VPlan?");
+
+ // Use the underlying value's name, if there is one.
+ if (auto *UV = V->getUnderlyingValue()) {
+ std::string Name;
+ raw_string_ostream S(Name);
+ UV->printAsOperand(S, false);
+ return (Twine("ir<") + Name + ">").str();
+ }
+
+ return "<badref>";
}
bool vputils::onlyFirstLaneUsed(const VPValue *Def) {
diff --git a/llvm/lib/Transforms/Vectorize/VPlanValue.h b/llvm/lib/Transforms/Vectorize/VPlanValue.h
index 8b221d30e5254c..da3a768552fc5e 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanValue.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanValue.h
@@ -23,6 +23,7 @@
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/TinyPtrVector.h"
#include "llvm/ADT/iterator_range.h"
@@ -443,29 +444,36 @@ class VPDef {
class VPlan;
class VPBasicBlock;
-/// This class can be used to assign consecutive numbers to all VPValues in a
-/// VPlan and allows querying the numbering for printing, similar to the
+/// This class can be used to assign names to VPValues. For VPValues without
+/// underlying value, assign consecutive numbers and use those as names (wrapped
+/// in vp<>). Otherwise, use the name from the underlying value (wrapped in
+/// ir<>), appending a .V version number if there are multiple uses of the same
+/// name. Allows querying names for VPValues for printing, similar to the
/// ModuleSlotTracker for IR values.
class VPSlotTracker {
- DenseMap<const VPValue *, unsigned> Slots;
+ /// Keep track of versioned names assigned to VPValues with underlying IR
+ /// values.
+ DenseMap<const VPValue *, std::string> VPValue2Name;
+ /// Keep track of the next number to use to version the base name.
+ StringMap<unsigned> BaseName2Version;
+
+ /// Number to assign to the next VPValue without underlying value.
unsigned NextSlot = 0;
- void assignSlot(const VPValue *V);
- void assignSlots(const VPlan &Plan);
- void assignSlots(const VPBasicBlock *VPBB);
+ void assignName(const VPValue *V);
+ void assignNames(const VPlan &Plan);
+ void assignNames(const VPBasicBlock *VPBB);
public:
VPSlotTracker(const VPlan *Plan = nullptr) {
if (Plan)
- assignSlots(*Plan);
+ assignNames(*Plan);
}
- unsigned getSlot(const VPValue *V) const {
- auto I = Slots.find(V);
- if (I == Slots.end())
- return -1;
- return I->second;
- }
+ /// Returns the name assigned to \p V, if there is one, otherwise try to
+ /// construct one from the underlying value, if there's one; else return
+ /// <badref>.
+ std::string getOrCreateName(const VPValue *V) const;
};
} // namespace llvm
diff --git a/llvm/test/Transforms/LoopVectorize/vplan-sink-scalars-and-merge.ll b/llvm/test/Transforms/LoopVectorize/vplan-sink-scalars-and-merge.ll
index 0cacb02dc48914..108b78a70fa1a9 100644
--- a/llvm/test/Transforms/LoopVectorize/vplan-sink-scalars-and-merge.ll
+++ b/llvm/test/Transforms/LoopVectorize/vplan-sink-scalars-and-merge.ll
@@ -986,8 +986,8 @@ define void @sinking_requires_duplication(ptr %addr) {
; CHECK-NEXT: Successor(s): pred.store.if, pred.store.continue
; CHECK-EMPTY:
; CHECK-NEXT: pred.store.if:
-; CHECK-NEXT: REPLICATE ir<%gep> = getelementptr ir<%addr>, vp<[[STEPS]]>
-; CHECK-NEXT: REPLICATE store ir<1.000000e+01>, ir<%gep>
+; CHECK-NEXT: REPLICATE ir<%gep>.1 = getelementptr ir<%addr>, vp<[[STEPS]]>
+; CHECK-NEXT: REPLICATE store ir<1.000000e+01>, ir<%gep>.1
; CHECK-NEXT: Successor(s): pred.store.continue
; CHECK-EMPTY:
; CHECK-NEXT: pred.store.continue:
@@ -1129,8 +1129,8 @@ define void @ptr_induction_remove_dead_recipe(ptr %start, ptr %end) {
; CHECK-NEXT: Successor(s): pred.store.if, pred.store.continue
; CHECK-EMPTY:
; CHECK-NEXT: pred.store.if:
-; CHECK-NEXT: REPLICATE ir<%ptr.iv.next> = getelementptr inbounds vp<[[PTR_IV]]>, ir<-1>
-; CHECK-NEXT: REPLICATE store ir<95>, ir<%ptr.iv.next>
+; CHECK-NEXT: REPLICATE ir<%ptr.iv.next>.1 = getelementptr inbounds vp<[[PTR_IV]]>, ir<-1>
+; CHECK-NEXT: REPLICATE store ir<95>, ir<%ptr.iv.next>.1
; CHECK-NEXT: Successor(s): pred.store.continue
; CHECK-EMPTY:
; CHECK-NEXT: pred.store.continue:
More information about the llvm-commits
mailing list