[PATCH] D136088: [AArch64]SME2 instructions that use ZTO operand

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:2399
+// ZERO instructions.
+class sme2_zero_zt<string mnemonic>
+    : I<(outs ZTR:$ZT), (ins ),
----------------
Can this class represent "SME2 zero lookup table"? then change the one use to pass in `opc`.


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:2408
+// SME2 lookup table load/store
+class sme2_spill_fill_vector<string mnemonic, bit isStore>
+    : I<!if(isStore, (outs ), (outs ZTR:$ZTt)),
----------------
Please pass in `opc` and `opc2` so this class better represents the encoding group.


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:2434
+  let Inst{14-12} = imm3;
+  let Inst{11-5}  = 0b0011111;
+  let Inst{4-0}   = Rt;
----------------
Please pass this in as a 7 bit opc to match the encoding group.


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:2446
+  let Inst{14-12} = imm3;
+  let Inst{11-5}  = 0b0011111;
+  let Inst{4-0}   = Rt;
----------------
Please pass this in as a 7 bit opc to match the encoding group.


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:2451
+//===----------------------------------------------------------------------===//
+// SME2 Lookup Table Expand
+class sme2_luti_vector_index<bits<2> sz, RegisterOperand vector_ty,
----------------
SME2 Lookup Table Expand One Register


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:2452
+// SME2 Lookup Table Expand
+class sme2_luti_vector_index<bits<2> sz, RegisterOperand vector_ty,
+                             AsmVectorIndexOpnd index_ty, string mnemonic>
----------------
Can you pass in `opc` and `opc2` so this class better represents the encoding group?


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:2471
+  bits<4> i;
+  let Inst{18}    = 0b1;
+  let Inst{17-14} = i;
----------------
With the above suggestion you can use the `{1, ?, ?...}` syntax when passing the opcode into `sme2_luti_vector_index`.

The following `let Inst{17-14} = i` is fine as is.

See `sve_int_bin_pred_shift_imm_left` for what I mean. Your code placement is fine (i.e. there's no need to move anything into `multiclass sme2_luti2_vector_index` to match the shift example).


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:2494
+}
+class sme2_luti_vector_vg2_index<bits<2> sz, RegisterOperand vector_ty,
+                                 AsmVectorIndexOpnd index_ty, string mnemonic>
----------------
Please add a comment containing the name of the encoding group.

Much like my comment on `sme2_luti2_vector_index` can you pass in `opc` and `opc2` so this class better represents the encoding group.


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:2539
+
+class sme2_luti_vector_vg4_index<bits<2> sz, RegisterOperand vector_ty,
+                                 AsmVectorIndexOpnd index_ty, string mnemonic>
----------------
Same comments as with `sme2_luti_vector_vg2_index`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136088