This is an archive of the discontinued LLVM Phabricator instance.

[SVE][CodeGen] Add bfloat16 support to scalable masked gather
ClosedPublic

Authored by kmclaughlin on Dec 15 2020, 8:35 AM.

Diff Detail

Event Timeline

kmclaughlin created this revision.Dec 15 2020, 8:35 AM
kmclaughlin requested review of this revision.Dec 15 2020, 8:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 15 2020, 8:35 AM
c-rhodes added inline comments.Dec 15 2020, 8:56 AM
llvm/test/CodeGen/AArch64/sve-masked-scatter-legalise.ll
28–36

Do we need a similar test for gather in test/CodeGen/AArch64/sve-masked-gather-legalize.ll? I also noticed these test files differ between American/British spelling of legalize/legalise in the filename, can this be renamed to use legalize?

david-arm added inline comments.Dec 15 2020, 9:02 AM
llvm/test/CodeGen/AArch64/sve-masked-scatter-legalise.ll
28–36

+1 for the gather-legalize.ll tests.

david-arm added inline comments.Dec 16 2020, 1:16 AM
llvm/test/CodeGen/AArch64/sve-masked-scatter-legalise.ll
28–36

Hi @c-rhodes, I wonder if it's better to do the renaming in a different patch because @kmclaughlin has made changes in this file too and it makes the diff a bit harder to read? Any new files created can be with the American spelling.

c-rhodes added inline comments.Dec 16 2020, 2:36 AM
llvm/test/CodeGen/AArch64/sve-masked-scatter-legalise.ll
28–36

Hi @c-rhodes, I wonder if it's better to do the renaming in a different patch because @kmclaughlin has made changes in this file too and it makes the diff a bit harder to read? Any new files created can be with the American spelling.

Sure, I think it's fine to push an NFC patch with that single change straight to main.

kmclaughlin added inline comments.Dec 16 2020, 2:52 AM
llvm/test/CodeGen/AArch64/sve-masked-scatter-legalise.ll
28–36

Hi @c-rhodes & @david-arm, I tried adding more tests to sve-masked-gather-legalize.ll for floating point after your comments and ran into some legalisation issues when the result of the gather needs to be split. This was for all floating-point types, not just bfloat so I think this should be addressed separately to this patch.
Once this lands I'm happy to push a patch changing the name of this to sve-masked-scatter-legalize.ll

This revision is now accepted and ready to land.Dec 16 2020, 2:57 AM
  • Added patterns for bfloat16 extract_subvector to AArch64SVEInstrInfo.td
david-arm accepted this revision.Dec 16 2020, 5:16 AM

Happy with latest version. Looks good to go!