[PATCH] D136680: [AArch64][SVE2] Add the SVE2.1 contiguous load to multiple consecutive vector

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:3598
+// Load to two registers
+def LD1B_2ZCXX    : sve2p1_mem_cld_ss_2z<"ld1b", 0b00, 0b0, ZZ_b_mul_r, GPR64shifted8>;
+def LD1H_2ZCXX    : sve2p1_mem_cld_ss_2z<"ld1h", 0b01, 0b0, ZZ_h_mul_r, GPR64shifted16>;
----------------
It looks like we were not as consistent with adding register suffixes for the SVE loads and store as with the other instructions.  I see two options:

1. Match the existing naming but include register counts (e.g. something like `LD1B_2Z and LD1B_2Z_IMM`, or
2. Reference all operand registers as you've done but include the zeroing aspect like we usually do for predicated instructions (i.e.  `LD1B_2ZCzXX and LD1B_2ZCzXI`).

Personally I prefer option 1 but the choice is yours.


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:3884-3899
+  // Not all predicates are followed by a '/z'.
+  MCAsmParser &Parser = getParser();
+  if (Parser.getTok().isNot(AsmToken::Slash))
+    return MatchOperand_Success;
+
+  // But when they do they shouldn't have an element type suffix.
+  if (!Kind.empty()) {
----------------
Is there now sufficient commonality that you can reuse `tryParseSVEPredicateVector` and just pass in the register kind or have that as a template parameter?  I know this only requires "\z" support, but I don't think that really matters does it?


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

https://reviews.llvm.org/D136680