[PATCH] D127275: [MVE] Fold fadd(select(..., +0.0)) into a predicated fadd

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

> I'm assuming that for MVE an fadd of +0 for a given lane is equivalent to simply not performing the fadd for that lane (i.e. by using predication).

Yes, but I'm not sure how that matters. `fadd -0.0, 0.0` should produce `0.0`. The same should be true if the first argument is in a variable holding -0.0, and the second is from a select that could be 0.0 - it should give the output 0.0, not of the variable.



================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:16709
 
   // The identity element for a fadd is -0.0, which these VMOV's represent.
   auto isNegativeZeroSplat = [&](SDValue Op) {
----------------
Can you update this comment. The Lambda name is no longer correct too.


================
Comment at: llvm/lib/Target/ARM/ARMISelLowering.cpp:16715
+    uint64_t ImmVal = Op.getOperand(0).getConstantOperandVal(0);
+    if (VT == MVT::v4f32 && (ImmVal == 1664 || !ImmVal))
       return true;
----------------
ImmVal == 0 would show the intent a little better.


================
Comment at: llvm/test/CodeGen/Thumb2/mve-pred-selectop3.ll:375
+  %c = call <4 x i1> @llvm.arm.mve.vctp32(i32 %n)
+  %a = select <4 x i1> %c, <4 x float> %y, <4 x float> <float 0.000000e+00, float 0.000000e+00, float 0.000000e+00, float 0.000000e+00>
+  %b = fadd <4 x float> %a, %x
----------------
Can you add nsz variants of the tests and make sure it is only those that get folded.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127275