[PATCH] D118133: [AArch64] Expand UADDLV patterns to SADDLV

Thu Jan 27 02:52:09 PST 2022

dmgreen added inline comments.

================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:5881
+}
+
+defm : SIMDAcrossLaneLongPairIntrinsic<"UADDLV", AArch64uaddlp>;
----------------
labrinea wrote:
> We seem to have patterns for all of `8B, 16B, 8H, 4H and 4S` if I read this right. I am seeing tests (in the diff at least) that only cover `4S, 8H and 16B` (datasize = 128). Do we have/need tests for the rest (datasize = 64)?
Yeah some of the extending vecreduce patterns don't simplify in the same way. Those will have to be left for another patch though. I've made sure there is test coverage.

================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:6079-6080

 defm : SIMDAcrossLanesSignedLongIntrinsic<"SADDLV", int_aarch64_neon_saddlv>;
 defm : SIMDAcrossLanesUnsignedLongIntrinsic<"UADDLV", int_aarch64_neon_uaddlv>;

----------------
labrinea wrote:
> The multiclass naming confuses me a bit. We also have these other patterns here for SADDLV and UADDLV. Not sure what would be a better naming scheme.
Yeah I just made up a name to do with Across Lane (addv) and pairwise (addlp).

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

https://reviews.llvm.org/D118133