Page MenuHomePhabricator

[VPlan] Turn VPReplicateRecipe into a VPValue.
ClosedPublic

Authored by fhahn on Nov 15 2020, 9:21 AM.

Details

Summary

Update VPReplicateRecipe to inherit from VPValue. This still does not
update scalarizeInstruction to set the result for the VPValue of
VPReplicateRecipe, because this first requires tracking scalar values in
VPTransformState.

Diff Detail

Event Timeline

fhahn created this revision.Nov 15 2020, 9:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2020, 9:21 AM
fhahn requested review of this revision.Nov 15 2020, 9:21 AM
reames added a subscriber: reames.Nov 22 2020, 4:42 PM

I don't have sufficiently broad context, but to an uneducated eye, it looks really weird to have a class which can produce multiple independent instructions declared as subclass of a class named Value. Particularly since I don't think there was a required build vector step at the end of the instructions emitted?

reames accepted this revision.Nov 25 2020, 4:50 PM

Had a long conversation w/Florian today; he was nice enough to give a lot of context and general direction which helped me understand where this is going. It also helps that e0c479cd, and 00a6601 landed recently showing alignment with the direction sketched here.

Given that, LGTM

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

Out of scope for this patch, but these three really look like evidence of missing isa infrastructure for the types in question.

This revision is now accepted and ready to land.Nov 25 2020, 4:50 PM
fhahn updated this revision to Diff 307845.Nov 26 2020, 5:38 AM

rebased.

Had a long conversation w/Florian today; he was nice enough to give a lot of context and general direction which helped me understand where this is going. It also helps that e0c479cd, and 00a6601 landed recently showing alignment with the direction sketched here.

Given that, LGTM

Thanks for taking a look!

fhahn added inline comments.Nov 26 2020, 5:48 AM
llvm/lib/Transforms/Vectorize/VPlan.cpp
140

Agreed. Currently the problem is that VPRecipeBase does not inherit from VPValue or VPUser. The toVPValue helpers are there to avoid casting between a base class and a different type that's not a subtype. Both of those helpers are only used during the transition (until all recipes have been updated) and should go away soon.

This revision was landed with ongoing or failed builds.Nov 26 2020, 5:56 AM
This revision was automatically updated to reflect the committed changes.