This is an archive of the discontinued LLVM Phabricator instance.

[VPlan] Initial modeling of middle block in VPlan.
ClosedPublic

Authored by fhahn on Apr 9 2022, 9:40 AM.

Details

Summary

This patch extends the scope of VPlan to also include the exit (aka
middle) block.

For now, the exit block remains empty, but handling of exit values will
subsequently be moved to VPlan, by adding recipes to model exit values
in the exit block.

As a first step, this will allow fixing #51366.

Diff Detail

Event Timeline

fhahn created this revision.Apr 9 2022, 9:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 9 2022, 9:40 AM
fhahn requested review of this revision.Apr 9 2022, 9:40 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 9 2022, 9:40 AM
Herald added a subscriber: vkmr. · View Herald Transcript
Ayal accepted this revision.Apr 12 2022, 11:39 PM

Nice preparation for expanding VPlan's scope to include exit/middle-block after its expansion to include preheader.
Looks good to me, adding minor nits.

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
3673

Calls for some VPLoopRegion2IRLoop API?

3871

Can getEnclosingLoopRegion() of PhiR's parent? (it can go up to Plan to retrieve/assert it)

8769

nit: "MiddleBlock" >> "MiddleVPBB" (or "ExitVPBB") for consistency?

llvm/lib/Transforms/Vectorize/VPlan.cpp
327–356

Deserves a comment, potentially starting with "0."

nit: can retain indentation as it should be an "else if"; perhaps better to switch and deal with exit block afterwards (should fail B) for a simpler "else if"("reusing" existing IR Exit Block can then be documented as "D")

This revision is now accepted and ready to land.Apr 12 2022, 11:39 PM
fhahn marked 4 inline comments as done.Apr 20 2022, 11:35 AM
fhahn added inline comments.
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
3673

Sounds good, there's at least one other potential user. I'll do it as follow-up.

3871

Updated to use getEnclosingLoopRegion, thanks!

8769

Renamed, thanks!

llvm/lib/Transforms/Vectorize/VPlan.cpp
327–356

Updated to reduce the indent and updated the comment. I added a separate comment for re-using ExitBB, as this is different to re-using the last BB. I also retained the oder of checks as this keeps the existing conditions simpler.

This revision was landed with ongoing or failed builds.Apr 20 2022, 11:35 AM
This revision was automatically updated to reflect the committed changes.
fhahn marked 4 inline comments as done.