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

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/test/tools/llvm-mca/AArch64/Neoverse/N2-sve-instructions.s:4475
 # CHECK-NEXT:  1      6     0.50    *                   ld1h	{ z5.h }, p3/z, [x17, x16, lsl #1]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rb	{ z0.b }, p0/z, [x0]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rb	{ z0.d }, p0/z, [x0]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rb	{ z0.h }, p0/z, [x0]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rb	{ z0.s }, p0/z, [x0]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rb	{ z31.b }, p7/z, [sp, #63]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rb	{ z31.d }, p7/z, [sp, #63]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rb	{ z31.h }, p7/z, [sp, #63]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rb	{ z31.s }, p7/z, [sp, #63]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rd	{ z0.d }, p0/z, [x0]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rd	{ z31.d }, p7/z, [sp, #504]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rh	{ z0.d }, p0/z, [x0]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rh	{ z0.h }, p0/z, [x0]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rh	{ z0.s }, p0/z, [x0]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rh	{ z31.d }, p7/z, [sp, #126]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rh	{ z31.h }, p7/z, [sp, #126]
-# CHECK-NEXT:  1      6     0.33    *             U     ld1rh	{ z31.s }, p7/z, [sp, #126]
+# CHECK-NEXT:  1      6     0.33    *                   ld1rb	{ z0.b }, p0/z, [x0]
+# CHECK-NEXT:  1      6     0.33    *                   ld1rb	{ z0.d }, p0/z, [x0]
----------------
Allen wrote:
> dmgreen wrote:
> > Allen wrote:
> > > Excuse me, I'm curious why the above changes affect the **SideEffects attribute** here. Would you give me some guidance? Thank you.
> > The way that hasSideEffects for an instruction works is that:
> >  - It defaults to true.
> >  - It gets set to false if there is a tablegen pattern that generated that instruction (I believe it might needs to be a single instruction generated by the pattern).
> >  - It can be overridden with `let hasSideEffects=1/0` on the instruction.
> > 
> > If you are looking to remove the side effects flags, and the instruction doesn't have a pattern, adding hasSideEffects=0 is usually a good way to go. Removing the side effects can help improve scheduling freedom and other code motion in the backend, so is usually good to do so long as the instruction doesn't do anything odd.
> Thansk @dmgreen  very much.
>    - As the above changes don't have some  overridden with `let hasSideEffects=1/0` , so I think the 2nd guidelines works in this case.
>    - Is the `def : LD1RPat<nxv4i32, load,        LD1RW_IMM,   PTRUE_S, i32, am_indexed32_6b, uimm6s4>;` before the change a tablegen pattern, and it also should set the instruction to false?
Yeah - IIRC, I think the logic in Tablegen might only apply when the pattern produces a single instruction. So the pattern with `(load (ptrue 31), ..` would not set `hasSideEffects=0` in the same way as the new `(load $pg, ..` pattern.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139637