This is an archive of the discontinued LLVM Phabricator instance.

[LV] Move code to place pointer induction increment to VPlan post-processing.
ClosedPublic

Authored by fhahn on Mar 14 2022, 9:57 AM.

Details

Summary

This patch moves the code to set the correct incoming block for the
backedge value to VPlan::execute.

When generating the phi node, the backedge value is temporarily added
using the pre-header as incoming block. The invalid phi node will be
fixed up during VPlan::execute after main VPlan code generation.
At the same time, the backedge value is also moved to the latch.

This change removes the requirement to create the latch block up-front
for VPWidenInductionPHIRecipe::execute, which in turn will enable
modeling the pre-header in VPlan.

Depends on D121617.

Diff Detail

Event Timeline

fhahn created this revision.Mar 14 2022, 9:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2022, 9:57 AM
fhahn requested review of this revision.Mar 14 2022, 9:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2022, 9:57 AM
Herald added a subscriber: vkmr. · View Herald Transcript
Ayal added inline comments.Mar 20 2022, 4:02 PM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
9604–9605

nit: set Instruction *InductionLoc = &*State.Builder.GetInsertPoint() ?

9616

Deserves a comment that this is temporary?

llvm/lib/Transforms/Vectorize/VPlan.cpp
983–984

Above TODO is obsolete?

992

An induction phi feeding only scalar users deserves a separate, non-'Widen', VP[Replicate]PointerInductionRecipe ?

fhahn updated this revision to Diff 418600.Mar 28 2022, 8:42 AM
fhahn marked 4 inline comments as done.

Address latest comments, thanks!

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

Added a similar comment to D121617

llvm/lib/Transforms/Vectorize/VPlan.cpp
983–984

Yes, removed!

992

Agreed, I added a todo.

Ayal accepted this revision.Mar 29 2022, 11:15 AM

Looks fine to me, with a couple of nits.

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

nit: can NewPointerPhi be used instead of CanonicalIV, which seems less relevant?

llvm/lib/Transforms/Vectorize/VPlan.cpp
990

nit: reverse the condition to handle the simpler int or fp case first.

This revision is now accepted and ready to land.Mar 29 2022, 11:15 AM
This revision was landed with ongoing or failed builds.Mar 29 2022, 12:29 PM
This revision was automatically updated to reflect the committed changes.
fhahn marked 2 inline comments as done.

Thanks, nits should be addressed in the committed version!