Page MenuHomePhabricator

[VPlan] Use defined and ops VPValues to print VPInterleaveRecipe.
ClosedPublic

Authored by fhahn on Jul 31 2021, 8:16 AM.

Details

Summary

This patch updates VPInterleaveRecipe::print to print the actual defined
VPValues for load groups and the store VPValue operands for store
groups.

The IR references may become outdated while transforming the VPlan and
the defined and stored VPValues always are up-to-date.

Diff Detail

Event Timeline

fhahn created this revision.Jul 31 2021, 8:16 AM
fhahn requested review of this revision.Jul 31 2021, 8:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2021, 8:16 AM
Herald added a subscriber: vkmr. · View Herald Transcript
Ayal added inline comments.Aug 1 2021, 11:13 PM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
9552

Would have been nice to iterate over the recipe's operands and/or def VPValues, instead of calling getOperand()/getVPValue() for each, but the former does not tell where the gaps are.

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

How about checking instead if the number of store operands is positive using
int getNumStoreOperands() const { return getNumOperands() - (HasMask ? 2 : 1); }
This can also serve getStoredValues(), so that isStore() isn't only an ifndef NDEBUG thing for printing.
Can alternatively have the constructor initialize a constant field, as the number of store operands does not change.

fhahn updated this revision to Diff 363486.Aug 2 2021, 8:35 AM

Replace isStore with getNumStoreOperands as suggested by Ayal, thanks!

Ayal accepted this revision.Aug 2 2021, 9:34 AM

Looks good to me, thanks!

This revision is now accepted and ready to land.Aug 2 2021, 9:34 AM
This revision was landed with ongoing or failed builds.Aug 2 2021, 10:39 AM
This revision was automatically updated to reflect the committed changes.
fhahn added inline comments.Aug 2 2021, 2:12 PM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
9552

Agreed! Perhaps it would be worth adding that information to the recipe, removing the requirement to reference the interleave group?

Ayal added inline comments.Aug 2 2021, 11:53 PM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
9552

Agreed! It would be better to have the recipe record that information (and avoid keeping IG altogether), rather than rely on whether IG->getMember(i) returns an Instruction* or null, but refrain from using that Instruction.