[llvm] 668045e - [VPlan] Unify Value2VPValue and VPExternalDefs maps (NFCI).
Florian Hahn via llvm-commits
llvm-commits at lists.llvm.org
Sun Apr 16 07:38:48 PDT 2023
Author: Florian Hahn
Date: 2023-04-16T15:38:31+01:00
New Revision: 668045eb77628be13e448ffbb855473ffca1cc43
URL: https://github.com/llvm/llvm-project/commit/668045eb77628be13e448ffbb855473ffca1cc43
DIFF: https://github.com/llvm/llvm-project/commit/668045eb77628be13e448ffbb855473ffca1cc43.diff
LOG: [VPlan] Unify Value2VPValue and VPExternalDefs maps (NFCI).
Before this patch, a VPlan contained 2 mappings for Values -> VPValue:
1) Value2VPValue and 2) VPExternalDefs.
This duplication is unnecessary and there are already cases where
external defs are added to Value2VPValue. This patch replaces all uses
of VPExternalDefs with Value2VPValue.
It clarifies the naming of getOrAddVPValue (to getOrAddExternalVPValue)
and addVPValue (to addExternalVPValue).
At the moment, this is NFC, but will enable additional simplifications
in D147783.
Depends on D147891.
Reviewed By: Ayal
Differential Revision: https://reviews.llvm.org/D147892
Added:
Modified:
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
llvm/lib/Transforms/Vectorize/VPlan.cpp
llvm/lib/Transforms/Vectorize/VPlan.h
llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp
llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index c419df8072650..b6a48fd253d1a 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -8146,7 +8146,7 @@ VPValue *VPRecipeBuilder::createEdgeMask(BasicBlock *Src, BasicBlock *Dst,
if (OrigLoop->isLoopExiting(Src))
return EdgeMaskCache[Edge] = SrcMask;
- VPValue *EdgeMask = Plan.getOrAddVPValue(BI->getCondition());
+ VPValue *EdgeMask = Plan.getVPValueOrAddLiveIn(BI->getCondition());
assert(EdgeMask && "No Edge Mask found for condition");
if (BI->getSuccessor(0) != Dst)
@@ -8157,7 +8157,7 @@ VPValue *VPRecipeBuilder::createEdgeMask(BasicBlock *Src, BasicBlock *Dst,
// 'select i1 SrcMask, i1 EdgeMask, i1 false'.
// The select version does not introduce new UB if SrcMask is false and
// EdgeMask is poison. Using 'and' here introduces undefined behavior.
- VPValue *False = Plan.getOrAddVPValue(
+ VPValue *False = Plan.getVPValueOrAddLiveIn(
ConstantInt::getFalse(BI->getCondition()->getType()));
EdgeMask =
Builder.createSelect(SrcMask, EdgeMask, False, BI->getDebugLoc());
@@ -8353,7 +8353,7 @@ VPWidenIntOrFpInductionRecipe *VPRecipeBuilder::tryToOptimizeInductionTruncate(
auto *Phi = cast<PHINode>(I->getOperand(0));
const InductionDescriptor &II = *Legal->getIntOrFpInductionDescriptor(Phi);
- VPValue *Start = Plan.getOrAddVPValue(II.getStartValue());
+ VPValue *Start = Plan.getVPValueOrAddLiveIn(II.getStartValue());
return createWidenInductionRecipes(Phi, I, Start, II, CM, Plan,
*PSE.getSE(), *OrigLoop, Range);
}
@@ -8486,7 +8486,7 @@ VPWidenCallRecipe *VPRecipeBuilder::tryToWidenCall(CallInst *CI,
if (Legal->isMaskRequired(CI))
Mask = createBlockInMask(CI->getParent(), *Plan);
else
- Mask = Plan->getOrAddVPValue(ConstantInt::getTrue(
+ Mask = Plan->getVPValueOrAddLiveIn(ConstantInt::getTrue(
IntegerType::getInt1Ty(Variant->getFunctionType()->getContext())));
VFShape Shape = VFShape::get(*CI, VariantVF, /*HasGlobalPred=*/true);
@@ -8538,8 +8538,8 @@ VPRecipeBase *VPRecipeBuilder::tryToWiden(Instruction *I,
if (CM.isPredicatedInst(I)) {
SmallVector<VPValue *> Ops(Operands.begin(), Operands.end());
VPValue *Mask = createBlockInMask(I->getParent(), *Plan);
- VPValue *One =
- Plan->getOrAddExternalDef(ConstantInt::get(I->getType(), 1u, false));
+ VPValue *One = Plan->getVPValueOrAddLiveIn(
+ ConstantInt::get(I->getType(), 1u, false));
auto *SafeRHS =
new VPInstruction(Instruction::Select, {Mask, Ops[1], One},
I->getDebugLoc());
@@ -8760,7 +8760,7 @@ void LoopVectorizationPlanner::buildVPlansWithVPRecipes(ElementCount MinVF,
static void addCanonicalIVRecipes(VPlan &Plan, Type *IdxTy, DebugLoc DL,
TailFoldingStyle Style) {
Value *StartIdx = ConstantInt::get(IdxTy, 0);
- auto *StartV = Plan.getOrAddVPValue(StartIdx);
+ auto *StartV = Plan.getVPValueOrAddLiveIn(StartIdx);
// Add a VPCanonicalIVPHIRecipe starting at 0 to the header.
auto *CanonicalIVPHI = new VPCanonicalIVPHIRecipe(StartV, DL);
@@ -8876,7 +8876,7 @@ static void addUsersInExitBlock(VPBasicBlock *HeaderVPBB,
for (PHINode &ExitPhi : ExitBB->phis()) {
Value *IncomingValue =
ExitPhi.getIncomingValueForBlock(ExitingBB);
- VPValue *V = Plan.getOrAddVPValue(IncomingValue, true);
+ VPValue *V = Plan.getVPValueOrAddLiveIn(IncomingValue);
Plan.addLiveOut(&ExitPhi, V);
}
}
@@ -8987,7 +8987,7 @@ std::optional<VPlanPtr> LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes(
SmallVector<VPValue *, 4> Operands;
auto *Phi = dyn_cast<PHINode>(Instr);
if (Phi && Phi->getParent() == OrigLoop->getHeader()) {
- Operands.push_back(Plan->getOrAddVPValue(
+ Operands.push_back(Plan->getVPValueOrAddLiveIn(
Phi->getIncomingValueForBlock(OrigLoop->getLoopPreheader())));
} else {
auto OpRange = Plan->mapToVPValues(Instr->operands());
@@ -10477,7 +10477,7 @@ bool LoopVectorizePass::processLoop(Loop *L) {
IndPhi, *ID, {EPI.MainLoopIterationCountCheck});
}
assert(ResumeV && "Must have a resume value");
- VPValue *StartVal = BestEpiPlan.getOrAddExternalDef(ResumeV);
+ VPValue *StartVal = BestEpiPlan.getVPValueOrAddLiveIn(ResumeV);
cast<VPHeaderPHIRecipe>(&R)->setStartValue(StartVal);
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index 4655399cca73f..f185c9aea6bc4 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -591,14 +591,12 @@ VPlan::~VPlan() {
VPBlockBase::deleteCFG(Entry);
}
- for (VPValue *VPV : VPValuesToFree)
+ for (VPValue *VPV : VPLiveInsToFree)
delete VPV;
if (TripCount)
delete TripCount;
if (BackedgeTakenCount)
delete BackedgeTakenCount;
- for (auto &P : VPExternalDefs)
- delete P.second;
}
VPActiveLaneMaskPHIRecipe *VPlan::getActiveLaneMaskPhi() {
@@ -641,7 +639,7 @@ void VPlan::prepareToExecute(Value *TripCountV, Value *VectorTripCountV,
// needs to be changed from zero to the value after the main vector loop.
// FIXME: Improve modeling for canonical IV start values in the epilogue loop.
if (CanonicalIVStartValue) {
- VPValue *VPV = getOrAddExternalDef(CanonicalIVStartValue);
+ VPValue *VPV = getVPValueOrAddLiveIn(CanonicalIVStartValue);
auto *IV = getCanonicalIV();
assert(all_of(IV->users(),
[](const VPUser *U) {
@@ -1131,9 +1129,9 @@ bool vputils::onlyFirstLaneUsed(VPValue *Def) {
VPValue *vputils::getOrCreateVPValueForSCEVExpr(VPlan &Plan, const SCEV *Expr,
ScalarEvolution &SE) {
if (auto *E = dyn_cast<SCEVConstant>(Expr))
- return Plan.getOrAddExternalDef(E->getValue());
+ return Plan.getVPValueOrAddLiveIn(E->getValue());
if (auto *E = dyn_cast<SCEVUnknown>(Expr))
- return Plan.getOrAddExternalDef(E->getValue());
+ return Plan.getVPValueOrAddLiveIn(E->getValue());
VPBasicBlock *Preheader = Plan.getEntry()->getEntryBasicBlock();
VPExpandSCEVRecipe *Step = new VPExpandSCEVRecipe(Expr, SE);
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index ec75c02a3a092..0e74a0b91aba4 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -2233,10 +2233,6 @@ class VPlan {
/// Holds the name of the VPlan, for printing.
std::string Name;
- /// Holds all the external definitions created for this VPlan. External
- /// definitions must be immutable and hold a pointer to their underlying IR.
- DenseMap<Value *, VPValue *> VPExternalDefs;
-
/// Represents the trip count of the original loop, for folding
/// the tail.
VPValue *TripCount = nullptr;
@@ -2252,9 +2248,9 @@ class VPlan {
/// VPlan.
Value2VPValueTy Value2VPValue;
- /// Contains all VPValues that been allocated by addVPValue directly and need
- /// to be free when the plan's destructor is called.
- SmallVector<VPValue *, 16> VPValuesToFree;
+ /// Contains all the external definitions created for this VPlan. External
+ /// definitions are VPValues that hold a pointer to their underlying IR.
+ SmallVector<VPValue *, 16> VPLiveInsToFree;
/// Indicates whether it is safe use the Value2VPValue mapping or if the
/// mapping cannot be used any longer, because it is stale.
@@ -2334,50 +2330,35 @@ class VPlan {
void setName(const Twine &newName) { Name = newName.str(); }
- /// Get the existing or add a new external definition for \p V.
- VPValue *getOrAddExternalDef(Value *V) {
- auto I = VPExternalDefs.insert({V, nullptr});
- if (I.second)
- I.first->second = new VPValue(V);
- return I.first->second;
- }
-
- void addVPValue(Value *V) {
- assert(Value2VPValueEnabled &&
- "IR value to VPValue mapping may be out of date!");
- assert(V && "Trying to add a null Value to VPlan");
- assert(!Value2VPValue.count(V) && "Value already exists in VPlan");
- VPValue *VPV = new VPValue(V);
- Value2VPValue[V] = VPV;
- VPValuesToFree.push_back(VPV);
- }
-
void addVPValue(Value *V, VPValue *VPV) {
- assert(Value2VPValueEnabled && "Value2VPValue mapping may be out of date!");
+ assert((Value2VPValueEnabled || !VPV->getDefiningRecipe()) &&
+ "Value2VPValue mapping may be out of date!");
assert(V && "Trying to add a null Value to VPlan");
assert(!Value2VPValue.count(V) && "Value already exists in VPlan");
Value2VPValue[V] = VPV;
}
/// Returns the VPValue for \p V. \p OverrideAllowed can be used to disable
- /// checking whether it is safe to query VPValues using IR Values.
+ /// /// checking whether it is safe to query VPValues using IR Values.
VPValue *getVPValue(Value *V, bool OverrideAllowed = false) {
- assert((OverrideAllowed || isa<Constant>(V) || Value2VPValueEnabled) &&
- "Value2VPValue mapping may be out of date!");
assert(V && "Trying to get the VPValue of a null Value");
assert(Value2VPValue.count(V) && "Value does not exist in VPlan");
+ assert((!Value2VPValue[V]->getDefiningRecipe() || Value2VPValueEnabled ||
+ OverrideAllowed) &&
+ "Value2VPValue mapping may be out of date!");
return Value2VPValue[V];
}
- /// Gets the VPValue or adds a new one (if none exists yet) for \p V. \p
- /// OverrideAllowed can be used to disable checking whether it is safe to
- /// query VPValues using IR Values.
- VPValue *getOrAddVPValue(Value *V, bool OverrideAllowed = false) {
- assert((OverrideAllowed || isa<Constant>(V) || Value2VPValueEnabled) &&
- "Value2VPValue mapping may be out of date!");
+ /// Gets the VPValue for \p V or adds a new live-in (if none exists yet) for
+ /// \p V.
+ VPValue *getVPValueOrAddLiveIn(Value *V) {
assert(V && "Trying to get or add the VPValue of a null Value");
- if (!Value2VPValue.count(V))
- addVPValue(V);
+ if (!Value2VPValue.count(V)) {
+ VPValue *VPV = new VPValue(V);
+ VPLiveInsToFree.push_back(VPV);
+ addVPValue(V, VPV);
+ }
+
return getVPValue(V);
}
@@ -2403,7 +2384,7 @@ class VPlan {
iterator_range<mapped_iterator<Use *, std::function<VPValue *(Value *)>>>
mapToVPValues(User::op_range Operands) {
std::function<VPValue *(Value *)> Fn = [this](Value *Op) {
- return getOrAddVPValue(Op);
+ return getVPValueOrAddLiveIn(Op);
};
return map_range(Operands, Fn);
}
diff --git a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
index 952ce72e36c15..10fefae1cacf1 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanHCFGBuilder.cpp
@@ -196,7 +196,7 @@ VPValue *PlainCFGBuilder::getOrCreateVPOperand(Value *IRVal) {
// A and B: Create VPValue and add it to the pool of external definitions and
// to the Value->VPValue map.
- VPValue *NewVPVal = Plan.getOrAddExternalDef(IRVal);
+ VPValue *NewVPVal = Plan.getVPValueOrAddLiveIn(IRVal);
IRDef2VPValue[IRVal] = NewVPVal;
return NewVPVal;
}
@@ -272,7 +272,7 @@ VPBasicBlock *PlainCFGBuilder::buildPlainCFG() {
for (auto &I : *ThePreheaderBB) {
if (I.getType()->isVoidTy())
continue;
- IRDef2VPValue[&I] = Plan.getOrAddExternalDef(&I);
+ IRDef2VPValue[&I] = Plan.getVPValueOrAddLiveIn(&I);
}
// Create empty VPBB for Loop H so that we can link PH->H.
VPBlockBase *HeaderVPBB = getOrCreateVPBB(TheLoop->getHeader());
diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
index abd8b4f8cf2f2..41c7c79c0e928 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp
@@ -52,7 +52,7 @@ void VPlanTransforms::VPInstructionsToVPRecipes(
if (auto *VPPhi = dyn_cast<VPWidenPHIRecipe>(&Ingredient)) {
auto *Phi = cast<PHINode>(VPPhi->getUnderlyingValue());
if (const auto *II = GetIntOrFpInductionDescriptor(Phi)) {
- VPValue *Start = Plan->getOrAddVPValue(II->getStartValue());
+ VPValue *Start = Plan->getVPValueOrAddLiveIn(II->getStartValue());
VPValue *Step =
vputils::getOrCreateVPValueForSCEVExpr(*Plan, II->getStep(), SE);
NewRecipe =
@@ -68,13 +68,15 @@ void VPlanTransforms::VPInstructionsToVPRecipes(
// Create VPWidenMemoryInstructionRecipe for loads and stores.
if (LoadInst *Load = dyn_cast<LoadInst>(Inst)) {
NewRecipe = new VPWidenMemoryInstructionRecipe(
- *Load, Plan->getOrAddVPValue(getLoadStorePointerOperand(Inst)),
+ *Load,
+ Plan->getVPValueOrAddLiveIn(getLoadStorePointerOperand(Inst)),
nullptr /*Mask*/, false /*Consecutive*/, false /*Reverse*/);
} else if (StoreInst *Store = dyn_cast<StoreInst>(Inst)) {
NewRecipe = new VPWidenMemoryInstructionRecipe(
- *Store, Plan->getOrAddVPValue(getLoadStorePointerOperand(Inst)),
- Plan->getOrAddVPValue(Store->getValueOperand()), nullptr /*Mask*/,
- false /*Consecutive*/, false /*Reverse*/);
+ *Store,
+ Plan->getVPValueOrAddLiveIn(getLoadStorePointerOperand(Inst)),
+ Plan->getVPValueOrAddLiveIn(Store->getValueOperand()),
+ nullptr /*Mask*/, false /*Consecutive*/, false /*Reverse*/);
} else if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(Inst)) {
NewRecipe =
new VPWidenGEPRecipe(GEP, Plan->mapToVPValues(GEP->operands()));
@@ -600,9 +602,9 @@ void VPlanTransforms::optimizeForVFAndUF(VPlan &Plan, ElementCount BestVF,
return;
LLVMContext &Ctx = SE.getContext();
- auto *BOC =
- new VPInstruction(VPInstruction::BranchOnCond,
- {Plan.getOrAddExternalDef(ConstantInt::getTrue(Ctx))});
+ auto *BOC = new VPInstruction(
+ VPInstruction::BranchOnCond,
+ {Plan.getVPValueOrAddLiveIn(ConstantInt::getTrue(Ctx))});
Term->eraseFromParent();
ExitingVPBB->appendRecipe(BOC);
Plan.setVF(BestVF);
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp
index e8c84cc2b62e8..f6f283a44a1de 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanHCFGTest.cpp
@@ -96,7 +96,7 @@ TEST_F(VPlanHCFGTest, testBuildHCFGInnerLoop) {
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
// Add an external value to check we do not print the list of external values,
// as this is not required with the new printing.
- Plan->addVPValue(&*F->arg_begin());
+ Plan->getVPValueOrAddLiveIn(&*F->arg_begin());
std::string FullDump;
raw_string_ostream OS(FullDump);
Plan->printDOT(OS);
diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
index 903e8232a617d..606d8d03fc63f 100644
--- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
+++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp
@@ -1203,8 +1203,8 @@ TEST(VPRecipeTest, dump) {
BinaryOperator::CreateAdd(UndefValue::get(Int32), UndefValue::get(Int32));
AI->setName("a");
SmallVector<VPValue *, 2> Args;
- VPValue *ExtVPV1 = Plan.getOrAddExternalDef(ConstantInt::get(Int32, 1));
- VPValue *ExtVPV2 = Plan.getOrAddExternalDef(ConstantInt::get(Int32, 2));
+ VPValue *ExtVPV1 = Plan.getVPValueOrAddLiveIn(ConstantInt::get(Int32, 1));
+ VPValue *ExtVPV2 = Plan.getVPValueOrAddLiveIn(ConstantInt::get(Int32, 2));
Args.push_back(ExtVPV1);
Args.push_back(ExtVPV2);
VPWidenRecipe *WidenR =
More information about the llvm-commits
mailing list