Page MenuHomePhabricator

[VPlan] Use VPValue def for VPWidenGEPRecipe.
ClosedPublic

Authored by fhahn on Jul 27 2020, 10:39 AM.

Details

Summary

This patch turns VPWidenGEPRecipe into a VPValue and uses it
during VPlan construction and codegeneration instead of the plain IR
reference where possible.

Diff Detail

Event Timeline

fhahn created this revision.Jul 27 2020, 10:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 27 2020, 10:39 AM
dmgreen added inline comments.
llvm/lib/Transforms/Vectorize/VPlan.h
944

Should this be called getUnderlyingGEP? Or should they standardize on getUnderlyingInstruction? If so I might recommend getUnderlyingInstr at least to cut down the lengths of the lines, without losing much by way of readability.

fhahn updated this revision to Diff 296244.Oct 5 2020, 11:31 AM

Addressed comments, thanks!

fhahn added inline comments.Oct 5 2020, 11:32 AM
llvm/lib/Transforms/Vectorize/VPlan.h
944

I think it's probably easiest to standardize on getUnderlyingInstr. I updated D84680 to add getUnderlyingInstr to VPRecipeBase, which allows us to get rid of the code here. The GEP pointer (vs the Instruction ptr) is only needed once, and most recipes are similar in that respect.

dmgreen accepted this revision.Oct 6 2020, 1:32 AM

LGTM along with the others

llvm/lib/Transforms/Vectorize/VPlan.cpp
127–128

Lets try and remove this again sooner rather than later ;)

llvm/lib/Transforms/Vectorize/VPlan.h
924

-> VPRecipeBase::VPWidenGEPSC

944

Yeah OK. One cast isn't too bad.

This revision is now accepted and ready to land.Oct 6 2020, 1:32 AM
fhahn added inline comments.Oct 6 2020, 1:49 AM
llvm/lib/Transforms/Vectorize/VPlan.cpp
127–128

Yeah, toVPValue and toVPUser are just convenient for the transition period. They can be removed as soon as the transition is complete (D88379, D88378)

fhahn updated this revision to Diff 305355.Sun, Nov 15, 6:57 AM

address remaining comment, rebase

This revision was landed with ongoing or failed builds.Sun, Nov 15, 7:13 AM
This revision was automatically updated to reflect the committed changes.