[PATCH] D121621: [VPlan] Track current vector loop in VPTransformState (NFC).
Ayal Zaks via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 29 12:02:11 PDT 2022
Ayal accepted this revision.
Ayal added a comment.
This revision is now accepted and ready to land.
This is fine; adding a clarifying comment.
================
Comment at: llvm/lib/Transforms/Vectorize/VPlan.cpp:914
+ State->CurrentVectorLoop = L;
State->CFG.LastBB = L->getExitBlock();
----------------
fhahn wrote:
> Ayal wrote:
> > Ayal wrote:
> > > Is CFG.LastBB now redundant, retrievable from CurrentVectorLoop->getExitBlock()?
> > > (Or could CurrentVectorLoop be retrieved from LastBB'd predecessor, if unique.)
> > (State->CurrentVectorLoop itself is also caching of State->LI->getLoopFor(State->CFG.VectorHeaderBB))
> > Is CFG.LastBB now redundant, retrievable from CurrentVectorLoop->getExitBlock()?
>
> Unfortunately this only works if we maintain a correct branch to the exit block throughout VPlan execution. But we change the header branch to unreachable, so the won't work I think.
>
> > (Or could CurrentVectorLoop be retrieved from LastBB'd predecessor, if unique.)
> > (State->CurrentVectorLoop itself is also caching of State->LI->getLoopFor(State->CFG.VectorHeaderBB))
>
> The main motivation for this change is to be able to create new loop objects for each non-replicator region during VPRegionBlock::execute. At that point, no basic blocks for the loop exist and when creating the first block in the loop (in VPBasicBlock::execute), there's no previous block we could use to get the loop object. We could change VPRegionBlock::execute to create a dummy block on entry, but keeping track of the loop object explicitly seems cleaner to me.
>
(This now relates to CFG.ExitBB instead of CFG.LastBB)
Ok, right; disconnecting the branch from VectorHeaderBB to middleBlock below will invalidate L->getExitBlock(), so ExitBB needs to be recorded above.
The current skeleton still creates VectorPreHeaderBB, VectorHeaderBB, and ExitBB/middleBlock, right? That's how L is retrieved above; the comment only clarifies that CurrentVectorLoop currently caches this retrieval (which should still be valid via the cumbersome
State->LI->getLoopFor(State->CFG.VectorPreHeader->getSingleSuccessor()))).
When that changes, and VectorHeaderBB is created during VPlan::execute() etc., the "Assumes a single LoopVectorBody basic-block was created" comment above should also be updated.
(In any case, no dummy blocks intended)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D121621/new/
https://reviews.llvm.org/D121621
More information about the llvm-commits
mailing list