This is an archive of the discontinued LLVM Phabricator instance.

[VPlan] Helper to check if a recipe only uses scalar values of op.
ClosedPublic

Authored by fhahn on Mar 2 2022, 9:00 AM.

Details

Summary

This patch adds a helper to check if a recipe only uses scalars of a
given operand. This is similar to onlyFirstLaneUsed, which was
introduced earlier.

By default, onlyScalarsUsed falls back on onlyFirstLaneUsed.

Will be used by a follow-up patch.

Diff Detail

Event Timeline

fhahn created this revision.Mar 2 2022, 9:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2022, 9:00 AM
fhahn requested review of this revision.Mar 2 2022, 9:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2022, 9:00 AM
Herald added a subscriber: vkmr. · View Herald Transcript
Ayal added a comment.EditedMar 8 2022, 7:31 AM

How about VPScalarIVStepsRecipe, VPBlendRecipe, VPBranchOnMaskRecipe, VPPredInstPHIRecipe, VPExpandSCEVRecipe?

Is it worth distinguishing between recipes that can immediately tell they only use scalars (or only use first scalar lane) such as VPRepilcateRecipe - and those that don't care and need to check if all their users are of the former type, such as VPBlendRecipe and possibly VPPredInstPHIRecipe?

fhahn updated this revision to Diff 414061.Mar 9 2022, 4:10 AM

How about VPScalarIVStepsRecipe, VPBlendRecipe, VPBranchOnMaskRecipe, VPPredInstPHIRecipe, VPExpandSCEVRecipe?

I updated the patch to also support VPScalarIVStepsRecipe, VPPredInstPHIRecipe & VPExpandSCEVRecipe.

Is it worth distinguishing between recipes that can immediately tell they only use scalars (or only use first scalar lane) such as VPRepilcateRecipe - and those that don't care and need to check if all their users are of the former type, such as VPBlendRecipe and possibly VPPredInstPHIRecipe?

I am not sure if we need that at the moment, but it might be useful to have in the future if compile-time comes a concern for some analysis?

Ayal added inline comments.Mar 9 2022, 10:43 AM
llvm/lib/Transforms/Vectorize/VPlan.h
1645

VPPredInstPHIRecipe has only PredInst as an operand, which is surely scalar.

1780

Does VPExpandSCEVRecipe use only first lane?

1921

VPScalarIVStepsRecipe uses only the first lane of its three operands?

Ayal added a comment.Mar 9 2022, 12:14 PM

Perhaps drop "only" - "usesScalars()" instead of "onlyScalarsUsed()"? As opposed to usesVectors.

In contrast, "onlyFirstLaneUsed()" as opposed to also using 2nd, 3rd, ... VFth lanes.

fhahn updated this revision to Diff 414190.Mar 9 2022, 12:48 PM
fhahn marked an inline comment as done.

Address comments, thanks!

Perhaps drop "only" - "usesScalars()" instead of "onlyScalarsUsed()"? As opposed to usesVectors.

Adjusted, thanks!

fhahn marked 2 inline comments as done.Mar 9 2022, 12:50 PM
fhahn added inline comments.
llvm/lib/Transforms/Vectorize/VPlan.h
1645

Yes. Are you referring to the generic comment?

1780

It doesn't use any VPValue operands, so this can never be called without asserting. I remove it.

1921

Yes, I changed the code, thanks!

Ayal added inline comments.Mar 10 2022, 6:33 AM
llvm/lib/Transforms/Vectorize/VPlan.h
776

Can drop "only" from comment too (here and below).
Add "Conservatively returns if only first (scalar) lane is used, as default" ?

1645

Just clarifying the behavior here.

1922

drop " /// Conservatively returns false."

Ayal accepted this revision.Mar 10 2022, 6:40 AM

Looks good to me, with some last minor nits.
Can drop 'only' also from title.
Specifying VPScalarIVStepsRecipe::onlyFirstLaneUsed() could be committed separately, it belongs to previous patches introducing VPScalarIVStepsRecipe and/or onlyFirstLaneUsed().

This revision is now accepted and ready to land.Mar 10 2022, 6:40 AM
fhahn marked 4 inline comments as done.Mar 11 2022, 5:40 AM

Looks good to me, with some last minor nits.
Can drop 'only' also from title.

Thanks, I'll address them in the committed version.

Specifying VPScalarIVStepsRecipe::onlyFirstLaneUsed() could be committed separately, it belongs to previous patches introducing VPScalarIVStepsRecipe and/or onlyFirstLaneUsed().

Will do!

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

Will do ,thanks!

This revision was landed with ongoing or failed builds.Mar 11 2022, 5:41 AM
This revision was automatically updated to reflect the committed changes.
fhahn marked an inline comment as done.