[PATCH] D136759: [AArch64][SVE2] Add the SVE2.1 while & pext predicate pair instructions

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.td:970
+  let Name = "SVEPredicateList" # NumRegs # "x" # ElementWidth;
+  let DiagnosticType = "Invalid" # Name;
+  let ParserMethod = "tryParseVectorList<RegKind::SVEPredicateVector>";
----------------
It this required? I cannot see any strings for `InvalidSVEPredicateList2x8` and thus I guess there's no tests for it? I see the equivalent `ZPRVectorList` doesn't set `DiagnosticType` so perhaps it's not needed.


================
Comment at: llvm/lib/Target/AArch64/AsmParser/AArch64AsmParser.cpp:1754
 
+    assert((RegTy != VecListIdx_PReg || NumRegs <= 2) &&
+           " NumRegs must be <= 2 for PRegs");
----------------
FYI: I guess it doesn't matter but you've added partial support for single register vector lists for predicates. I'm not saying there's anything wrong I'm just making you aware just incase we see weird errors later one.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:8784-8786
+class sve2p1_int_ctr_to_mask_pair<string mnemonic, bits<2> sz, RegisterOperand pprty>
+    : I<(outs pprty:$Pd), (ins PNRAny_p8to15:$PNn, VectorIndexD:$i1),
+        mnemonic, "\t$Pd, $PNn$i1",
----------------
Rather than create a new base encoding class can you extend the existing one for `PEXT (predicate)` to match the encoding group for `SVE extract mask predicate from predicate-as-counter` and then inherit from that using the `{ 1, 0, ? }` like syntax. You already have to pass in some of the operand types to account for the element type so that'll be no different.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:9095
+
+class sve2p1_int_while_rr_pair<string mnemonic, bits<2> sz, bits<3> opc,
+                             RegisterOperand ppr_ty>
----------------
Please add a comment referencing the name of the encoding group. In this case `SVE integer compare scalar count and limit (predicate pair)`.


================
Comment at: llvm/test/MC/AArch64/SVE2p1/pext-diagnostics.s:6
+
+pext {p0.h, p2.h}, pn8[0]
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: registers must be sequential
----------------
Perhaps worth adding tests that try to use a one and four register vector list.


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

https://reviews.llvm.org/D136759