[PATCH] D135468: [AArch64]SME2 Multiple vector ternary int/float 2 and 4 registers

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:1320
+      return DiagnosticPredicateTy::NoMatch;
+    ;
+    if (((VectorList.RegNum - AArch64::Z0) % NumRegs) != 0)
----------------
typo?


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:5531
+                      "SVE vectors, where the first vector is a multiple of 2 "
+                      "and with correct element type");
+  case Match_InvalidSVEVectorListMul4x32:
----------------
s/with correct element type/with matching element types/?


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:5536
+                      "SVE vectors, where the first vector is a multiple of 4 "
+                      "and with correct element type");
   default:
----------------
s/with correct element type/with matching element types/?


================
Comment at: llvm/lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp:637
+                                                const void *Decoder) {
+  if (RegNo * 2 > (32 - 2))
+    return Fail;
----------------
Can this just be 30 to reflect the maximum register is z30? I don't really see what value the maths gives us, it's not like the existing functions use `32 - 1`.


================
Comment at: llvm/lib/Target/AArch64/Disassembler/AArch64Disassembler.cpp:648
+                                                const void *Decoder) {
+  if (RegNo * 4 > (32 - 4))
+    return Fail;
----------------
Similar comment as above.


================
Comment at: llvm/lib/Target/AArch64/SMEInstrFormats.td:1254
+//===----------------------------------------------------------------------===//
+// SME2 multiple vectors ternary INT/FP  two and four registers
+class sme2_mul_add_sub_array_vg2_multi<bit sz, bits<2> op,
----------------
too much whitespace


================
Comment at: llvm/test/MC/AArch64/SME2/add-diagnostics.s:58-66
+add za.s[w10, 3, vgx2], {z10.s-z11.s}, {z21.s-z22.s}
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: Invalid vector list, expected list with 2 consecutive SVE vectors, where the first vector is a multiple of 2 and with correct element type
+// CHECK-NEXT: add za.s[w10, 3, vgx2], {z10.s-z11.s}, {z21.s-z22.s}
+// CHECK-NOT: [[@LINE-1]]:{{[0-9]+}}:
+
+add za.d[w11, 7, vgx4], {z12.d-z15.d}, {z9.d-z12.d}
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: Invalid vector list, expected list with 4 consecutive SVE vectors, where the first vector is a multiple of 4 and with correct element type
----------------
These test the "where the first vector is a multiple of" part of the syntax but not the "with correct element type" part.  Can you add tests for the matching element type side of thing?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135468