This is an archive of the discontinued LLVM Phabricator instance.

[Vectorize] Fix vectorization, scalarization and folding of llvm.is.fpclass
ClosedPublic

Authored by foad on Apr 21 2023, 1:54 AM.

Details

Summary

llvm.is.fpclass is different from other vectorizable intrinsics in that
it is overloaded on an argument type, not on the return type.

Diff Detail

Event Timeline

foad created this revision.Apr 21 2023, 1:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 21 2023, 1:54 AM
foad requested review of this revision.Apr 21 2023, 1:54 AM
foad updated this revision to Diff 515709.Apr 21 2023, 6:18 AM

Fold D148803 into this one.

foad added a reviewer: hokein.Apr 21 2023, 6:21 AM
foad retitled this revision from [Vectorize] Fix vectorization of llvm.is.fpclass to [Vectorize] Fix vectorization and folding of llvm.is.fpclass.
fhahn added a comment.Apr 21 2023, 7:08 AM

Similar to D132344, but it look like this patch has stalled.

llvm/lib/Analysis/VectorUtils.cpp
131

I'm not sure about the change in the default here, IICU this is now considering any other intrinsic to have the return type overloaded?

llvm/test/Transforms/LoopVectorize/ARM/is_fpclass.ll
4 ↗(On Diff #515648)

The triple doesn't match the sub-directory, aarch64- needs to be in the AArch64 subdirectory.

But does the test need to be target specific in the first place? Could you just use -force-vector-width=4 -force-vector-interleave=1 or something, without a triple?

Also, would be good to add the test separately.

foad added inline comments.Apr 21 2023, 7:24 AM
llvm/lib/Analysis/VectorUtils.cpp
131

Yes. All callers of isVectorIntrinsicWithOverloadTypeAtArg were already making this assumption implicitly. I have changed them to make an explicit check for isVectorIntrinsicWithOverloadTypeAtArg(ID, -1).

llvm/test/Transforms/LoopVectorize/ARM/is_fpclass.ll
4 ↗(On Diff #515648)

Thanks. I didn't know how to remove the triple. I will try the -force-vector-* options.

foad updated this revision to Diff 515731.Apr 21 2023, 7:46 AM

Rebase on target-independent loop vectorize test.

Are there any test for the Scalarizer and SLP vectorizer yet? Similar to D124358.

foad updated this revision to Diff 516336.Apr 24 2023, 3:08 AM

Add tests for scalarizer and SLP vectorizer.

foad retitled this revision from [Vectorize] Fix vectorization and folding of llvm.is.fpclass to [Vectorize] Fix vectorization, scalarization and folding of llvm.is.fpclass.

Are there any test for the Scalarizer and SLP vectorizer yet? Similar to D124358.

Added them, thanks!

foad updated this revision to Diff 516345.Apr 24 2023, 3:25 AM

Rebase.

fhahn accepted this revision.Apr 24 2023, 4:23 AM

LGTM, thanks!

llvm/lib/Transforms/Vectorize/VPlanRecipes.cpp
485

Could you add a comment here with an explanation? I.e. we are adding the return type if the intrinsic is overloaded on it.

Might be good to add this in all updated places in the patch.

This revision is now accepted and ready to land.Apr 24 2023, 4:23 AM
This revision was landed with ongoing or failed builds.Apr 24 2023, 5:42 AM
This revision was automatically updated to reflect the committed changes.
foad marked an inline comment as done.Apr 24 2023, 5:42 AM