[llvm] 710aceb - Revert "[VPlan] Use VPValue def for VPMemoryInstructionRecipe."
Vitaly Buka via llvm-commits
llvm-commits at lists.llvm.org
Tue Oct 13 03:14:28 PDT 2020
Author: Vitaly Buka
Date: 2020-10-13T03:14:08-07:00
New Revision: 710aceb645e7dba4de7053eef2c616311b9163d4
URL: https://github.com/llvm/llvm-project/commit/710aceb645e7dba4de7053eef2c616311b9163d4
DIFF: https://github.com/llvm/llvm-project/commit/710aceb645e7dba4de7053eef2c616311b9163d4.diff
LOG: Revert "[VPlan] Use VPValue def for VPMemoryInstructionRecipe."
It introduced a memory leak.
This reverts commit 525b085a65d30a5f2ae2af38c0be252fe8d4781b.
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
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
index 397dae34bceb..95d55d062da0 100644
--- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
+++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
@@ -531,10 +531,6 @@ class InnerLoopVectorizer {
/// value into a vector.
Value *getOrCreateVectorValue(Value *V, unsigned Part);
- void setVectorValue(Value *Scalar, unsigned Part, Value *Vector) {
- VectorLoopValueMap.setVectorValue(Scalar, Part, Vector);
- }
-
/// Return a value in the new loop corresponding to \p V from the original
/// loop at unroll and vector indices \p Instance. If the value has been
/// vectorized but not scalarized, the necessary extractelement instruction
@@ -557,8 +553,8 @@ class InnerLoopVectorizer {
/// non-null. Use \p State to translate given VPValues to IR values in the
/// vectorized loop.
void vectorizeMemoryInstruction(Instruction *Instr, VPTransformState &State,
- VPValue *Def, VPValue *Addr,
- VPValue *StoredValue, VPValue *BlockInMask);
+ VPValue *Addr, VPValue *StoredValue,
+ VPValue *BlockInMask);
/// Set the debug location in the builder using the debug location in
/// the instruction.
@@ -2507,9 +2503,11 @@ void InnerLoopVectorizer::vectorizeInterleaveGroup(
}
}
-void InnerLoopVectorizer::vectorizeMemoryInstruction(
- Instruction *Instr, VPTransformState &State, VPValue *Def, VPValue *Addr,
- VPValue *StoredValue, VPValue *BlockInMask) {
+void InnerLoopVectorizer::vectorizeMemoryInstruction(Instruction *Instr,
+ VPTransformState &State,
+ VPValue *Addr,
+ VPValue *StoredValue,
+ VPValue *BlockInMask) {
// Attempt to issue a wide load.
LoadInst *LI = dyn_cast<LoadInst>(Instr);
StoreInst *SI = dyn_cast<StoreInst>(Instr);
@@ -2638,8 +2636,7 @@ void InnerLoopVectorizer::vectorizeMemoryInstruction(
if (Reverse)
NewLI = reverseVector(NewLI);
}
-
- State.set(Def, Instr, NewLI, Part);
+ VectorLoopValueMap.setVectorValue(Instr, Part, NewLI);
}
}
@@ -7757,16 +7754,6 @@ VPlanPtr LoopVectorizationPlanner::buildVPlanWithVPRecipes(
if (auto Recipe =
RecipeBuilder.tryToCreateWidenRecipe(Instr, Range, Plan)) {
- // Check if the recipe can be converted to a VPValue. We need the extra
- // down-casting step until VPRecipeBase inherits from VPValue.
- VPValue *MaybeVPValue = Recipe->toVPValue();
- if (!Instr->getType()->isVoidTy() && MaybeVPValue) {
- if (NeedDef.contains(Instr))
- Plan->addOrReplaceVPValue(Instr, MaybeVPValue);
- else
- Plan->addVPValue(Instr, MaybeVPValue);
- }
-
RecipeBuilder.setRecipe(Instr, Recipe);
VPBB->appendRecipe(Recipe);
continue;
@@ -7816,14 +7803,7 @@ VPlanPtr LoopVectorizationPlanner::buildVPlanWithVPRecipes(
for (unsigned i = 0; i < IG->getFactor(); ++i)
if (Instruction *Member = IG->getMember(i)) {
- VPValue *NewVPV = nullptr;
- if (!Member->getType()->isVoidTy()) {
- NewVPV = new VPValue(Member);
- Plan->getVPValue(Member)->replaceAllUsesWith(NewVPV);
- }
RecipeBuilder.getRecipe(Member)->eraseFromParent();
- if (NewVPV)
- Plan->addVPValue(Member, NewVPV);
}
}
@@ -8165,11 +8145,9 @@ void VPPredInstPHIRecipe::execute(VPTransformState &State) {
}
void VPWidenMemoryInstructionRecipe::execute(VPTransformState &State) {
- Instruction *Instr = getUnderlyingInstr();
VPValue *StoredValue = isa<StoreInst>(Instr) ? getStoredValue() : nullptr;
- State.ILV->vectorizeMemoryInstruction(Instr, State,
- StoredValue ? nullptr : this, getAddr(),
- StoredValue, getMask());
+ State.ILV->vectorizeMemoryInstruction(&Instr, State, getAddr(), StoredValue,
+ getMask());
}
// Determine how to lower the scalar epilogue, which depends on 1) optimising
@@ -8215,12 +8193,6 @@ static ScalarEpilogueLowering getScalarEpilogueLowering(
return CM_ScalarEpilogueAllowed;
}
-void VPTransformState::set(VPValue *Def, Value *IRDef, Value *V,
- unsigned Part) {
- set(Def, V, Part);
- ILV->setVectorValue(IRDef, Part, V);
-}
-
// Process the loop in the VPlan-native vectorization path. This path builds
// VPlan upfront in the vectorization pipeline, which allows to apply
// VPlan-to-VPlan transformations from the very beginning without modifying the
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp
index f2659d2b2664..054920645a9a 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.cpp
+++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp
@@ -101,22 +101,6 @@ VPUser *VPRecipeBase::toVPUser() {
return nullptr;
}
-VPValue *VPRecipeBase::toVPValue() {
- if (auto *V = dyn_cast<VPInstruction>(this))
- return V;
- if (auto *V = dyn_cast<VPWidenMemoryInstructionRecipe>(this))
- return V;
- return nullptr;
-}
-
-const VPValue *VPRecipeBase::toVPValue() const {
- if (auto *V = dyn_cast<VPInstruction>(this))
- return V;
- if (auto *V = dyn_cast<VPWidenMemoryInstructionRecipe>(this))
- return V;
- return nullptr;
-}
-
// Get the top-most entry block of \p Start. This is the entry block of the
// containing VPlan. This function is templated to support both const and non-const blocks
template <typename T> static T *getPlanEntry(T *Start) {
@@ -421,15 +405,14 @@ void VPRecipeBase::removeFromParent() {
Parent = nullptr;
}
+VPValue *VPRecipeBase::toVPValue() {
+ if (auto *V = dyn_cast<VPInstruction>(this))
+ return V;
+ return nullptr;
+}
+
iplist<VPRecipeBase>::iterator VPRecipeBase::eraseFromParent() {
assert(getParent() && "Recipe not in any VPBasicBlock");
- // If the recipe is a VPValue and has been added to the containing VPlan,
- // remove the mapping.
- if (Value *UV = getUnderlyingInstr())
- if (!UV->getType()->isVoidTy())
- if (auto *Plan = getParent()->getPlan())
- Plan->removeVPValueFor(UV);
-
return getParent()->getRecipeList().erase(getIterator());
}
@@ -920,8 +903,7 @@ void VPPredInstPHIRecipe::print(raw_ostream &O, const Twine &Indent,
void VPWidenMemoryInstructionRecipe::print(raw_ostream &O, const Twine &Indent,
VPSlotTracker &SlotTracker) const {
- O << "\"WIDEN "
- << Instruction::getOpcodeName(getUnderlyingInstr()->getOpcode()) << " ";
+ O << "\"WIDEN " << Instruction::getOpcodeName(Instr.getOpcode()) << " ";
bool First = true;
for (VPValue *Op : operands()) {
diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h
index 81a9d67d2976..30f984fd39d7 100644
--- a/llvm/lib/Transforms/Vectorize/VPlan.h
+++ b/llvm/lib/Transforms/Vectorize/VPlan.h
@@ -282,10 +282,6 @@ struct VPTransformState {
// delegates the call to ILV below.
if (Data.PerPartOutput.count(Def)) {
auto *VecPart = Data.PerPartOutput[Def][Instance.Part];
- if (!VecPart->getType()->isVectorTy()) {
- assert(Instance.Lane == 0 && "cannot get lane > 0 for scalar");
- return VecPart;
- }
// TODO: Cache created scalar values.
return Builder.CreateExtractElement(VecPart,
Builder.getInt32(Instance.Lane));
@@ -302,7 +298,6 @@ struct VPTransformState {
}
Data.PerPartOutput[Def][Part] = V;
}
- void set(VPValue *Def, Value *IRDef, Value *V, unsigned Part);
/// Hold state information used when constructing the CFG of the output IR,
/// traversing the VPBasicBlocks and generating corresponding IR BasicBlocks.
@@ -689,20 +684,6 @@ class VPRecipeBase : public ilist_node_with_parent<VPRecipeBase, VPBasicBlock> {
/// Returns a pointer to a VPValue, if the recipe inherits from VPValue or
/// nullptr otherwise.
VPValue *toVPValue();
- const VPValue *toVPValue() const;
-
- /// Returns the underlying instruction, if the recipe is a VPValue or nullptr
- /// otherwise.
- Instruction *getUnderlyingInstr() {
- if (auto *VPV = toVPValue())
- return cast_or_null<Instruction>(VPV->getUnderlyingValue());
- return nullptr;
- }
- const Instruction *getUnderlyingInstr() const {
- if (auto *VPV = toVPValue())
- return cast_or_null<Instruction>(VPV->getUnderlyingValue());
- return nullptr;
- }
};
inline bool VPUser::classof(const VPRecipeBase *Recipe) {
@@ -744,6 +725,10 @@ class VPInstruction : public VPUser, public VPValue, public VPRecipeBase {
void generateInstruction(VPTransformState &State, unsigned Part);
protected:
+ Instruction *getUnderlyingInstr() {
+ return cast_or_null<Instruction>(getUnderlyingValue());
+ }
+
void setUnderlyingInstr(Instruction *I) { setUnderlyingValue(I); }
public:
@@ -1222,9 +1207,8 @@ class VPPredInstPHIRecipe : public VPRecipeBase {
/// - For store: Address, stored value, optional mask
/// TODO: We currently execute only per-part unless a specific instance is
/// provided.
-class VPWidenMemoryInstructionRecipe : public VPRecipeBase,
- public VPValue,
- public VPUser {
+class VPWidenMemoryInstructionRecipe : public VPRecipeBase, public VPUser {
+ Instruction &Instr;
void setMask(VPValue *Mask) {
if (!Mask)
@@ -1233,22 +1217,20 @@ class VPWidenMemoryInstructionRecipe : public VPRecipeBase,
}
bool isMasked() const {
- return (isa<LoadInst>(getUnderlyingInstr()) && getNumOperands() == 2) ||
- (isa<StoreInst>(getUnderlyingInstr()) && getNumOperands() == 3);
+ return (isa<LoadInst>(Instr) && getNumOperands() == 2) ||
+ (isa<StoreInst>(Instr) && getNumOperands() == 3);
}
public:
VPWidenMemoryInstructionRecipe(LoadInst &Load, VPValue *Addr, VPValue *Mask)
- : VPRecipeBase(VPWidenMemoryInstructionSC),
- VPValue(VPValue::VPMemoryInstructionSC, &Load), VPUser({Addr}) {
+ : VPRecipeBase(VPWidenMemoryInstructionSC), VPUser({Addr}), Instr(Load) {
setMask(Mask);
}
VPWidenMemoryInstructionRecipe(StoreInst &Store, VPValue *Addr,
VPValue *StoredValue, VPValue *Mask)
- : VPRecipeBase(VPWidenMemoryInstructionSC),
- VPValue(VPValue::VPMemoryInstructionSC, &Store),
- VPUser({Addr, StoredValue}) {
+ : VPRecipeBase(VPWidenMemoryInstructionSC), VPUser({Addr, StoredValue}),
+ Instr(Store) {
setMask(Mask);
}
@@ -1271,7 +1253,7 @@ class VPWidenMemoryInstructionRecipe : public VPRecipeBase,
/// Return the address accessed by this recipe.
VPValue *getStoredValue() const {
- assert(isa<StoreInst>(getUnderlyingInstr()) &&
+ assert(isa<StoreInst>(Instr) &&
"Stored value only available for store instructions");
return getOperand(1); // Stored value is the 2nd, mandatory operand.
}
@@ -1637,10 +1619,6 @@ 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;
-
/// Holds the VPLoopInfo analysis for this VPlan.
VPLoopInfo VPLInfo;
@@ -1656,8 +1634,8 @@ class VPlan {
~VPlan() {
if (Entry)
VPBlockBase::deleteCFG(Entry);
- for (VPValue *VPV : VPValuesToFree)
- delete VPV;
+ for (auto &MapEntry : Value2VPValue)
+ delete MapEntry.second;
if (BackedgeTakenCount)
delete BackedgeTakenCount;
for (VPValue *Def : VPExternalDefs)
@@ -1707,24 +1685,7 @@ class VPlan {
void addVPValue(Value *V) {
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(V && "Trying to add a null Value to VPlan");
- assert(!Value2VPValue.count(V) && "Value already exists in VPlan");
- Value2VPValue[V] = VPV;
- }
-
- void addOrReplaceVPValue(Value *V, VPValue *VPV) {
- assert(V && "Trying to add a null Value to VPlan");
- auto I = Value2VPValue.find(V);
- if (I == Value2VPValue.end())
- Value2VPValue[V] = VPV;
- else
- I->second = VPV;
+ Value2VPValue[V] = new VPValue(V);
}
VPValue *getVPValue(Value *V) {
@@ -1740,8 +1701,6 @@ class VPlan {
return getVPValue(V);
}
- void removeVPValueFor(Value *V) { Value2VPValue.erase(V); }
-
/// Return the VPLoopInfo analysis for this VPlan.
VPLoopInfo &getVPLoopInfo() { return VPLInfo; }
const VPLoopInfo &getVPLoopInfo() const { return VPLInfo; }
@@ -1823,9 +1782,9 @@ class VPlanPrinter {
};
struct VPlanIngredient {
- const Value *V;
+ Value *V;
- VPlanIngredient(const Value *V) : V(V) {}
+ VPlanIngredient(Value *V) : V(V) {}
};
inline raw_ostream &operator<<(raw_ostream &OS, const VPlanIngredient &I) {
diff --git a/llvm/lib/Transforms/Vectorize/VPlanValue.h b/llvm/lib/Transforms/Vectorize/VPlanValue.h
index ec8c5bfaae9a..e51c19601f88 100644
--- a/llvm/lib/Transforms/Vectorize/VPlanValue.h
+++ b/llvm/lib/Transforms/Vectorize/VPlanValue.h
@@ -43,7 +43,6 @@ class VPValue {
friend class VPBasicBlock;
friend class VPInterleavedAccessInfo;
friend class VPSlotTracker;
- friend class VPRecipeBase;
const unsigned char SubclassID; ///< Subclass identifier (for isa/dyn_cast).
@@ -78,7 +77,7 @@ 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 };
+ enum { VPValueSC, VPInstructionSC };
VPValue(Value *UV = nullptr) : VPValue(VPValueSC, UV) {}
VPValue(const VPValue &) = delete;
More information about the llvm-commits
mailing list