[PATCH] D137561: [AArch64][SVE2] Add the SVE2.1 pmov instructions

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:9289
+// SVE move predicate from vector
+class sve2p1_vector_to_pred<bits<2> opc, bits<2> opc2, string mnemonic,
+                            PPRRegOp ppr_ty, Operand itype>
----------------
Please can you join these into a single 4-bit opcode?


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:9306-9316
+class sve2p1_vector_to_pred_byte<string mnemonic>
+    : I<(outs PPR8:$Pd), (ins ZPRAny:$Zn),
+        mnemonic, "\t$Pd, $Zn",
+        "", []>, Sched<[]> {
+  bits<4> Pd;
+  bits<5> Zn;
+  let Inst{31-10} = 0b0000010100101010001110;
----------------
I think you might not need this based on https://developer.arm.com/documentation/ddi0602/2022-09/SVE-Instructions/PMOV--to-predicate---Move-predicate-from-vector-.

It says `When the predicate element specifier is.B, each bit [N] from the least-significant VL/8 bits in the source vector register is copied to bit [N] of the destination predicate register. The immediate index, if specified, must be 0.`

Note the last part.  This suggests `PMOV <Pd>.B, <Zn>[0]` is allowed, but `PMOV <Pd>.B, <Zn>` is the preferred disassembly.  I know the alias isn't shown but the text seems clear and favourable because you can just add/implement `VectorIndex032b` along with an InastAlias rather than needing special instruction classes.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:9336
+// SVE move predicate into vector
+class sve2p1_pred_to_vector<bits<2> opc, bits<2> opc2, string mnemonic,
+                            PPRRegOp ppr_ty, Operand itype>
----------------
Please can you join these into a single 4-bit opcode?


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:9347-9348
+  let Inst{18-17} = opc2;
+  let Inst{16-10} = 0b1001110;
+  let Inst{9}     = 0b0;
+  let Inst{8-5}   = Pn;
----------------
Not sure why the docs split this but feel free to join them if you want.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137561