[PATCH] D137411: [AArch64][SVE2] Add the SVE2.1 logical quadword reduction instructions

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:9208
+    : I<(outs V128:$Vd), (ins PPR3bAny:$Pg, zpr_ty:$Zn),
+        mnemonic, "\t$Vd." # vec_sfx # ", $Pg, $Zn",
+        "", []>, Sched<[]> {
----------------
paulwalker-arm wrote:
> Not your fault but this is a shame because otherwise we could just extend `sve_int_reduce`.
> 
> With that said, are you able to follow the same idiom as `sve_int_reduce` for the `QV` reductions because your current direction will mean a separate instruction class per type of reduction and I cannot immediately see a need for that.
Given they are quite different encodings does this really make sense?

These three encoding groups all differ by 3 bits on top of the opc
SVE bitwise logical reduction (quadwords): bits 21-19: 011
SVE integer add reduction (quadwords): bits 21-19: 000
SVE integer min/max reduction (quadwords): bits 21-19: 001

To be honest I'm quite confused by how the existing sve_int_reduce hierarchy is written. For example, what does the _1 in sve_int_reduce_1 mean and how does that relate to the instructions? Is it just a random suffix? Personally, if we have to go down this route I'd prefer something more meaningful than just copying how we've done this before. Would this be ok?

defm EORQV_VPZ : sve2p1_log_int_reduce_q<...>;
...
defm ADDQV_VPZ : sve2p1_arith_int_reduce_q<...>;
...
defm SMAXQV_VPZ : sve2p1_minmax_int_reduce_q<...>;

where the multiclasses look like

multiclass sve2p1_log_int_reduce_q<bits<2> opc, string mnemonic> {
  def _B : sve2p1_int_reduction_q<0b00, opc, 0b011, mnemonic, ZPR8,  "16b">;
  def _H : sve2p1_int_reduction_q<0b01, opc, 0b011, mnemonic, ZPR16, "8h">;
  def _S : sve2p1_int_reduction_q<0b10, opc, 0b011, mnemonic, ZPR32, "4s">;
  def _D : sve2p1_int_reduction_q<0b11, opc, 0b011, mnemonic, ZPR64, "2d">;
}

where 0b011 refers to bits 21-19 of the encoding group.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137411