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

[David Sherwood via Phabricator via llvm-commits]

david-arm added inline comments.


================
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]",
----------------
paulwalker-arm wrote:
> 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.
I assume you mean sve_mem_cld_si_base, not sve_mem_cst_si since they are stores? If you're referring to sve_mem_cld_si_base then they're not quite structurally the same since the `dtype` field is only bits 24-23 for quadwords. By bringing them together it does become a bit confusing because treating the quadword encoding group as having a 24-21 bit dtype field leads to exactly the same dtypes we have for LD1SB (1100) and LD1SH (1000).

Reusing the classes reduces code for sure, but it might make it more confusing. I'll give it a try anyway and see how it looks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137245