[PATCH] D119424: [AArch64][SVE] Invert VSelect operand order and condition for predicated arithmetic operations

[Matt Devereau via Phabricator via llvm-commits]

MattDevereau marked 2 inline comments as done.
MattDevereau added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17200-17201
+// (vselect (p) (a) (op (a) (b))) => (vselect (!p) (op (a) (b)) (a))
+static Optional<SDValue> tryInvertVSelectWithSetCC(SDNode *N,
+                                                   SelectionDAG &DAG) {
+  SDValue SetCC = N->getOperand(0);
----------------
bsmith wrote:
> MattDevereau wrote:
> > bsmith wrote:
> > > I believe this transform would loop given something like:
> > > 
> > > ```
> > > m = fmul a, b
> > > p = setcc <cond> m, 0
> > > vselect p, m, m
> > > ```
> > isnt `vselect p, m, m` a nop?
> > 
> > i've created a test for the example which doesn't loop
> > 
> > ```define <vscale x 4 x float> @fcmp_select_f32_double_op(<vscale x 4 x float> %a, <vscale x 4 x float> %b) {
> > ; CHECK-LABEL: fcmp_select_f32_double_op:
> > ; CHECK:       // %bb.0:
> > ; CHECK-NEXT:    fmul z0.s, z0.s, z1.s
> > ; CHECK-NEXT:    ret
> >   %m = fmul <vscale x 4 x float> %a, %b
> >   %fcmp = fcmp oeq <vscale x 4 x float> %m, zeroinitializer
> >   %sel = select <vscale x 4 x i1> %fcmp, <vscale x 4 x float> %m, <vscale x 4 x float> %m
> >   ret <vscale x 4 x float> %sel
> > }
> > ```
> It likely will get removed as redundant yes, I just worry about things like this that could end up getting through in esoteric cases.
added test `fcmp_select_f32_double_op` to `llvm/test/CodeGen/AArch64/sve-select.ll` and added condition `if (SetCCOp0 == NOp2) return None;` after condition `SetCCOp0 != NOp1`


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:17204
+  if (SetCC.getOpcode() != ISD::SETCC ||
+      SetCC.getOperand(0) != N->getOperand(1))
+    return None;
----------------
bsmith wrote:
> The comment above describing this transform isn't accurate as it doesn't reflect these restrictions around setcc.
updated comment to
// (vselect (setcc (a) (0)) (a) (op (a) (b)))
//  => (vselect (not setcc (a) (0)) (op (a) (b)) (a))


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119424