[DAGCombine] GetNegatedExpression - add FMA\FMAD support

[RKSimon]

Summary

If the accumulator and either of the multiply operands are negateable then we can we negate the entire expression.

[RKSimon]

Seeing a weird EXPENSIVE_CHECKS failure that I need to get to the bottom of

[RKSimon]

updated to match FMUL/FDIV approach.

[arsenm]

LGTM. Does everything still pass if you delete the FMA/FMAD case in AMDGPUTargetLowering::performFNegCombine?

[RKSimon]

In D63141#1548152, @arsenm wrote:

LGTM. Does everything still pass if you delete the FMA/FMAD case in AMDGPUTargetLowering::performFNegCombine?

Thanks for pointing that out - it made me realise there's an issue in that we're not checking for UnsafeMath/hasNoSignedZeros like we do for FADD.

[RKSimon]

Updated patch to correctly respect NSZ flags. I've also improved the logic to ensure we negate the most profitable multiplication factor, not just the first one.

[RKSimon]

Once this is in place, the plan is to start work on https://bugs.llvm.org/show_bug.cgi?id=42863 so we can hook this into propagating negation through target opcodes as well.

[arsenm]

Should this also remove the AMDGPU version?

[RKSimon]

In D63141#1641307, @arsenm wrote:

Should this also remove the AMDGPU version?

The problem we currently have is that AMDGPUTargetLowering::performFNegCombine allows multiple uses of the FMA/FMAD but the DAGCombine doesn't, so I'm going to suggest we wait until PR42863 to handle that.

[arsenm]

LGTM

llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

case ISD::FDIV:
     // Ignore X * 2.0 because that is expected to be canonicalized to X + X.
     if (auto *C = isConstOrConstSplatFP(Op.getOperand(1)))
       if (C->isExactlyValue(2.0) && Op.getOpcode() == ISD::FMUL)
         return 0;
 
     return isNegatibleForFree(Op.getOperand(1), LegalOperations, TLI, Options,
                               ForCodeSize, Depth + 1);
 
+  case ISD::FMA:
+  case ISD::FMAD: {
+    if (!Options->NoSignedZerosFPMath && !Flags.hasNoSignedZeros())
+      return 0;
+
+    // fold (fneg (fma X, Y, Z)) -> (fma (fneg X), Y, (fneg Z))
+    // fold (fneg (fma X, Y, Z)) -> (fma X, (fneg Y), (fneg Z))
+    char V2 = isNegatibleForFree(Op.getOperand(2), LegalOperations, TLI,
+                                 Options, ForCodeSize, Depth + 1);
+    if (!V2)
+      return 0;
+
+    // One of Op0/Op1 must be cheaply negatible, then select the cheapest.
+    char V0 = isNegatibleForFree(Op.getOperand(0), LegalOperations, TLI,
+                                 Options, ForCodeSize, Depth + 1);
+    char V1 = isNegatibleForFree(Op.getOperand(1), LegalOperations, TLI,
+                                 Options, ForCodeSize, Depth + 1);
+    char V01 = std::max(V0, V1);
+    return V01 ? std::max(V01, V2) : 0;
+  }
+
   case ISD::FP_EXTEND:
   case ISD::FP_ROUND:
   case ISD::FSIN:
     return isNegatibleForFree(Op.getOperand(0), LegalOperations, TLI, Options,
                               ForCodeSize, Depth + 1);
   }
 }
 
for (SDValue C : Op->op_values()) {
       }
       APFloat V = cast<ConstantFPSDNode>(C)->getValueAPF();
       V.changeSign();
       Ops.push_back(DAG.getConstantFP(V, SDLoc(Op), C.getValueType()));
     }
     return DAG.getBuildVector(Op.getValueType(), SDLoc(Op), Ops);
   }
   case ISD::FADD:
-    assert(Options.NoSignedZerosFPMath || Flags.hasNoSignedZeros());
+    assert((Options.NoSignedZerosFPMath || Flags.hasNoSignedZeros()) &&
+           "Expected NSZ fp-flag");
 
     // fold (fneg (fadd A, B)) -> (fsub (fneg A), B)
     if (isNegatibleForFree(Op.getOperand(0), LegalOperations,
                            DAG.getTargetLoweringInfo(), &Options, ForCodeSize,
                            Depth + 1))
       return DAG.getNode(ISD::FSUB, SDLoc(Op), Op.getValueType(),
                          GetNegatedExpression(Op.getOperand(0), DAG,
                                               LegalOperations, ForCodeSize,
case ISD::FDIV:
 
     // fold (fneg (fmul X, Y)) -> (fmul X, (fneg Y))
     return DAG.getNode(Op.getOpcode(), SDLoc(Op), Op.getValueType(),
                        Op.getOperand(0),
                        GetNegatedExpression(Op.getOperand(1), DAG,
                                             LegalOperations, ForCodeSize,
                                             Depth + 1), Flags);
 
+  case ISD::FMA:
+  case ISD::FMAD: {
+    assert((Options.NoSignedZerosFPMath || Flags.hasNoSignedZeros()) &&
+           "Expected NSZ fp-flag");
+
+    SDValue Neg2 = GetNegatedExpression(Op.getOperand(2), DAG, LegalOperations,
+                                        ForCodeSize, Depth + 1);
+
+    char V0 = isNegatibleForFree(Op.getOperand(0), LegalOperations,
+                                 DAG.getTargetLoweringInfo(), &Options,
+                                 ForCodeSize, Depth + 1);
+    char V1 = isNegatibleForFree(Op.getOperand(1), LegalOperations,
+                                 DAG.getTargetLoweringInfo(), &Options,
+                                 ForCodeSize, Depth + 1);
+    if (V0 >= V1) {
+      // fold (fneg (fma X, Y, Z)) -> (fma (fneg X), Y, (fneg Z))
+      SDValue Neg0 = GetNegatedExpression(
+          Op.getOperand(0), DAG, LegalOperations, ForCodeSize, Depth + 1);
+      return DAG.getNode(Op.getOpcode(), SDLoc(Op), Op.getValueType(), Neg0,
+                         Op.getOperand(1), Neg2, Flags);
+    }
+
+    // fold (fneg (fma X, Y, Z)) -> (fma X, (fneg Y), (fneg Z))
+    SDValue Neg1 = GetNegatedExpression(Op.getOperand(1), DAG, LegalOperations,
+                                        ForCodeSize, Depth + 1);
+    return DAG.getNode(Op.getOpcode(), SDLoc(Op), Op.getValueType(),
+                       Op.getOperand(0), Neg1, Neg2, Flags);
+  }
+
   case ISD::FP_EXTEND:
   case ISD::FSIN:
     return DAG.getNode(Op.getOpcode(), SDLoc(Op), Op.getValueType(),
                        GetNegatedExpression(Op.getOperand(0), DAG,
                                             LegalOperations, ForCodeSize,
                                             Depth + 1));
   case ISD::FP_ROUND:
     return DAG.getNode(ISD::FP_ROUND, SDLoc(Op), Op.getValueType(),

llvm/trunk/test/CodeGen/X86/fma-fneg-combine-2.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+avx,fma | FileCheck %s --check-prefixes=CHECK,FMA3
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -mattr=+avx,fma4 | FileCheck %s --check-prefixes=CHECK,FMA4
 
 define float @test_fneg_fma_subx_y_negz_f32(float %w, float %x, float %y, float %z)  {
 ; FMA3-LABEL: test_fneg_fma_subx_y_negz_f32:
 ; FMA3:       # %bb.0: # %entry
-; FMA3-NEXT:    vsubss %xmm1, %xmm0, %xmm0
-; FMA3-NEXT:    vfnmadd213ss {{.*#+}} xmm0 = -(xmm2 * xmm0) + xmm3
+; FMA3-NEXT:    vsubss %xmm0, %xmm1, %xmm0
+; FMA3-NEXT:    vfmadd213ss {{.*#+}} xmm0 = (xmm2 * xmm0) + xmm3
 ; FMA3-NEXT:    retq
 ;
 ; FMA4-LABEL: test_fneg_fma_subx_y_negz_f32:
 ; FMA4:       # %bb.0: # %entry
-; FMA4-NEXT:    vsubss %xmm1, %xmm0, %xmm0
-; FMA4-NEXT:    vfnmaddss %xmm3, %xmm2, %xmm0, %xmm0
+; FMA4-NEXT:    vsubss %xmm0, %xmm1, %xmm0
+; FMA4-NEXT:    vfmaddss %xmm3, %xmm2, %xmm0, %xmm0
 ; FMA4-NEXT:    retq
 entry:
   %subx = fsub nsz float %w, %x
   %negz = fsub float -0.000000e+00, %z
   %0 = tail call nsz float @llvm.fma.f32(float %subx, float %y, float %negz)
   %1 = fsub float -0.000000e+00, %0
   ret float %1
 }
 
 define float @test_fneg_fma_x_suby_negz_f32(float %w, float %x, float %y, float %z)  {
 ; FMA3-LABEL: test_fneg_fma_x_suby_negz_f32:
 ; FMA3:       # %bb.0: # %entry
-; FMA3-NEXT:    vsubss %xmm2, %xmm0, %xmm0
-; FMA3-NEXT:    vfnmadd213ss {{.*#+}} xmm0 = -(xmm1 * xmm0) + xmm3
+; FMA3-NEXT:    vsubss %xmm0, %xmm2, %xmm0
+; FMA3-NEXT:    vfmadd213ss {{.*#+}} xmm0 = (xmm1 * xmm0) + xmm3
 ; FMA3-NEXT:    retq
 ;
 ; FMA4-LABEL: test_fneg_fma_x_suby_negz_f32:
 ; FMA4:       # %bb.0: # %entry
-; FMA4-NEXT:    vsubss %xmm2, %xmm0, %xmm0
-; FMA4-NEXT:    vfnmaddss %xmm3, %xmm0, %xmm1, %xmm0
+; FMA4-NEXT:    vsubss %xmm0, %xmm2, %xmm0
+; FMA4-NEXT:    vfmaddss %xmm3, %xmm0, %xmm1, %xmm0
 ; FMA4-NEXT:    retq
 entry:
   %suby = fsub nsz float %w, %y
   %negz = fsub float -0.000000e+00, %z
   %0 = tail call nsz float @llvm.fma.f32(float %x, float %suby, float %negz)
   %1 = fsub float -0.000000e+00, %0
   ret float %1
 }
 
 define float @test_fneg_fma_subx_suby_negz_f32(float %w, float %x, float %y, float %z)  {
 ; FMA3-LABEL: test_fneg_fma_subx_suby_negz_f32:
 ; FMA3:       # %bb.0: # %entry
-; FMA3-NEXT:    vsubss %xmm1, %xmm0, %xmm1
+; FMA3-NEXT:    vsubss %xmm0, %xmm1, %xmm1
 ; FMA3-NEXT:    vsubss %xmm2, %xmm0, %xmm0
-; FMA3-NEXT:    vfnmadd213ss {{.*#+}} xmm0 = -(xmm1 * xmm0) + xmm3
+; FMA3-NEXT:    vfmadd213ss {{.*#+}} xmm0 = (xmm1 * xmm0) + xmm3
 ; FMA3-NEXT:    retq
 ;
 ; FMA4-LABEL: test_fneg_fma_subx_suby_negz_f32:
 ; FMA4:       # %bb.0: # %entry
-; FMA4-NEXT:    vsubss %xmm1, %xmm0, %xmm1
+; FMA4-NEXT:    vsubss %xmm0, %xmm1, %xmm1
 ; FMA4-NEXT:    vsubss %xmm2, %xmm0, %xmm0
-; FMA4-NEXT:    vfnmaddss %xmm3, %xmm0, %xmm1, %xmm0
+; FMA4-NEXT:    vfmaddss %xmm3, %xmm0, %xmm1, %xmm0
 ; FMA4-NEXT:    retq
 entry:
   %subx = fsub nsz float %w, %x
   %suby = fsub nsz float %w, %y
   %negz = fsub float -0.000000e+00, %z
   %0 = tail call nsz float @llvm.fma.f32(float %subx, float %suby, float %negz)
   %1 = fsub float -0.000000e+00, %0
   ret float %1