This is an archive of the discontinued LLVM Phabricator instance.

[AArch64][SVE] Guard perm and select bfloat16 intrinsic patterns
ClosedPublic

Authored by c-rhodes on Jun 24 2020, 12:25 PM.

Details

Summary

Permutation and selection bfloat16 intrinsic patterns should be guarded
on the feature flag +bf16. Missed in D82182 and D80850.

Diff Detail

Event Timeline

c-rhodes created this revision.Jun 24 2020, 12:25 PM
Herald added a project: Restricted Project. · View Herald Transcript
fpetrogalli requested changes to this revision.Jun 24 2020, 12:42 PM

Marking as "request changes" to make sure we don't forget to discuss the "nit" :)

llvm/test/CodeGen/AArch64/sve-intrinsics-perm-select-matmul-fp64.ll
1

Nit: given that the fact that we have added the bfloat tests in the same file as the non-bfloat tests might have contributes to overseeing the problem, I wonder if it make sense to use the same approach we have done for the ACLE tests and have the tests requiring +bf16 to be extracted into a separate *-bfloat file.

This revision now requires changes to proceed.Jun 24 2020, 12:42 PM
fpetrogalli added inline comments.Jun 24 2020, 9:07 PM
llvm/test/CodeGen/AArch64/sve-intrinsics-perm-select-matmul-fp64.ll
1

FWIW, we could keep everything in one test without adding the extra +bf16 at command line invocation and by using a function attribute attached to the bfloat functions, as I have done in https://reviews.llvm.org/D82429

c-rhodes added inline comments.Jun 25 2020, 1:56 AM
llvm/test/CodeGen/AArch64/sve-intrinsics-perm-select-matmul-fp64.ll
1

FWIW, we could keep everything in one test without adding the extra +bf16 at command line invocation and by using a function attribute attached to the bfloat functions

I like this approach, I'll go with that

c-rhodes updated this revision to Diff 273324.Jun 25 2020, 6:01 AM

Changes:

  • Enable +bf16 with function attribute.
c-rhodes marked an inline comment as done.Jun 25 2020, 6:01 AM
fpetrogalli accepted this revision.Jun 25 2020, 7:52 AM

LGTM, thank you!

This revision is now accepted and ready to land.Jun 25 2020, 7:52 AM
This revision was automatically updated to reflect the committed changes.