[PATCH] D136150: [AArch64]SME2 Single and Multi vector Shift and Multiply instructions

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

A couple of small requests but otherwise looks good.



================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:3591
+multiclass sme2_sat_shift_vector_vg2<string mnemonic, bit op, bit u> {
+  def _S : sme2_sat_shift_vector_vg2<mnemonic, op, u>;
+}
----------------
This should be `_H`? because we typically reference the result element type, like you've done for the vg4 variants.


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:3604-3605
+  let Inst{23-22} = sz;
+  let Inst{21}    = 0b1;
+  let Inst{15-11} = 0b11011;
+  let Inst{10}    = n;
----------------
I'm not a fan of leaving holes in `Inst` like this.  However, in this instance the hole is probably a bit big to hide within an opcode parameter so perhaps just add a comment like
```
// Inst{20-16} = imm5;
```


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:3613
+multiclass sme2_sat_shift_vector_vg4<string mnemonic, bits<3> op> {
+  def _B : sme2_sat_shift_vector_vg4<{0,1}, op{2}, op{1-0}, ZPR8, ZZZZ_s_mul_r, vecshiftR32,
+                                 mnemonic>{
----------------
Can this be kept as `op` and the split done within `sme2_sat_shift_vector_vg4`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136150/new/

https://reviews.llvm.org/D136150