-
Notifications
You must be signed in to change notification settings - Fork 13.6k
[VPlan] Introduce all loop regions as VPlan transform. (NFC) #129402
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
Changes from 10 commits
d5ba9a3
9854453
a88f03e
646a112
29f6ddf
2a500dd
cc81801
2a8344e
67a30f1
3b1618e
2d3f087
c6903d6
decf567
29b7487
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 | ||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -14,26 +14,93 @@ | |||||||||||||||||||||||||||||||||||||
#include "LoopVectorizationPlanner.h" | ||||||||||||||||||||||||||||||||||||||
#include "VPlan.h" | ||||||||||||||||||||||||||||||||||||||
#include "VPlanCFG.h" | ||||||||||||||||||||||||||||||||||||||
#include "VPlanDominatorTree.h" | ||||||||||||||||||||||||||||||||||||||
#include "VPlanTransforms.h" | ||||||||||||||||||||||||||||||||||||||
#include "llvm/Analysis/LoopInfo.h" | ||||||||||||||||||||||||||||||||||||||
#include "llvm/Analysis/ScalarEvolution.h" | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
using namespace llvm; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
void VPlanTransforms::introduceTopLevelVectorLoopRegion( | ||||||||||||||||||||||||||||||||||||||
VPlan &Plan, Type *InductionTy, PredicatedScalarEvolution &PSE, | ||||||||||||||||||||||||||||||||||||||
bool RequiresScalarEpilogueCheck, bool TailFolded, Loop *TheLoop) { | ||||||||||||||||||||||||||||||||||||||
// TODO: Generalize to introduce all loop regions. | ||||||||||||||||||||||||||||||||||||||
auto *HeaderVPBB = cast<VPBasicBlock>(Plan.getEntry()->getSingleSuccessor()); | ||||||||||||||||||||||||||||||||||||||
VPBlockUtils::disconnectBlocks(Plan.getEntry(), HeaderVPBB); | ||||||||||||||||||||||||||||||||||||||
/// Checks if \p HeaderVPB is a loop header block in the plain CFG; that is, it | ||||||||||||||||||||||||||||||||||||||
/// has exactly 2 predecessors (preheader and latch), where the block | ||||||||||||||||||||||||||||||||||||||
/// dominates the latch and the preheader dominates the block. If it is a | ||||||||||||||||||||||||||||||||||||||
/// header block, returns a pair with the corresponding preheader and latch | ||||||||||||||||||||||||||||||||||||||
/// blocks. Otherwise return std::nullopt. | ||||||||||||||||||||||||||||||||||||||
static std::optional<std::pair<VPBasicBlock *, VPBasicBlock *>> | ||||||||||||||||||||||||||||||||||||||
getPreheaderAndLatch(VPBlockBase *HeaderVPB, const VPDominatorTree &VPDT) { | ||||||||||||||||||||||||||||||||||||||
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. How about having the plain CFG-based VPlan which mirrors the IR have consistent recipes and CFG where the predecessors of header blocks are ordered preheader, latch? That would simplify a 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 means effectively going back to the orignial verison, which enforced the specific order? The current version moves the canonicalization of order outside the CFG construction, and leaves the plain CFG a 1-1 mapping of the incomig CFG, with both phi recipes and VPBB predecessor order matching the order in the original CFG. 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! |
||||||||||||||||||||||||||||||||||||||
ArrayRef<VPBlockBase *> Preds = HeaderVPB->getPredecessors(); | ||||||||||||||||||||||||||||||||||||||
if (Preds.size() != 2) | ||||||||||||||||||||||||||||||||||||||
return std::nullopt; | ||||||||||||||||||||||||||||||||||||||
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 |
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
VPBasicBlock *OriginalLatch = | ||||||||||||||||||||||||||||||||||||||
cast<VPBasicBlock>(HeaderVPBB->getSinglePredecessor()); | ||||||||||||||||||||||||||||||||||||||
VPBlockUtils::disconnectBlocks(OriginalLatch, HeaderVPBB); | ||||||||||||||||||||||||||||||||||||||
VPBasicBlock *VecPreheader = Plan.createVPBasicBlock("vector.ph"); | ||||||||||||||||||||||||||||||||||||||
VPBlockUtils::connectBlocks(Plan.getEntry(), VecPreheader); | ||||||||||||||||||||||||||||||||||||||
assert(OriginalLatch->getNumSuccessors() == 0 && | ||||||||||||||||||||||||||||||||||||||
"Plan should end at top level latch"); | ||||||||||||||||||||||||||||||||||||||
auto *PreheaderVPBB = cast<VPBasicBlock>(Preds[0]); | ||||||||||||||||||||||||||||||||||||||
auto *LatchVPBB = cast<VPBasicBlock>(Preds[1]); | ||||||||||||||||||||||||||||||||||||||
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
or are the casts 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. Removed, thanks |
||||||||||||||||||||||||||||||||||||||
if (VPDT.dominates(PreheaderVPBB, HeaderVPB) && | ||||||||||||||||||||||||||||||||||||||
VPDT.dominates(HeaderVPB, LatchVPBB)) | ||||||||||||||||||||||||||||||||||||||
return {std::make_pair(PreheaderVPBB, LatchVPBB)}; | ||||||||||||||||||||||||||||||||||||||
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 |
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
std::swap(PreheaderVPBB, LatchVPBB); | ||||||||||||||||||||||||||||||||||||||
if (VPDT.dominates(PreheaderVPBB, HeaderVPB) && | ||||||||||||||||||||||||||||||||||||||
VPDT.dominates(HeaderVPB, LatchVPBB)) | ||||||||||||||||||||||||||||||||||||||
return {std::make_pair(PreheaderVPBB, LatchVPBB)}; | ||||||||||||||||||||||||||||||||||||||
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! |
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
return std::nullopt; | ||||||||||||||||||||||||||||||||||||||
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! |
||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
/// Create a new VPRegionBlock if there is a loop starting at \p HeaderVPB. | ||||||||||||||||||||||||||||||||||||||
static void createLoopRegion(VPlan &Plan, VPBlockBase *HeaderVPB, | ||||||||||||||||||||||||||||||||||||||
VPDominatorTree &VPDT) { | ||||||||||||||||||||||||||||||||||||||
auto Res = getPreheaderAndLatch(HeaderVPB, VPDT); | ||||||||||||||||||||||||||||||||||||||
if (!Res) | ||||||||||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
const auto &[PreheaderVPBB, LatchVPBB] = *Res; | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
// Swap the operands of header phis if needed. After creating the region, the | ||||||||||||||||||||||||||||||||||||||
// incoming value from the preheader must be the first operand and the one | ||||||||||||||||||||||||||||||||||||||
// from the latch must be the second operand. | ||||||||||||||||||||||||||||||||||||||
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. The consistency is between recipes and CFG, so better set it correctly before introducing regions/HCFG? (Specifically, between operands of phi recipes and predecessors, and between conditions/cases of branch/switch recipes and successors). 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. Currently it is consistent in the plain CFG, and needs to be update here to match the new order of predecessors introduced here. |
||||||||||||||||||||||||||||||||||||||
if (HeaderVPB->getPredecessors()[0] != PreheaderVPBB) { | ||||||||||||||||||||||||||||||||||||||
for (VPRecipeBase &R : cast<VPBasicBlock>(HeaderVPB)->phis()) { | ||||||||||||||||||||||||||||||||||||||
VPValue *Inc0 = R.getOperand(0); | ||||||||||||||||||||||||||||||||||||||
R.setOperand(0, R.getOperand(1)); | ||||||||||||||||||||||||||||||||||||||
R.setOperand(1, Inc0); | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
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 |
||||||||||||||||||||||||||||||||||||||
VPBlockUtils::disconnectBlocks(PreheaderVPBB, HeaderVPB); | ||||||||||||||||||||||||||||||||||||||
VPBlockUtils::disconnectBlocks(LatchVPBB, HeaderVPB); | ||||||||||||||||||||||||||||||||||||||
VPBlockBase *Succ = LatchVPBB->getSingleSuccessor(); | ||||||||||||||||||||||||||||||||||||||
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! |
||||||||||||||||||||||||||||||||||||||
assert(LatchVPBB->getNumSuccessors() <= 1 && | ||||||||||||||||||||||||||||||||||||||
"Latch has more than one successor"); | ||||||||||||||||||||||||||||||||||||||
if (Succ) | ||||||||||||||||||||||||||||||||||||||
VPBlockUtils::disconnectBlocks(LatchVPBB, Succ); | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
auto *R = Plan.createVPRegionBlock(HeaderVPB, LatchVPBB, "", | ||||||||||||||||||||||||||||||||||||||
false /*isReplicator*/); | ||||||||||||||||||||||||||||||||||||||
R->setParent(HeaderVPB->getParent()); | ||||||||||||||||||||||||||||||||||||||
// All VPBB's reachable shallowly from HeaderVPB belong to top level loop, | ||||||||||||||||||||||||||||||||||||||
// because VPlan is expected to end at top level latch disconnected above. | ||||||||||||||||||||||||||||||||||||||
for (VPBlockBase *VPBB : vp_depth_first_shallow(HeaderVPB)) | ||||||||||||||||||||||||||||||||||||||
VPBB->setParent(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. What about setting the parent of the newly introduced region itself? 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! |
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
VPBlockUtils::insertBlockAfter(R, PreheaderVPBB); | ||||||||||||||||||||||||||||||||||||||
if (Succ) | ||||||||||||||||||||||||||||||||||||||
VPBlockUtils::connectBlocks(R, Succ); | ||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
void VPlanTransforms::createLoopRegions(VPlan &Plan, Type *InductionTy, | ||||||||||||||||||||||||||||||||||||||
PredicatedScalarEvolution &PSE, | ||||||||||||||||||||||||||||||||||||||
bool RequiresScalarEpilogueCheck, | ||||||||||||||||||||||||||||||||||||||
bool TailFolded, Loop *TheLoop) { | ||||||||||||||||||||||||||||||||||||||
VPDominatorTree VPDT; | ||||||||||||||||||||||||||||||||||||||
VPDT.recalculate(Plan); | ||||||||||||||||||||||||||||||||||||||
for (VPBlockBase *HeaderVPB : vp_depth_first_shallow(Plan.getEntry())) | ||||||||||||||||||||||||||||||||||||||
createLoopRegion(Plan, HeaderVPB, VPDT); | ||||||||||||||||||||||||||||||||||||||
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
? Otherwise createLoopRegion() should more accurately be called tryToCreateLoopRegion()... 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 the name to |
||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
VPRegionBlock *TopRegion = Plan.getVectorLoopRegion(); | ||||||||||||||||||||||||||||||||||||||
auto *OrigExiting = TopRegion->getExiting(); | ||||||||||||||||||||||||||||||||||||||
VPBasicBlock *LatchVPBB = Plan.createVPBasicBlock("vector.latch"); | ||||||||||||||||||||||||||||||||||||||
VPBlockUtils::insertBlockAfter(LatchVPBB, OrigExiting); | ||||||||||||||||||||||||||||||||||||||
TopRegion->setExiting(LatchVPBB); | ||||||||||||||||||||||||||||||||||||||
Comment on lines
+93
to
+96
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 additional latch block really needed, or can it be removed (independently)? 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 unsed only for convenience, when adjusting reductions it is used to place selects there if needed. I'll look into remove it separartely. 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 a comment, along with explaining that this section assigns distinct names to the (just created) Top Region and its latch block, which is introduced as a convenience. 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! |
||||||||||||||||||||||||||||||||||||||
TopRegion->setName("vector loop"); | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
// Create SCEV and VPValue for the trip count. | ||||||||||||||||||||||||||||||||||||||
// We use the symbolic max backedge-taken-count, which works also when | ||||||||||||||||||||||||||||||||||||||
|
@@ -47,18 +114,9 @@ void VPlanTransforms::introduceTopLevelVectorLoopRegion( | |||||||||||||||||||||||||||||||||||||
Plan.setTripCount( | ||||||||||||||||||||||||||||||||||||||
vputils::getOrCreateVPValueForSCEVExpr(Plan, TripCount, SE)); | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
// Create VPRegionBlock, with existing header and new empty latch block, to be | ||||||||||||||||||||||||||||||||||||||
// filled. | ||||||||||||||||||||||||||||||||||||||
VPBasicBlock *LatchVPBB = Plan.createVPBasicBlock("vector.latch"); | ||||||||||||||||||||||||||||||||||||||
VPBlockUtils::insertBlockAfter(LatchVPBB, OriginalLatch); | ||||||||||||||||||||||||||||||||||||||
auto *TopRegion = Plan.createVPRegionBlock( | ||||||||||||||||||||||||||||||||||||||
HeaderVPBB, LatchVPBB, "vector loop", false /*isReplicator*/); | ||||||||||||||||||||||||||||||||||||||
// All VPBB's reachable shallowly from HeaderVPBB belong to top level loop, | ||||||||||||||||||||||||||||||||||||||
// because VPlan is expected to end at top level latch. | ||||||||||||||||||||||||||||||||||||||
for (VPBlockBase *VPBB : vp_depth_first_shallow(HeaderVPBB)) | ||||||||||||||||||||||||||||||||||||||
VPBB->setParent(TopRegion); | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
VPBlockUtils::insertBlockAfter(TopRegion, VecPreheader); | ||||||||||||||||||||||||||||||||||||||
VPBasicBlock *VecPreheader = Plan.createVPBasicBlock("vector.ph"); | ||||||||||||||||||||||||||||||||||||||
VPBlockUtils::insertBlockAfter(VecPreheader, Plan.getEntry()); | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
VPBasicBlock *MiddleVPBB = Plan.createVPBasicBlock("middle.block"); | ||||||||||||||||||||||||||||||||||||||
VPBlockUtils::insertBlockAfter(MiddleVPBB, TopRegion); | ||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||
|
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.
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.
Done thanks!