-
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] Remove loop region in optimizeForVFAndUF. #108378
Changes from 7 commits
d3614bc
5f8fabe
c68ddd1
f0421c6
4a0eb12
9499aaa
a4843b5
f5d2bc6
1222e23
706b681
71436fc
38cbdf6
cc43362
e758945
d4e8c7e
1d4b2e6
98529f3
0c76e9d
dd45cad
e72a71f
407dbc1
af48fcc
f51412a
f2b5e53
1f4febc
3d412a1
56a4181
88fcf60
303ce93
3f016cb
430c369
f9db2d0
cabc591
057f2e9
60a046a
fe2c3a5
26c94b1
92c0ccc
05b2e4e
df67f2e
e17003f
f180edb
71ff80a
911c50a
4f74827
d17571d
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 | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -221,9 +221,10 @@ VPBasicBlock::iterator VPBasicBlock::getFirstNonPhi() { | |||||||
|
||||||||
VPTransformState::VPTransformState(ElementCount VF, unsigned UF, LoopInfo *LI, | ||||||||
DominatorTree *DT, IRBuilderBase &Builder, | ||||||||
InnerLoopVectorizer *ILV, VPlan *Plan) | ||||||||
InnerLoopVectorizer *ILV, VPlan *Plan, | ||||||||
Type *CanonicalIVTy) | ||||||||
: VF(VF), CFG(DT), LI(LI), Builder(Builder), ILV(ILV), Plan(Plan), | ||||||||
LVer(nullptr), TypeAnalysis(Plan->getCanonicalIV()->getScalarType()) {} | ||||||||
LVer(nullptr), TypeAnalysis(CanonicalIVTy) {} | ||||||||
|
||||||||
Value *VPTransformState::get(VPValue *Def, const VPLane &Lane) { | ||||||||
if (Def->isLiveIn()) | ||||||||
|
@@ -452,7 +453,6 @@ void VPBasicBlock::connectToPredecessors(VPTransformState::CFGState &CFG) { | |||||||
CFG.DTU.applyUpdates({{DominatorTree::Insert, PredBB, NewBB}}); | ||||||||
} | ||||||||
} | ||||||||
|
||||||||
void VPIRBasicBlock::execute(VPTransformState *State) { | ||||||||
assert(getHierarchicalSuccessors().size() <= 2 && | ||||||||
"VPIRBasicBlock can have at most two successors at the moment!"); | ||||||||
|
@@ -960,7 +960,6 @@ void VPlan::prepareToExecute(Value *TripCountV, Value *VectorTripCountV, | |||||||
|
||||||||
IRBuilder<> Builder(State.CFG.PrevBB->getTerminator()); | ||||||||
// FIXME: Model VF * UF computation completely in VPlan. | ||||||||
assert(VFxUF.getNumUsers() && "VFxUF expected to always have users"); | ||||||||
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. Is this related? 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. We may remove the users (the canonical IV increment). Updated to account for that in assertion |
||||||||
unsigned UF = getUF(); | ||||||||
if (VF.getNumUsers()) { | ||||||||
Value *RuntimeVF = getRuntimeVF(Builder, TCTy, State.VF); | ||||||||
|
@@ -1028,8 +1027,8 @@ void VPlan::execute(VPTransformState *State) { | |||||||
// skeleton creation, so we can only create the VPIRBasicBlocks now during | ||||||||
// VPlan execution rather than earlier during VPlan construction. | ||||||||
BasicBlock *MiddleBB = State->CFG.ExitBB; | ||||||||
VPBasicBlock *MiddleVPBB = getMiddleBlock(); | ||||||||
BasicBlock *ScalarPh = MiddleBB->getSingleSuccessor(); | ||||||||
VPBasicBlock *MiddleVPBB = getMiddleBlock(); | ||||||||
replaceVPBBWithIRVPBB(getScalarPreheader(), ScalarPh); | ||||||||
replaceVPBBWithIRVPBB(MiddleVPBB, MiddleBB); | ||||||||
|
||||||||
|
@@ -1050,53 +1049,59 @@ void VPlan::execute(VPTransformState *State) { | |||||||
for (VPBlockBase *Block : vp_depth_first_shallow(Entry)) | ||||||||
Block->execute(State); | ||||||||
|
||||||||
VPBasicBlock *LatchVPBB = getVectorLoopRegion()->getExitingBasicBlock(); | ||||||||
BasicBlock *VectorLatchBB = State->CFG.VPBB2IRBB[LatchVPBB]; | ||||||||
|
||||||||
// Fix the latch value of canonical, reduction and first-order recurrences | ||||||||
// phis in the vector loop. | ||||||||
VPBasicBlock *Header = getVectorLoopRegion()->getEntryBasicBlock(); | ||||||||
for (VPRecipeBase &R : Header->phis()) { | ||||||||
// Skip phi-like recipes that generate their backedege values themselves. | ||||||||
if (isa<VPWidenPHIRecipe>(&R)) | ||||||||
continue; | ||||||||
|
||||||||
if (isa<VPWidenPointerInductionRecipe>(&R) || | ||||||||
isa<VPWidenIntOrFpInductionRecipe>(&R)) { | ||||||||
PHINode *Phi = nullptr; | ||||||||
if (isa<VPWidenIntOrFpInductionRecipe>(&R)) { | ||||||||
Phi = cast<PHINode>(State->get(R.getVPSingleValue())); | ||||||||
} else { | ||||||||
auto *WidenPhi = cast<VPWidenPointerInductionRecipe>(&R); | ||||||||
assert(!WidenPhi->onlyScalarsGenerated(State->VF.isScalable()) && | ||||||||
"recipe generating only scalars should have been replaced"); | ||||||||
auto *GEP = cast<GetElementPtrInst>(State->get(WidenPhi)); | ||||||||
Phi = cast<PHINode>(GEP->getPointerOperand()); | ||||||||
} | ||||||||
|
||||||||
Phi->setIncomingBlock(1, VectorLatchBB); | ||||||||
if (auto *LoopRegion = | ||||||||
dyn_cast<VPRegionBlock>(getEntry()->getSingleSuccessor())) { | ||||||||
VPBasicBlock *LatchVPBB = LoopRegion->getExitingBasicBlock(); | ||||||||
BasicBlock *VectorLatchBB = State->CFG.VPBB2IRBB[LatchVPBB]; | ||||||||
|
||||||||
// Fix the latch value of canonical, reduction and first-order recurrences | ||||||||
// phis in the vector loop. | ||||||||
VPBasicBlock *Header = LoopRegion->getEntryBasicBlock(); | ||||||||
for (VPRecipeBase &R : Header->phis()) { | ||||||||
// Skip phi-like recipes that generate their backedege values themselves. | ||||||||
if (isa<VPWidenPHIRecipe>(&R)) | ||||||||
continue; | ||||||||
|
||||||||
// Move the last step to the end of the latch block. This ensures | ||||||||
// consistent placement of all induction updates. | ||||||||
Instruction *Inc = cast<Instruction>(Phi->getIncomingValue(1)); | ||||||||
Inc->moveBefore(VectorLatchBB->getTerminator()->getPrevNode()); | ||||||||
if (isa<VPWidenPointerInductionRecipe>(&R) || | ||||||||
isa<VPWidenIntOrFpInductionRecipe>(&R)) { | ||||||||
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! |
||||||||
PHINode *Phi = nullptr; | ||||||||
if (isa<VPWidenIntOrFpInductionRecipe>(&R)) { | ||||||||
Phi = cast<PHINode>(State->get(R.getVPSingleValue())); | ||||||||
} else { | ||||||||
auto *WidenPhi = cast<VPWidenPointerInductionRecipe>(&R); | ||||||||
assert(!WidenPhi->onlyScalarsGenerated(State->VF.isScalable()) && | ||||||||
"recipe generating only scalars should have been replaced"); | ||||||||
auto *GEP = cast<GetElementPtrInst>(State->get(WidenPhi)); | ||||||||
Phi = cast<PHINode>(GEP->getPointerOperand()); | ||||||||
} | ||||||||
|
||||||||
Phi->setIncomingBlock(1, VectorLatchBB); | ||||||||
|
||||||||
// Move the last step to the end of the latch block. This ensures | ||||||||
// consistent placement of all induction updates. | ||||||||
Instruction *Inc = cast<Instruction>(Phi->getIncomingValue(1)); | ||||||||
Inc->moveBefore(VectorLatchBB->getTerminator()->getPrevNode()); | ||||||||
|
||||||||
// Use the steps for the last part as backedge value for the induction. | ||||||||
if (auto *IV = dyn_cast<VPWidenIntOrFpInductionRecipe>(&R)) | ||||||||
Inc->setOperand(0, State->get(IV->getLastUnrolledPartOperand())); | ||||||||
continue; | ||||||||
} | ||||||||
|
||||||||
// Use the steps for the last part as backedge value for the induction. | ||||||||
if (auto *IV = dyn_cast<VPWidenIntOrFpInductionRecipe>(&R)) | ||||||||
Inc->setOperand(0, State->get(IV->getLastUnrolledPartOperand())); | ||||||||
continue; | ||||||||
// For canonical IV, first-order recurrences and in-order reduction phis, | ||||||||
// only a single part is generated, which provides the last part from the | ||||||||
// previous iteration. For non-ordered reductions all UF parts are | ||||||||
// generated. | ||||||||
auto *PhiR = cast<VPHeaderPHIRecipe>(&R); | ||||||||
bool NeedsScalar = | ||||||||
isa<VPCanonicalIVPHIRecipe, VPEVLBasedIVPHIRecipe>(PhiR) || | ||||||||
(isa<VPReductionPHIRecipe>(PhiR) && | ||||||||
cast<VPReductionPHIRecipe>(PhiR)->isInLoop()); | ||||||||
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. Replace isa+cast by dyn_cast? 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! |
||||||||
Value *Phi = State->get(PhiR, NeedsScalar); | ||||||||
Value *Val = State->get(PhiR->getBackedgeValue(), NeedsScalar); | ||||||||
cast<PHINode>(Phi)->addIncoming(Val, VectorLatchBB); | ||||||||
} | ||||||||
|
||||||||
auto *PhiR = cast<VPHeaderPHIRecipe>(&R); | ||||||||
bool NeedsScalar = | ||||||||
isa<VPCanonicalIVPHIRecipe, VPEVLBasedIVPHIRecipe>(PhiR) || | ||||||||
(isa<VPReductionPHIRecipe>(PhiR) && | ||||||||
cast<VPReductionPHIRecipe>(PhiR)->isInLoop()); | ||||||||
Value *Phi = State->get(PhiR, NeedsScalar); | ||||||||
Value *Val = State->get(PhiR->getBackedgeValue(), NeedsScalar); | ||||||||
cast<PHINode>(Phi)->addIncoming(Val, VectorLatchBB); | ||||||||
} | ||||||||
|
||||||||
State->CFG.DTU.flush(); | ||||||||
assert(State->CFG.DTU.getDomTree().verify( | ||||||||
DominatorTree::VerificationLevel::Fast) && | ||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2357,7 +2357,9 @@ void VPBranchOnMaskRecipe::execute(VPTransformState &State) { | |
// Replace the temporary unreachable terminator with a new conditional branch, | ||
// whose two destinations will be set later when they are created. | ||
auto *CurrentTerminator = State.CFG.PrevBB->getTerminator(); | ||
assert(isa<UnreachableInst>(CurrentTerminator) && | ||
assert((isa<UnreachableInst>(CurrentTerminator) || | ||
(isa<BranchInst>(CurrentTerminator) && | ||
!CurrentTerminator->getOperand(0))) && | ||
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. ? 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 with the latest version, removed, thanks |
||
"Expected to replace unreachable terminator with conditional branch."); | ||
auto *CondBr = BranchInst::Create(State.CFG.PrevBB, nullptr, ConditionBit); | ||
CondBr->setSuccessor(0, nullptr); | ||
|
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
|
@@ -692,16 +692,46 @@ void VPlanTransforms::optimizeForVFAndUF(VPlan &Plan, ElementCount BestVF, | |||||||||
!SE.isKnownPredicate(CmpInst::ICMP_ULE, TripCount, C)) | ||||||||||
return; | ||||||||||
|
||||||||||
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. Say something about what is about to happen now. 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 |
||||||||||
LLVMContext &Ctx = SE.getContext(); | ||||||||||
auto *BOC = | ||||||||||
new VPInstruction(VPInstruction::BranchOnCond, | ||||||||||
{Plan.getOrAddLiveIn(ConstantInt::getTrue(Ctx))}); | ||||||||||
|
||||||||||
SmallVector<VPValue *> PossiblyDead(Term->operands()); | ||||||||||
Term->eraseFromParent(); | ||||||||||
auto *Header = cast<VPBasicBlock>(Plan.getVectorLoopRegion()->getEntry()); | ||||||||||
if (all_of(Header->phis(), [](VPRecipeBase &R) { | ||||||||||
return !isa<VPWidenIntOrFpInductionRecipe, VPReductionPHIRecipe>(&R); | ||||||||||
})) { | ||||||||||
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. Thanks, updated and reordered conditions |
||||||||||
for (VPRecipeBase &R : make_early_inc_range(Header->phis())) { | ||||||||||
auto *P = cast<VPHeaderPHIRecipe>(&R); | ||||||||||
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 |
||||||||||
P->replaceAllUsesWith(P->getStartValue()); | ||||||||||
P->eraseFromParent(); | ||||||||||
} | ||||||||||
|
||||||||||
VPBlockBase *Preheader = Plan.getVectorLoopRegion()->getSinglePredecessor(); | ||||||||||
auto *Exiting = | ||||||||||
cast<VPBasicBlock>(Plan.getVectorLoopRegion()->getExiting()); | ||||||||||
|
||||||||||
auto *LoopRegion = Plan.getVectorLoopRegion(); | ||||||||||
VPBlockBase *Middle = LoopRegion->getSingleSuccessor(); | ||||||||||
VPBlockUtils::disconnectBlocks(Preheader, LoopRegion); | ||||||||||
VPBlockUtils::disconnectBlocks(LoopRegion, Middle); | ||||||||||
|
||||||||||
Header->setParent(nullptr); | ||||||||||
Exiting->setParent(nullptr); | ||||||||||
VPBlockUtils::connectBlocks(Preheader, Header); | ||||||||||
|
||||||||||
VPBlockUtils::connectBlocks(Exiting, Middle); | ||||||||||
// Set LoopRegion's Entry to nullptr, as the CFG from LoopRegion shouldn't | ||||||||||
// be deleted when the region is deleted. | ||||||||||
LoopRegion->clearEntry(); | ||||||||||
delete LoopRegion; | ||||||||||
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. Potentially unsafe, may leak if the programmer forgets about it. Maybe, use Destructor or something like RAII? 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. I'm not sure if there is a nice way to do so currently. The whole replace-and-erase logic in the block needs to happen atomically at the moment. To avoid having to do manual delete's, VPlan could track all blocks added to it and delete them all when the VPlan is deleted (at the moment it only deletes reachable blocks). But that would require some extra logic to make sure all blocks are added properly. |
||||||||||
} else { | ||||||||||
LLVMContext &Ctx = SE.getContext(); | ||||||||||
auto *BOC = | ||||||||||
new VPInstruction(VPInstruction::BranchOnCond, | ||||||||||
{Plan.getOrAddLiveIn(ConstantInt::getTrue(Ctx))}); | ||||||||||
|
||||||||||
ExitingVPBB->appendRecipe(BOC); | ||||||||||
} | ||||||||||
for (VPValue *Op : PossiblyDead) | ||||||||||
recursivelyDeleteDeadRecipes(Op); | ||||||||||
ExitingVPBB->appendRecipe(BOC); | ||||||||||
Plan.setVF(BestVF); | ||||||||||
Plan.setUF(BestUF); | ||||||||||
// TODO: Further simplifications are possible | ||||||||||
|
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.
Overwrite ExitVPBB set to middle block above?
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.
Name always refers to the middle block, fixed in 11c6af6, thanks