[PATCH] D139637: [AArch64][SVE][ISel] Combine dup of load to replicating load

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm accepted this revision.
paulwalker-arm added a comment.
This revision is now accepted and ready to land.

Some test recommendations but otherwise looks good.



================
Comment at: llvm/test/CodeGen/AArch64/sve-ld1r.ll:843
+    %ld = load i8, i8* %addr
+    %res = call <vscale x 16 x i8> @llvm.aarch64.sve.dup.nxv16i8(<vscale x 16 x i8> undef, <vscale x 16 x i1> %pg, i8 %ld)
+    ret <vscale x 16 x i8> %res
----------------
To cover all the new variants you also want sign/zext forms for the integer types.


================
Comment at: llvm/test/CodeGen/AArch64/sve-ld1r.ll:873
+}
+define <vscale x 8 x half> @dup_ld1rqh_half_passthruundef(<vscale x 8 x i1> %pg, half* %addr) {
+; CHECK-LABEL: dup_ld1rqh_half_passthruundef:
----------------
To cover all the new variants you also want unpacked forms for the floating-point types.


================
Comment at: llvm/test/CodeGen/AArch64/sve-ld1r.ll:963-973
+define <vscale x 16 x i8> @dupx_ld1rqb_i8_immlo(i8* %addr) {
+; CHECK-LABEL: dupx_ld1rqb_i8_immlo:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    ptrue p0.b
+; CHECK-NEXT:    ld1rb { z0.b }, p0/z, [x0, #1]
+; CHECK-NEXT:    ret
+    %gep = getelementptr i8, i8* %addr, i32 1
----------------
I don't think `dup.x` tests are necessary because we don't have any isel patterns for them. Instead we always lower such intrinsics to `splat_vector`, which is already tested?

That said, the `dup.x` tests are validating the immediate range so perhaps they can be converted to use `dup` into undef. That that said, I suppose there's an argument that given the immediate operand type is shared by the other pattern within the multiclass it's already being sufficiently tested. It's up to you depending on how paranoid you want to be. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139637