[PATCH] D137245: [AArch64][SVE2] Add the SVE2.1 quadword variants of ld1w/ld1d/st1w/st1d

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:1299
+  let Predicates = [HasSVE2p1] in {
+  def ST1W_Q  : sve_mem_cst_ss_base<0b1000, "st1w", Z_q, GPR64NoXZRshifted32>;
+  }
----------------
As with previous comments I'm guessing you only using this so you don't get the alias, but I think we're ok having the alias.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:5760-5766
+multiclass sve_mem_128b_cst_si<bits<2> msz, bits<2> esz, string asm>
+{
+  def NAME : sve_mem_cst_si<msz, esz, asm, Z_q>;
+
+  def : InstAlias<asm # "\t$Zt, $Pg, [$Rn]",
+                  (!cast<Instruction>(NAME) Z_q:$Zt, PPR3bAny:$Pg, GPR64sp:$Rn, 0), 1>;
+}
----------------
Do we need this class?  It looks like the only difference is the support aliases, but I don't see a reason the quad case need to be more restrictive.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:9213-9215
+class sve_mem_128b_cld_si<bits<2> dtype, string mnemonic>
+    : I<(outs Z_q:$Zt), (ins PPR3bAny:$Pg, GPR64sp:$Rn, simm4s1:$imm4),
+        mnemonic, "\t$Zt, $Pg/z, [$Rn, $imm4, mul vl]",
----------------
Given they're structurally identical what about extending `sve_mem_cst_si` to include an extra `q` bit.  This will also mean the new instructions have a matching set of InstAliases. This also matches the path you've been able to take regarding the stores.


================
Comment at: llvm/lib/Target/AArch64/SVEInstrFormats.td:9242
+// SVE contiguous load (quadwords, scalar plus scalar)
+class sve_mem_128b_cld_ss<bits<2> dtype, string mnemonic, RegisterOperand gprsh_ty>
+    : I<(outs Z_q:$Zt), (ins PPR3bAny:$Pg, GPR64sp:$Rn, gprsh_ty:$Rm),
----------------
Perhaps just extend `sve_mem_cld_ss_base`? I'd be tempted to extend `sve_mem_cld_ss` also, but that's up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137245