From 348d85a6c7950a5f14ee6c8741380b5876d99afd Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Sat, 3 Oct 2020 17:40:42 +0100 Subject: [PATCH] [VPlan] Clean up uses/operands on VPBB deletion. Update the code responsible for deleting VPBBs and recipes to properly update users and release operands. This is another preparation for D84680 & following patches towards enabling modeling def-use chains in VPlan. --- llvm/lib/Transforms/Vectorize/VPlan.cpp | 26 ++++++++++++++++++- llvm/lib/Transforms/Vectorize/VPlan.h | 13 +++++++--- .../Transforms/Vectorize/VPlanTransforms.cpp | 3 +++ llvm/lib/Transforms/Vectorize/VPlanValue.h | 4 +++ .../Transforms/Vectorize/VPlanSlpTest.cpp | 23 ++++++++++++++++ .../Transforms/Vectorize/VPlanTest.cpp | 18 +++++++++++++ 6 files changed, 82 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Transforms/Vectorize/VPlan.cpp b/llvm/lib/Transforms/Vectorize/VPlan.cpp index cb5a43272e54..97292f138448 100644 --- a/llvm/lib/Transforms/Vectorize/VPlan.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlan.cpp @@ -181,8 +181,15 @@ VPBlockBase *VPBlockBase::getEnclosingBlockWithPredecessors() { void VPBlockBase::deleteCFG(VPBlockBase *Entry) { SmallVector Blocks; - for (VPBlockBase *Block : depth_first(Entry)) + + VPValue DummyValue; + for (VPBlockBase *Block : depth_first(Entry)) { + // Drop all references in VPBasicBlocks and replace all uses with + // DummyValue. + if (auto *VPBB = dyn_cast(Block)) + VPBB->dropAllReferences(&DummyValue); Blocks.push_back(Block); + } for (VPBlockBase *Block : Blocks) delete Block; @@ -305,6 +312,17 @@ void VPBasicBlock::execute(VPTransformState *State) { LLVM_DEBUG(dbgs() << "LV: filled BB:" << *NewBB); } +void VPBasicBlock::dropAllReferences(VPValue *NewValue) { + for (VPRecipeBase &R : Recipes) { + if (auto *VPV = R.toVPValue()) + VPV->replaceAllUsesWith(NewValue); + + if (auto *User = R.toVPUser()) + for (unsigned I = 0, E = User->getNumOperands(); I != E; I++) + User->setOperand(I, NewValue); + } +} + void VPRegionBlock::execute(VPTransformState *State) { ReversePostOrderTraversal RPOT(Entry); @@ -376,6 +394,12 @@ void VPRecipeBase::removeFromParent() { Parent = nullptr; } +VPValue *VPRecipeBase::toVPValue() { + if (auto *V = dyn_cast(this)) + return V; + return nullptr; +} + iplist::iterator VPRecipeBase::eraseFromParent() { assert(getParent() && "Recipe not in any VPBasicBlock"); return getParent()->getRecipeList().erase(getIterator()); diff --git a/llvm/lib/Transforms/Vectorize/VPlan.h b/llvm/lib/Transforms/Vectorize/VPlan.h index fae73fdc5782..2a51162fa7c8 100644 --- a/llvm/lib/Transforms/Vectorize/VPlan.h +++ b/llvm/lib/Transforms/Vectorize/VPlan.h @@ -680,6 +680,10 @@ public: /// Returns a pointer to a VPUser, if the recipe inherits from VPUser or /// nullptr otherwise. VPUser *toVPUser(); + + /// Returns a pointer to a VPValue, if the recipe inherits from VPValue or + /// nullptr otherwise. + VPValue *toVPValue(); }; inline bool VPUser::classof(const VPRecipeBase *Recipe) { @@ -1362,6 +1366,10 @@ public: /// this VPBasicBlock, thereby "executing" the VPlan. void execute(struct VPTransformState *State) override; + /// Replace all operands of VPUsers in the block with \p NewValue and also + /// replaces all uses of VPValues defined in the block with NewValue. + void dropAllReferences(VPValue *NewValue); + private: /// Create an IR BasicBlock to hold the output instructions generated by this /// VPBasicBlock, and return it. Update the CFGState accordingly. @@ -2006,10 +2014,7 @@ class VPlanSlp { public: VPlanSlp(VPInterleavedAccessInfo &IAI, VPBasicBlock &BB) : IAI(IAI), BB(BB) {} - ~VPlanSlp() { - for (auto &KV : BundleToCombined) - delete KV.second; - } + ~VPlanSlp() = default; /// Tries to build an SLP tree rooted at \p Operands and returns a /// VPInstruction combining \p Operands, if they can be combined. diff --git a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp index 3a4872a72122..45aeb201c28e 100644 --- a/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp +++ b/llvm/lib/Transforms/Vectorize/VPlanTransforms.cpp @@ -35,6 +35,7 @@ void VPlanTransforms::VPInstructionsToVPRecipes( Plan->addCBV(NCondBit); } } + VPValue DummyValue; for (VPBlockBase *Base : RPOT) { // Do not widen instructions in pre-header and exit blocks. if (Base->getNumPredecessors() == 0 || Base->getNumSuccessors() == 0) @@ -48,6 +49,7 @@ void VPlanTransforms::VPInstructionsToVPRecipes( VPInstruction *VPInst = cast(Ingredient); Instruction *Inst = cast(VPInst->getUnderlyingValue()); if (DeadInstructions.count(Inst)) { + VPInst->replaceAllUsesWith(&DummyValue); Ingredient->eraseFromParent(); continue; } @@ -77,6 +79,7 @@ void VPlanTransforms::VPInstructionsToVPRecipes( new VPWidenRecipe(*Inst, Plan->mapToVPValues(Inst->operands())); NewRecipe->insertBefore(Ingredient); + VPInst->replaceAllUsesWith(&DummyValue); Ingredient->eraseFromParent(); } } diff --git a/llvm/lib/Transforms/Vectorize/VPlanValue.h b/llvm/lib/Transforms/Vectorize/VPlanValue.h index 0882837170f2..e51c19601f88 100644 --- a/llvm/lib/Transforms/Vectorize/VPlanValue.h +++ b/llvm/lib/Transforms/Vectorize/VPlanValue.h @@ -168,6 +168,10 @@ public: VPUser(const VPUser &) = delete; VPUser &operator=(const VPUser &) = delete; + virtual ~VPUser() { + for (VPValue *Op : operands()) + Op->removeUser(*this); + } void addOperand(VPValue *Operand) { Operands.push_back(Operand); diff --git a/llvm/unittests/Transforms/Vectorize/VPlanSlpTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanSlpTest.cpp index 6ebef45e3cdf..2d868d3cd8af 100644 --- a/llvm/unittests/Transforms/Vectorize/VPlanSlpTest.cpp +++ b/llvm/unittests/Transforms/Vectorize/VPlanSlpTest.cpp @@ -116,6 +116,11 @@ TEST_F(VPlanSlpTest, testSlpSimple_2) { auto *CombinedLoadB = cast(CombinedAdd->getOperand(1)); EXPECT_EQ(VPInstruction::SLPLoad, CombinedLoadA->getOpcode()); EXPECT_EQ(VPInstruction::SLPLoad, CombinedLoadB->getOpcode()); + + delete CombinedStore; + delete CombinedAdd; + delete CombinedLoadA; + delete CombinedLoadB; } TEST_F(VPlanSlpTest, testSlpSimple_3) { @@ -190,6 +195,11 @@ TEST_F(VPlanSlpTest, testSlpSimple_3) { VPInstruction *GetB = cast(&*std::next(Body->begin(), 3)); EXPECT_EQ(GetA, CombinedLoadA->getOperand(0)); EXPECT_EQ(GetB, CombinedLoadB->getOperand(0)); + + delete CombinedStore; + delete CombinedAdd; + delete CombinedLoadA; + delete CombinedLoadB; } TEST_F(VPlanSlpTest, testSlpReuse_1) { @@ -249,6 +259,10 @@ TEST_F(VPlanSlpTest, testSlpReuse_1) { auto *CombinedLoadA = cast(CombinedAdd->getOperand(0)); EXPECT_EQ(CombinedLoadA, CombinedAdd->getOperand(1)); EXPECT_EQ(VPInstruction::SLPLoad, CombinedLoadA->getOpcode()); + + delete CombinedStore; + delete CombinedAdd; + delete CombinedLoadA; } TEST_F(VPlanSlpTest, testSlpReuse_2) { @@ -355,6 +369,15 @@ static void checkReorderExample(VPInstruction *Store1, VPInstruction *Store2, VPInstruction *LoadvD1 = cast(&*std::next(Body->begin(), 19)); EXPECT_EQ(LoadvD0->getOperand(0), CombinedLoadD->getOperand(0)); EXPECT_EQ(LoadvD1->getOperand(0), CombinedLoadD->getOperand(1)); + + delete CombinedStore; + delete CombinedAdd; + delete CombinedMulAB; + delete CombinedMulCD; + delete CombinedLoadA; + delete CombinedLoadB; + delete CombinedLoadC; + delete CombinedLoadD; } TEST_F(VPlanSlpTest, testSlpReorder_1) { diff --git a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp index 39727ca01d84..e325439f72be 100644 --- a/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp +++ b/llvm/unittests/Transforms/Vectorize/VPlanTest.cpp @@ -179,6 +179,24 @@ TEST(VPInstructionTest, replaceAllUsesWith) { delete VPV3; } +TEST(VPInstructionTest, releaseOperandsAtDeletion) { + VPValue *VPV1 = new VPValue(); + VPValue *VPV2 = new VPValue(); + VPInstruction *I1 = new VPInstruction(0, {VPV1, VPV2}); + + EXPECT_EQ(1u, VPV1->getNumUsers()); + EXPECT_EQ(I1, *VPV1->user_begin()); + EXPECT_EQ(1u, VPV2->getNumUsers()); + EXPECT_EQ(I1, *VPV2->user_begin()); + + delete I1; + + EXPECT_EQ(0u, VPV1->getNumUsers()); + EXPECT_EQ(0u, VPV2->getNumUsers()); + + delete VPV1; + delete VPV2; +} TEST(VPBasicBlockTest, getPlan) { { VPBasicBlock *VPBB1 = new VPBasicBlock();