[PATCH] D121624: [VPlan] Model pre-header explicitly.
Ayal Zaks via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sun Apr 3 14:49:05 PDT 2022
Ayal added inline comments.
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:3103
ReplaceInstWithInst(LoopMiddleBlock->getTerminator(), BrInst);
// Update dominator for loop exit.
----------------
Worth commenting, say in the summary, that the skeleton is left creating only the preheader and the middle-block (and the scalar loop preheader), with the "vector.body" header-block now being created by VPlan::execute?
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:7575
// 1. Create a new empty loop. Unlink the old loop and connect the new one.
VPTransformState State{BestVF, BestUF, LI, DT, ILV.Builder, &ILV, &BestVPlan};
----------------
Comment deserves updating?
================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:8709
// Create initial VPlan skeleton, with separate header and latch blocks.
VPBasicBlock *HeaderVPBB = new VPBasicBlock();
----------------
fhahn wrote:
> Ayal wrote:
> > Comment deserves updating.
> >
> > Start by creating Preheader, then Plan, and later insert TopRegion after Preheader?
> Added comment with up-to-date info about the initial skeleton.
> Start by creating Preheader, then Plan, and later insert TopRegion after Preheader?
suggestion was, i.e., to have
```
VPBasicBlock *Preheader = new VPBasicBlock("vector.ph");
auto Plan = std::make_unique<VPlan>(Preheader);
VPBasicBlock *HeaderVPBB = new VPBasicBlock();
VPBasicBlock *LatchVPBB = new VPBasicBlock("vector.latch");
VPBlockUtils::insertBlockAfter(LatchVPBB, HeaderVPBB);
auto *TopRegion = new VPRegionBlock(HeaderVPBB, LatchVPBB, "vector loop");
VPBlockUtils::insertBlockAfter(TopRegion, Preheader);
```
================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:491
+ BasicBlock *VectorPH =
+ State->CFG.VPBB2IRBB[getSinglePredecessor()->getExitBasicBlock()];
+ Loop *ParentLoop = State->LI->getLoopFor(VectorPH);
----------------
VPBB2IRBB[getPreheaderVPBB()]? (suggested above)
================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:1460
+ VPBasicBlock *PreheaderVPBB =
+ getParent()->getSingleHierarchicalPredecessor()->getExitBasicBlock();
+ EntryPart->addIncoming(Start, State.CFG.VPBB2IRBB[PreheaderVPBB]);
----------------
getPreheaderBBFor()?
================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:249
+BasicBlock *VPTransformState::CFGState::getPreheaderBBFor(VPBasicBlock *VPBB) {
+ return VPBB2IRBB[VPBB->getSingleHierarchicalPredecessor()
+ ->getExitBasicBlock()];
----------------
This assumes VPBB is the header.
Can alternatively climb from VPBB to its enclosing loop region (ensure there is one), and take its predecessor; that will only assume VPBB belongs to a loop, which is implied by asking for its preheader.
All uses actually have a recipe to provide, can take it's parent VPBB here.
Perhaps a loop region should provide its preheader VPBB?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121624/new/
https://reviews.llvm.org/D121624
More information about the llvm-commits
mailing list