[PATCH] D135599: [AArch64]SME2 Single and multiple vectors Int/FP min/max two/four registers

[Caroline via Phabricator via llvm-commits]

CarolineConcatto marked an inline comment as done.
CarolineConcatto added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64SMEInstrInfo.td:374-384
+defm SMAX_VG2_2ZZ  : sme2_int_max_min_vector_vg2_single<"smax", 0b00>;
+defm SMAX_VG4_4ZZ  : sme2_int_max_min_vector_vg4_single<"smax", 0b00>;
+defm SMAX_VG2_2Z2Z : sme2_int_max_min_vector_vg2_multi<"smax",  0b00>;
+defm SMAX_VG4_4Z2Z : sme2_int_max_min_vector_vg4_multi<"smax",  0b00>;
+
+defm UMAX_VG2_2ZZ  : sme2_int_max_min_vector_vg2_single<"umax", 0b01>;
+defm UMAX_VG4_4ZZ  : sme2_int_max_min_vector_vg4_single<"umax", 0b01>;
----------------
paulwalker-arm wrote:
> Just related to layout but it's more typical to group instructions based on the multiclass rather instruction name.
Do you want me to group them in this patch by group of instructions?
Sorry, but I don't understand what you would like me to fix.
I can group them here, and do another NFC for the other instructions, otherwise things will get a bit moew random.


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:1831-1832
+
+class sme2_max_min_vector_vg24<bits<2> sz, bit f, bits<2> op,
+                               RegisterOperand vector_ty,
+                               RegisterOperand zpr_ty, string mnemonic>
----------------
paulwalker-arm wrote:
> There doesn't look to be much commonality between the derived classes and the splitting of the `Inst` fields makes it hard to spot bugs.  I guess I'm asking why the base class is advantageous over implementing `sme2_max_min_vector_vg2_single`, `sme2_max_min_vector_vg4_single`,... as isolated instruction classes.
I created a class for each group now.
 I hope that is better now to review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135599