-
Notifications
You must be signed in to change notification settings - Fork 12.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[VPlan] Manage created blocks directly in VPlan. (NFC) #120918
Changes from 5 commits
dd45cad
e72a71f
407dbc1
af48fcc
f51412a
ec4f283
0b80912
fc34ca5
bc7f445
8f83ad8
d6b4d78
99924fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2477,7 +2477,7 @@ static void introduceCheckBlockInVPlan(VPlan &Plan, BasicBlock *CheckIRBB) { | |
assert(PreVectorPH->getNumSuccessors() == 2 && "Expected 2 successors"); | ||
assert(PreVectorPH->getSuccessors()[0] == ScalarPH && | ||
"Unexpected successor"); | ||
VPIRBasicBlock *CheckVPIRBB = VPIRBasicBlock::fromBasicBlock(CheckIRBB); | ||
VPIRBasicBlock *CheckVPIRBB = Plan.createVPIRBasicBlock(CheckIRBB); | ||
VPBlockUtils::insertOnEdge(PreVectorPH, VectorPH, CheckVPIRBB); | ||
PreVectorPH = CheckVPIRBB; | ||
} | ||
|
@@ -8189,11 +8189,10 @@ EpilogueVectorizerEpilogueLoop::emitMinimumVectorEpilogueIterCountCheck( | |
|
||
// A new entry block has been created for the epilogue VPlan. Hook it in, as | ||
// otherwise we would try to modify the entry to the main vector loop. | ||
VPIRBasicBlock *NewEntry = VPIRBasicBlock::fromBasicBlock(Insert); | ||
VPIRBasicBlock *NewEntry = Plan.createVPIRBasicBlock(Insert); | ||
VPBasicBlock *OldEntry = Plan.getEntry(); | ||
VPBlockUtils::reassociateBlocks(OldEntry, NewEntry); | ||
Plan.setEntry(NewEntry); | ||
delete OldEntry; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Perhaps worth leaving behind a note that OldEntry is now dead and will be collected later? Same with other deleted deletes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added, thanks! |
||
|
||
introduceCheckBlockInVPlan(Plan, Insert); | ||
return Insert; | ||
|
@@ -9463,7 +9462,7 @@ LoopVectorizationPlanner::tryToBuildVPlanWithVPRecipes(VFRange &Range) { | |
VPBB->appendRecipe(Recipe); | ||
} | ||
|
||
VPBlockUtils::insertBlockAfter(new VPBasicBlock(), VPBB); | ||
VPBlockUtils::insertBlockAfter(Plan->createVPBasicBlock(""), VPBB); | ||
VPBB = cast<VPBasicBlock>(VPBB->getSingleSuccessor()); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -205,11 +205,6 @@ VPBlockBase *VPBlockBase::getEnclosingBlockWithPredecessors() { | |||||||||||
return Parent->getEnclosingBlockWithPredecessors(); | ||||||||||||
} | ||||||||||||
|
||||||||||||
void VPBlockBase::deleteCFG(VPBlockBase *Entry) { | ||||||||||||
for (VPBlockBase *Block : to_vector(vp_depth_first_shallow(Entry))) | ||||||||||||
delete Block; | ||||||||||||
} | ||||||||||||
|
||||||||||||
VPBasicBlock::iterator VPBasicBlock::getFirstNonPhi() { | ||||||||||||
iterator It = begin(); | ||||||||||||
while (It != end() && It->isPhi()) | ||||||||||||
|
@@ -474,6 +469,16 @@ void VPIRBasicBlock::execute(VPTransformState *State) { | |||||||||||
connectToPredecessors(State->CFG); | ||||||||||||
} | ||||||||||||
|
||||||||||||
VPIRBasicBlock *VPIRBasicBlock::clone() { | ||||||||||||
auto *NewBlock = getPlan()->createVPIRBasicBlock(IRBB); | ||||||||||||
for (VPRecipeBase &R : make_early_inc_range(*NewBlock)) | ||||||||||||
R.eraseFromParent(); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Better to have another createVPIRBasicBlock() which avoids populating it with recipes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a new constructor to create empty VPIRBBs |
||||||||||||
|
||||||||||||
for (VPRecipeBase &R : Recipes) | ||||||||||||
NewBlock->appendRecipe(R.clone()); | ||||||||||||
Comment on lines
+474
to
+475
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New constructor should avoid need to rename. |
||||||||||||
return NewBlock; | ||||||||||||
} | ||||||||||||
|
||||||||||||
void VPBasicBlock::execute(VPTransformState *State) { | ||||||||||||
bool Replica = bool(State->Lane); | ||||||||||||
BasicBlock *NewBB = State->CFG.PrevBB; // Reuse it if possible. | ||||||||||||
|
@@ -523,6 +528,13 @@ void VPBasicBlock::dropAllReferences(VPValue *NewValue) { | |||||||||||
} | ||||||||||||
} | ||||||||||||
|
||||||||||||
VPBasicBlock *VPBasicBlock::clone() { | ||||||||||||
auto *NewBlock = getPlan()->createVPBasicBlock(getName()); | ||||||||||||
for (VPRecipeBase &R : *this) | ||||||||||||
NewBlock->appendRecipe(R.clone()); | ||||||||||||
return NewBlock; | ||||||||||||
} | ||||||||||||
|
||||||||||||
void VPBasicBlock::executeRecipes(VPTransformState *State, BasicBlock *BB) { | ||||||||||||
LLVM_DEBUG(dbgs() << "LV: vectorizing VPBB:" << getName() | ||||||||||||
<< " in BB:" << BB->getName() << '\n'); | ||||||||||||
|
@@ -541,7 +553,7 @@ VPBasicBlock *VPBasicBlock::splitAt(iterator SplitAt) { | |||||||||||
|
||||||||||||
SmallVector<VPBlockBase *, 2> Succs(successors()); | ||||||||||||
// Create new empty block after the block to split. | ||||||||||||
auto *SplitBlock = new VPBasicBlock(getName() + ".split"); | ||||||||||||
auto *SplitBlock = getPlan()->createVPBasicBlock(getName() + ".split"); | ||||||||||||
VPBlockUtils::insertBlockAfter(SplitBlock, this); | ||||||||||||
|
||||||||||||
// Finally, move the recipes starting at SplitAt to new block. | ||||||||||||
|
@@ -701,8 +713,8 @@ static std::pair<VPBlockBase *, VPBlockBase *> cloneFrom(VPBlockBase *Entry) { | |||||||||||
|
||||||||||||
VPRegionBlock *VPRegionBlock::clone() { | ||||||||||||
const auto &[NewEntry, NewExiting] = cloneFrom(getEntry()); | ||||||||||||
auto *NewRegion = | ||||||||||||
new VPRegionBlock(NewEntry, NewExiting, getName(), isReplicator()); | ||||||||||||
auto *NewRegion = getPlan()->createVPRegionBlock(NewEntry, NewExiting, | ||||||||||||
getName(), isReplicator()); | ||||||||||||
for (VPBlockBase *Block : vp_depth_first_shallow(NewEntry)) | ||||||||||||
Block->setParent(NewRegion); | ||||||||||||
return NewRegion; | ||||||||||||
|
@@ -822,32 +834,27 @@ void VPRegionBlock::print(raw_ostream &O, const Twine &Indent, | |||||||||||
#endif | ||||||||||||
|
||||||||||||
VPlan::VPlan(Loop *L) { | ||||||||||||
setEntry(VPIRBasicBlock::fromBasicBlock(L->getLoopPreheader())); | ||||||||||||
ScalarHeader = VPIRBasicBlock::fromBasicBlock(L->getHeader()); | ||||||||||||
setEntry(createVPIRBasicBlock(L->getLoopPreheader())); | ||||||||||||
ScalarHeader = createVPIRBasicBlock(L->getHeader()); | ||||||||||||
} | ||||||||||||
|
||||||||||||
VPlan::~VPlan() { | ||||||||||||
if (Entry) { | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this check if (Entry) be dropped now that CreatedBlocks are traversed (and probably need to be regardless of Entry)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done, thanks! |
||||||||||||
VPValue DummyValue; | ||||||||||||
for (VPBlockBase *Block : vp_depth_first_shallow(Entry)) | ||||||||||||
Block->dropAllReferences(&DummyValue); | ||||||||||||
|
||||||||||||
VPBlockBase::deleteCFG(Entry); | ||||||||||||
for (auto *VPB : reverse(CreatedBlocks)) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why/Is reverse needed? Are there any assumptions about the order in which blocks are created and appended to a VPlan? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not needed in the latest version, also merge the loops and inlined the only use of |
||||||||||||
VPB->dropAllReferences(&DummyValue); | ||||||||||||
|
||||||||||||
for (auto *VPB : reverse(CreatedBlocks)) { | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why/Is reverse needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not needed in the latest version, thanks |
||||||||||||
delete VPB; | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can VPB be deleted immediately after dropping all references from its recipes (fusing the two loops together), or need to drop all references from recipes of all blocks first? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep, merged the loops. thanks |
||||||||||||
} | ||||||||||||
} | ||||||||||||
for (VPValue *VPV : VPLiveInsToFree) | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: |
||||||||||||
delete VPV; | ||||||||||||
if (BackedgeTakenCount) | ||||||||||||
delete BackedgeTakenCount; | ||||||||||||
} | ||||||||||||
|
||||||||||||
VPIRBasicBlock *VPIRBasicBlock::fromBasicBlock(BasicBlock *IRBB) { | ||||||||||||
auto *VPIRBB = new VPIRBasicBlock(IRBB); | ||||||||||||
for (Instruction &I : | ||||||||||||
make_range(IRBB->begin(), IRBB->getTerminator()->getIterator())) | ||||||||||||
VPIRBB->appendRecipe(new VPIRInstruction(I)); | ||||||||||||
return VPIRBB; | ||||||||||||
} | ||||||||||||
|
||||||||||||
VPlanPtr VPlan::createInitialVPlan(Type *InductionTy, | ||||||||||||
PredicatedScalarEvolution &PSE, | ||||||||||||
bool RequiresScalarEpilogueCheck, | ||||||||||||
|
@@ -861,7 +868,7 @@ VPlanPtr VPlan::createInitialVPlan(Type *InductionTy, | |||||||||||
// an epilogue vector loop, the original entry block here will be replaced by | ||||||||||||
// a new VPIRBasicBlock wrapping the entry to the epilogue vector loop after | ||||||||||||
// generating code for the main vector loop. | ||||||||||||
VPBasicBlock *VecPreheader = new VPBasicBlock("vector.ph"); | ||||||||||||
VPBasicBlock *VecPreheader = Plan->createVPBasicBlock("vector.ph"); | ||||||||||||
VPBlockUtils::connectBlocks(Plan->getEntry(), VecPreheader); | ||||||||||||
|
||||||||||||
// Create SCEV and VPValue for the trip count. | ||||||||||||
|
@@ -878,17 +885,17 @@ VPlanPtr VPlan::createInitialVPlan(Type *InductionTy, | |||||||||||
|
||||||||||||
// Create VPRegionBlock, with empty header and latch blocks, to be filled | ||||||||||||
// during processing later. | ||||||||||||
VPBasicBlock *HeaderVPBB = new VPBasicBlock("vector.body"); | ||||||||||||
VPBasicBlock *LatchVPBB = new VPBasicBlock("vector.latch"); | ||||||||||||
VPBasicBlock *HeaderVPBB = Plan->createVPBasicBlock("vector.body"); | ||||||||||||
VPBasicBlock *LatchVPBB = Plan->createVPBasicBlock("vector.latch"); | ||||||||||||
VPBlockUtils::insertBlockAfter(LatchVPBB, HeaderVPBB); | ||||||||||||
auto *TopRegion = new VPRegionBlock(HeaderVPBB, LatchVPBB, "vector loop", | ||||||||||||
false /*isReplicator*/); | ||||||||||||
auto *TopRegion = Plan->createVPRegionBlock( | ||||||||||||
HeaderVPBB, LatchVPBB, "vector loop", false /*isReplicator*/); | ||||||||||||
|
||||||||||||
VPBlockUtils::insertBlockAfter(TopRegion, VecPreheader); | ||||||||||||
VPBasicBlock *MiddleVPBB = new VPBasicBlock("middle.block"); | ||||||||||||
VPBasicBlock *MiddleVPBB = Plan->createVPBasicBlock("middle.block"); | ||||||||||||
VPBlockUtils::insertBlockAfter(MiddleVPBB, TopRegion); | ||||||||||||
|
||||||||||||
VPBasicBlock *ScalarPH = new VPBasicBlock("scalar.ph"); | ||||||||||||
VPBasicBlock *ScalarPH = Plan->createVPBasicBlock("scalar.ph"); | ||||||||||||
VPBlockUtils::connectBlocks(ScalarPH, ScalarHeader); | ||||||||||||
if (!RequiresScalarEpilogueCheck) { | ||||||||||||
VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH); | ||||||||||||
|
@@ -904,7 +911,7 @@ VPlanPtr VPlan::createInitialVPlan(Type *InductionTy, | |||||||||||
// we unconditionally branch to the scalar preheader. Do nothing. | ||||||||||||
// 3) Otherwise, construct a runtime check. | ||||||||||||
BasicBlock *IRExitBlock = TheLoop->getUniqueLatchExitBlock(); | ||||||||||||
auto *VPExitBlock = VPIRBasicBlock::fromBasicBlock(IRExitBlock); | ||||||||||||
auto *VPExitBlock = Plan->createVPIRBasicBlock(IRExitBlock); | ||||||||||||
// The connection order corresponds to the operands of the conditional branch. | ||||||||||||
VPBlockUtils::insertBlockAfter(VPExitBlock, MiddleVPBB); | ||||||||||||
VPBlockUtils::connectBlocks(MiddleVPBB, ScalarPH); | ||||||||||||
|
@@ -960,15 +967,13 @@ void VPlan::prepareToExecute(Value *TripCountV, Value *VectorTripCountV, | |||||||||||
/// have a single predecessor, which is rewired to the new VPIRBasicBlock. All | ||||||||||||
/// successors of VPBB, if any, are rewired to the new VPIRBasicBlock. | ||||||||||||
static void replaceVPBBWithIRVPBB(VPBasicBlock *VPBB, BasicBlock *IRBB) { | ||||||||||||
VPIRBasicBlock *IRVPBB = VPIRBasicBlock::fromBasicBlock(IRBB); | ||||||||||||
VPIRBasicBlock *IRVPBB = VPBB->getPlan()->createVPIRBasicBlock(IRBB); | ||||||||||||
for (auto &R : make_early_inc_range(*VPBB)) { | ||||||||||||
assert(!R.isPhi() && "Tried to move phi recipe to end of block"); | ||||||||||||
R.moveBefore(*IRVPBB, IRVPBB->end()); | ||||||||||||
} | ||||||||||||
|
||||||||||||
VPBlockUtils::reassociateBlocks(VPBB, IRVPBB); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added, thanks |
||||||||||||
|
||||||||||||
delete VPBB; | ||||||||||||
} | ||||||||||||
|
||||||||||||
/// Generate the code inside the preheader and body of the vectorized loop. | ||||||||||||
|
@@ -1217,6 +1222,7 @@ static void remapOperands(VPBlockBase *Entry, VPBlockBase *NewEntry, | |||||||||||
} | ||||||||||||
|
||||||||||||
VPlan *VPlan::duplicate() { | ||||||||||||
unsigned CreatedBlockSize = CreatedBlocks.size(); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed , thanks |
||||||||||||
// Clone blocks. | ||||||||||||
const auto &[NewEntry, __] = cloneFrom(Entry); | ||||||||||||
|
||||||||||||
|
@@ -1257,9 +1263,24 @@ VPlan *VPlan::duplicate() { | |||||||||||
assert(Old2NewVPValues.contains(TripCount) && | ||||||||||||
"TripCount must have been added to Old2NewVPValues"); | ||||||||||||
NewPlan->TripCount = Old2NewVPValues[TripCount]; | ||||||||||||
|
||||||||||||
// Transfer cloned blocks to new VPlan. | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated, thanks |
||||||||||||
for (unsigned I : seq<unsigned>(CreatedBlockSize, CreatedBlocks.size())) | ||||||||||||
NewPlan->CreatedBlocks.push_back(CreatedBlocks[I]); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done thanks |
||||||||||||
CreatedBlocks.truncate(CreatedBlockSize); | ||||||||||||
|
||||||||||||
return NewPlan; | ||||||||||||
} | ||||||||||||
|
||||||||||||
VPIRBasicBlock *VPlan::createVPIRBasicBlock(BasicBlock *IRBB) { | ||||||||||||
auto *VPIRBB = new VPIRBasicBlock(IRBB); | ||||||||||||
for (Instruction &I : | ||||||||||||
make_range(IRBB->begin(), IRBB->getTerminator()->getIterator())) | ||||||||||||
VPIRBB->appendRecipe(new VPIRInstruction(I)); | ||||||||||||
CreatedBlocks.push_back(VPIRBB); | ||||||||||||
return VPIRBB; | ||||||||||||
} | ||||||||||||
|
||||||||||||
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) | ||||||||||||
|
||||||||||||
Twine VPlanPrinter::getUID(const VPBlockBase *Block) { | ||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -636,9 +636,6 @@ class VPBlockBase { | |||||
/// Return the cost of the block. | ||||||
virtual InstructionCost cost(ElementCount VF, VPCostContext &Ctx) = 0; | ||||||
|
||||||
/// Delete all blocks reachable from a given VPBlockBase, inclusive. | ||||||
static void deleteCFG(VPBlockBase *Entry); | ||||||
|
||||||
/// Return true if it is legal to hoist instructions into this block. | ||||||
bool isLegalToHoistInto() { | ||||||
// There are currently no constraints that prevent an instruction to be | ||||||
|
@@ -3638,12 +3635,7 @@ class VPBasicBlock : public VPBlockBase { | |||||
|
||||||
/// Clone the current block and it's recipes, without updating the operands of | ||||||
/// the cloned recipes. | ||||||
VPBasicBlock *clone() override { | ||||||
auto *NewBlock = new VPBasicBlock(getName()); | ||||||
for (VPRecipeBase &R : *this) | ||||||
NewBlock->appendRecipe(R.clone()); | ||||||
return NewBlock; | ||||||
} | ||||||
VPBasicBlock *clone() override; | ||||||
|
||||||
protected: | ||||||
/// Execute the recipes in the IR basic block \p BB. | ||||||
|
@@ -3679,20 +3671,11 @@ class VPIRBasicBlock : public VPBasicBlock { | |||||
return V->getVPBlockID() == VPBlockBase::VPIRBasicBlockSC; | ||||||
} | ||||||
|
||||||
/// Create a VPIRBasicBlock from \p IRBB containing VPIRInstructions for all | ||||||
/// instructions in \p IRBB, except its terminator which is managed in VPlan. | ||||||
static VPIRBasicBlock *fromBasicBlock(BasicBlock *IRBB); | ||||||
|
||||||
/// The method which generates the output IR instructions that correspond to | ||||||
/// this VPBasicBlock, thereby "executing" the VPlan. | ||||||
void execute(VPTransformState *State) override; | ||||||
|
||||||
VPIRBasicBlock *clone() override { | ||||||
auto *NewBlock = new VPIRBasicBlock(IRBB); | ||||||
for (VPRecipeBase &R : Recipes) | ||||||
NewBlock->appendRecipe(R.clone()); | ||||||
return NewBlock; | ||||||
} | ||||||
VPIRBasicBlock *clone() override; | ||||||
|
||||||
BasicBlock *getIRBasicBlock() const { return IRBB; } | ||||||
}; | ||||||
|
@@ -3731,13 +3714,7 @@ class VPRegionBlock : public VPBlockBase { | |||||
: VPBlockBase(VPRegionBlockSC, Name), Entry(nullptr), Exiting(nullptr), | ||||||
IsReplicator(IsReplicator) {} | ||||||
|
||||||
~VPRegionBlock() override { | ||||||
if (Entry) { | ||||||
VPValue DummyValue; | ||||||
Entry->dropAllReferences(&DummyValue); | ||||||
deleteCFG(Entry); | ||||||
} | ||||||
} | ||||||
~VPRegionBlock() override {} | ||||||
|
||||||
/// Method to support type inquiry through isa, cast, and dyn_cast. | ||||||
static inline bool classof(const VPBlockBase *V) { | ||||||
|
@@ -3863,6 +3840,10 @@ class VPlan { | |||||
/// been modeled in VPlan directly. | ||||||
DenseMap<const SCEV *, VPValue *> SCEVToExpansion; | ||||||
|
||||||
/// Blocks allocated and owned by the VPlan. They will be deleted once the | ||||||
/// VPlan is destroyed. | ||||||
SmallVector<VPBlockBase *> CreatedBlocks; | ||||||
|
||||||
Comment on lines
+3784
to
+3787
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: perhaps better positioned next to VPLiveInsToFree above. |
||||||
/// Construct a VPlan with \p Entry to the plan and with \p ScalarHeader | ||||||
/// wrapping the original header of the scalar loop. | ||||||
VPlan(VPBasicBlock *Entry, VPIRBasicBlock *ScalarHeader) | ||||||
|
@@ -3881,8 +3862,8 @@ class VPlan { | |||||
/// Construct a VPlan with a new VPBasicBlock as entry, a VPIRBasicBlock | ||||||
/// wrapping \p ScalarHeaderBB and a trip count of \p TC. | ||||||
VPlan(BasicBlock *ScalarHeaderBB, VPValue *TC) { | ||||||
setEntry(new VPBasicBlock("preheader")); | ||||||
ScalarHeader = VPIRBasicBlock::fromBasicBlock(ScalarHeaderBB); | ||||||
setEntry(createVPBasicBlock("preheader")); | ||||||
ScalarHeader = createVPIRBasicBlock(ScalarHeaderBB); | ||||||
TripCount = TC; | ||||||
} | ||||||
|
||||||
|
@@ -4080,6 +4061,44 @@ class VPlan { | |||||
/// Clone the current VPlan, update all VPValues of the new VPlan and cloned | ||||||
/// recipes to refer to the clones, and return it. | ||||||
VPlan *duplicate(); | ||||||
|
||||||
/// Create a new VPBasicBlock with \p Name and containing \p Recipe if | ||||||
/// present. The returned block is owned by the VPlan and deleted once the | ||||||
/// VPlan is destroyed. | ||||||
VPBasicBlock *createVPBasicBlock(const Twine &Name, | ||||||
VPRecipeBase *Recipe = nullptr) { | ||||||
auto *VPB = new VPBasicBlock(Name, Recipe); | ||||||
CreatedBlocks.push_back(VPB); | ||||||
return VPB; | ||||||
} | ||||||
|
||||||
/// Create a new VPRegionBlock with \p Entry, \p Exiting and \p Name. If \p | ||||||
/// IsReplicator is true, the region is a replicate region. The returned block | ||||||
/// is owned by the VPlan and deleted once the VPlan is destroyed. | ||||||
VPRegionBlock *createVPRegionBlock(VPBlockBase *Entry, VPBlockBase *Exiting, | ||||||
const std::string &Name = "", | ||||||
bool IsReplicator = false) { | ||||||
auto *VPB = new VPRegionBlock(Entry, Exiting, Name, IsReplicator); | ||||||
CreatedBlocks.push_back(VPB); | ||||||
return VPB; | ||||||
} | ||||||
|
||||||
/// Create a new VPRegionBlock with \p Name and entry and exiting blocks set | ||||||
/// to nullptr. If \p IsReplicator is true, the region is a replicate region. | ||||||
/// The returned block is owned by the VPlan and deleted once the VPlan is | ||||||
/// destroyed. | ||||||
VPRegionBlock *createVPRegionBlock(const std::string &Name = "", | ||||||
bool IsReplicator = false) { | ||||||
auto *VPB = new VPRegionBlock(Name, IsReplicator); | ||||||
CreatedBlocks.push_back(VPB); | ||||||
return VPB; | ||||||
} | ||||||
|
||||||
/// Create a VPIRBasicBlock from \p IRBB containing VPIRInstructions for all | ||||||
/// instructions in \p IRBB, except its terminator which is managed in VPlan. | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
(the block is managed in VPlan, as explained next, rather than its terminator). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This part refers to the fact that the terminator is dropped and managed via VPlan successors. Maybe needs clarifying? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yes, better then to say something like "managed by the successors of VPlan's block". Although VPlan does employ a terminator recipe when a block has more than a single successor? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Clarified thanks |
||||||
/// The returned block is owned by the VPlan and deleted once the VPlan is | ||||||
/// destroyed. | ||||||
VPIRBasicBlock *createVPIRBasicBlock(BasicBlock *IRBB); | ||||||
}; | ||||||
|
||||||
#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such construction of VPBB's seems complementary of
VPBuilder
,VPlanHCFGBuilder
,VPRecipeBuilder
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, but in this case it is directly tied to VPlan so we can track liveness conveniently; we could track liveness separately from VPlan, but then we would have 2 data structures to keep in sync and to delete together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This thought can be pursued as follow-up.
A builder can be initialized with VPlan. The builder would provide an API for creating blocks (including but not limited to), whose implementation registers the new block within VPlan. Would be good to create VPlan components consistently, rather than having the abovementioned four distinct mechanisms.