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

[Bradley Smith via Phabricator via llvm-commits]

bsmith 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);
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119424