[PATCH] D138570: [AArch64] Add patterns for SVE predicated add/sub and mov combine

Mon Nov 28 08:32:46 PST 2022

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

Thanks @NicolaLancellotti. I still think it would be better to canonicalise to two variants (i.e. add/sub op, #1) instead of the current three, but that can be changed later is it turns out be advantageous.

================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:3151-3152
+
+  def : Pat<(nxv16i8 (sub ZPR:$op, (sext (nxv16i1 PPR:$pred)))),
+            (SUB_ZPmZ_B PPR:$pred, ZPR:$op, (DUP_ZI_B 255, 0))>;
+  def : Pat<(nxv8i16 (sub ZPR:$op, (sext (nxv8i1 PPR:$pred)))),
----------------
NicolaLancellotti wrote:
> paulwalker-arm wrote:
> > Presumably you'll need `zext` variants of these patterns before reverting you're previous patch?
> > 
> > Perhaps it's worth canonicalising the constant just in case there are multiple instances of these operations.  By which I mean:
> > ```
> > add ZPR:$op, (sext ($pred)) -> SUB $op, $pred, DUP(1)
> > add ZPR:$op, (zext ($pred)) -> ADD $op, $pred, DUP(1)
> > sub ZPR:$op, (sext ($pred)) -> ADD $op, $pred, DUP(1)
> > sub ZPR:$op, (zext ($pred)) -> SUB $op, $pred, DUP(1)
> > ```
> > 
> As you can see from the tests, instead of subtracting 1, LLVM adds -1, which is the same.
> So, we don't need them, do we?
I see what you mean.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138570