This is an archive of the discontinued LLVM Phabricator instance.

[AArch64]SME2 Multiple vectors Int/FP clamp instructions for two/four registers
ClosedPublic

Authored by CarolineConcatto on Oct 10 2022, 9:32 AM.

Details

Summary

This patch adds the assembly/disassembly for the following instruction:
Int:

SCLAMP:Multi-vector signed clamp to minimum/maximum vector.
UCLAMP:Multi-vector unsigned clamp to minimum/maximum vector.

FP:

FCLAMP: Multi-vector floating-point clamp to minimum/maximum number.

The reference can be found here:

  https://developer.arm.com/documentation/ddi0602/2022-09

  Depends on: D135563

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptOct 10 2022, 9:32 AM
CarolineConcatto requested review of this revision.Oct 10 2022, 9:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 10 2022, 9:32 AM
Matt added a subscriber: Matt.Oct 10 2022, 10:53 PM
CarolineConcatto edited the summary of this revision. (Show Details)Oct 21 2022, 6:56 AM
llvm/lib/Target/AArch64/SMEInstrFormats.td
1893

The base class shows value here but what about making op1 a two bit opcode?

llvm/lib/Target/AArch64/SMEInstrFormats.td
1893

I did not do this because this class is also used by:
sme2_int_clamp_vector_vg2_multi
that needs to set the value o u in AArch64SMEInstrInfo.td.

I can change class sme2_clamp_vector_vg2_multi, and then change the multiclass sme2_int_clamp_vector_vg2_multi to be
multiclass sme2_int_clamp_vector_vg2_multi<string mnemonic, bits<2> u>
instead of only
multiclass sme2_int_clamp_vector_vg2_multi<string mnemonic, bit u>
If you see value in doing that.

paulwalker-arm added inline comments.Oct 22 2022, 5:17 AM
llvm/lib/Target/AArch64/SMEInstrFormats.td
1893

I'm not sure where u comes into it. I am referring to the fact that sme2_clamp_vector_vg24_multi does not set Inst{11}, however both sme2_clamp_vector_vg2_multi and sme2_clamp_vector_vg4_multi set Inst{11} and thus I'm proposing that rather than them setting it explicitly you could just make op1 represent Inst{11--10}?

  • Address review comments
CarolineConcatto marked an inline comment as done.Oct 24 2022, 1:45 AM
CarolineConcatto added inline comments.
llvm/lib/Target/AArch64/SMEInstrFormats.td
1893

Sorry Paul,
I misunderstood what you wanted. Thank you for making it clear.
I hope I addressed it correctly.

paulwalker-arm accepted this revision.EditedOct 24 2022, 7:36 AM

Happy as is but I offer some advice that I think means supporting ZIP and UZP would just require a couple of extra lines plus some tests.

llvm/lib/Target/AArch64/SMEInstrFormats.td
1903

A bit like with D135599, you'll be better placed passing this into here as a single opcode (along with inst{12}. Doing this means you can reuse the same class for "SME2 multi-vec ZIP two registers" that also sits within this encoding group.

This revision is now accepted and ready to land.Oct 24 2022, 7:36 AM
CarolineConcatto marked an inline comment as done.
  • rebase
This revision was landed with ongoing or failed builds.Oct 25 2022, 1:12 AM
This revision was automatically updated to reflect the committed changes.