Page MenuHomePhabricator

[AArch64][SVE] Improve codegen for select nodes with fixed types
ClosedPublic

Authored by bsmith on Mar 26 2021, 8:01 AM.

Details

Summary

Add support for lowering select nodes with fixed SVE types.

Additionally, move the existing fixed vselect tests to *-vselect.ll.

Diff Detail

Event Timeline

bsmith created this revision.Mar 26 2021, 8:01 AM
bsmith requested review of this revision.Mar 26 2021, 8:01 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 26 2021, 8:01 AM
paulwalker-arm added inline comments.Mar 30 2021, 2:32 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
7037

Is the call to LowerFixedLengthVectorSelectToSVE strictly necessary? I would have thought return DAG.getNode(ISD::VSELECT.... would be sufficient. We'll then iterate back into the custom lowering code, if required, to lower the VSELECT.

bsmith updated this revision to Diff 334113.Mar 30 2021, 5:02 AM
bsmith marked an inline comment as done.
  • Remove superfluous call to LowerFixedLengthVectorSelectToSVE
llvm/test/CodeGen/AArch64/sve-fixed-length-fp-select.ll
57

Sanity check: should this be [[PRES]] ?

bsmith updated this revision to Diff 334140.Mar 30 2021, 6:38 AM
bsmith marked an inline comment as done.
  • Fixed incorrect parameter in tests
peterwaller-arm accepted this revision.Mar 30 2021, 7:04 AM
This revision is now accepted and ready to land.Mar 30 2021, 7:04 AM
paulwalker-arm added inline comments.Mar 31 2021, 2:53 AM
llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
7035

Is this safe? I know for larger than NEON vectors we'll truncate the mask and thus only care about the bottom bit, however, you're tests show that for NEON sized vectors bif is used and so every bit of the mask is meaningful. I believe this is because it expects the mask to be extended based on setBooleanVectorContents(ZeroOrNegativeOneBooleanContent). Which means a sign extend must be used here to maintain that requirement?

bsmith updated this revision to Diff 334442.Mar 31 2021, 7:53 AM
  • Use sext for predicate mask extension, rather than anyext
bsmith marked an inline comment as done.Mar 31 2021, 7:53 AM
paulwalker-arm accepted this revision.Apr 1 2021, 6:44 AM