[DAGCombiner] Initial support for the fast-math flag contract

[anemet]

Summary

Now alternatively to the TargetOption.AllowFPOpFusion global flag, FMUL->FADD
can also use the per operation FMF to allow fusion.

The idea here is not to port everything to the new scheme (e.g. fused
multiply-and-sub will be ported later) but that this work all the way from
clang.

The transformation is conditionalized on *both* the FADD and the FMUL having
the FMF contract flag.

[anemet]

Ping

[arsenm]

test/CodeGen/AArch64/neon-fma-FMF.ll

5–6 ↗

(On Diff #92425)

Needs tests where only the fmul or fadd are marked contract

[anemet]

Address Matt's comment

[anemet]

Fix typo

[anemet]

Does this look OK now?

[spatel]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

8721–8725 ↗

(On Diff #93186)

If the target has FMAD, then did we just bail out unnecessarily?

[arsenm]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

8721–8725 ↗

(On Diff #93186)

Yes, the point of fmad is it is safe anyway

test/CodeGen/AArch64/neon-fma-FMF.ll

13 ↗

(On Diff #93186)

Positive checks for the separate add and mul would be less fragile

[spatel]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

8721–8725 ↗

(On Diff #93186)

Yikes...this means there are no regression tests for any of the folds in this function for an FMAD-capable target?

[anemet]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

8721–8725 ↗

(On Diff #93186)

Good point. Let me do some digging.

[anemet]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

8721–8725 ↗

(On Diff #93186)

Looks like there is: CodeGen/R600/mad-combine.ll. I just didn't help that target enabled :(. Thanks for catching this!

[anemet]

Address review comments. Also test with all target enabled now. Sorry again
for the silly mistake.

[spatel]

See inline for one more possible test case. I have no other comments, but I'm cc'ing some people who have worked on this function recently for potential feedback.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

8753–8756 ↗

(On Diff #93426)

I think this diff exposes a possibility that didn't exist before, so it's worth adding a test for it:

define float @cannot_choose_fma(float %f1, float %f2, float %f3, float %f4) {
  %mul1 = fmul contract float %f1, %f2
  %mul2 = fmul float %f3, %f4
  %add = fadd contract float %mul1, %mul2
  %second_use_of_mul1 = fdiv float %mul1, %add
  ret float %second_use_of_mul1
}

$ ./llc -o - fma.ll -mtriple=powerpc64-darwin 
  fmuls f0, f3, f4
  fmuls f13, f1, f2
  fmadds f0, f1, f2, f0
  fdivs f1, f13, f0

[anemet]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

8753–8756 ↗

(On Diff #93426)

Makes sense, I've added it. Thanks for all your feedback and sorry again about the testing mishap. I haven't worked on the backend for a while.

[anemet]

Add a new test requested by Sanjay.

[spatel]

LGTM - see a couple of small nits for the PPC test comment.

Also, you can use the script at utils/update_llc_test_checks.py to auto-generate exact checks for PPC (avoid CHECK-NOT whenever possible). Bonus points if you add aarch64 to the supported triples in that script and improve those checks too. :)

test/CodeGen/PowerPC/fma-aggr-FMF.ll

15 ↗

(On Diff #93495)

"no use" -> "one use"

16 ↗

(On Diff #93495)

"a use" -> "two uses"

[anemet]

In D31169#714312, @spatel wrote:

LGTM - see a couple of small nits for the PPC test comment.

Thanks!

Also, you can use the script at utils/update_llc_test_checks.py to auto-generate exact checks for PPC (avoid CHECK-NOT whenever possible).

Done. Please take a look at the committed version. The powerpc64le assembly looked correct to me but I don't work on PPC.

test/CodeGen/PowerPC/fma-aggr-FMF.ll

15 ↗

(On Diff #93495)

I added the word "extra" use which is what I meant to write :)

[spatel]

In D31169#714398, @anemet wrote:

Done. Please take a look at the committed version. The powerpc64le assembly looked correct to me but I don't work on PPC.

Seems right. PPC-LE is foreign to me too...much preferred the BE and Darwin syntax. :)

[anemet]

The counterpart of fused multiply-and-sub was committed under rL299572.

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

for (const SDValue &Op : BV->op_values()) {
     // For big endian targets, swap the order of the pieces of each element.
     if (DAG.getDataLayout().isBigEndian())
       std::reverse(Ops.end()-NumOutputsPerInput, Ops.end());
   }
 
   return DAG.getBuildVector(VT, DL, Ops);
 }
 
+static bool isContractable(SDNode *N) {
+  SDNodeFlags F = cast<BinaryWithFlagsSDNode>(N)->Flags;
+  return F.hasAllowContract() || F.hasUnsafeAlgebra();
+}
+
 /// Try to perform FMA combining on a given FADD node.
 SDValue DAGCombiner::visitFADDForFMACombine(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   EVT VT = N->getValueType(0);
   SDLoc SL(N);
 
   const TargetOptions &Options = DAG.getTarget().Options;
-  bool AllowFusion =
-      (Options.AllowFPOpFusion == FPOpFusion::Fast || Options.UnsafeFPMath);
 
   // Floating-point multiply-add with intermediate rounding.
   bool HasFMAD = (LegalOperations && TLI.isOperationLegal(ISD::FMAD, VT));
 
   // Floating-point multiply-add without intermediate rounding.
   bool HasFMA =
-      AllowFusion && TLI.isFMAFasterThanFMulAndFAdd(VT) &&
+      TLI.isFMAFasterThanFMulAndFAdd(VT) &&
       (!LegalOperations || TLI.isOperationLegalOrCustom(ISD::FMA, VT));
 
   // No valid opcode, do not combine.
   if (!HasFMAD && !HasFMA)
     return SDValue();
 
+  bool AllowFusionGlobally = (Options.AllowFPOpFusion == FPOpFusion::Fast ||
+                              Options.UnsafeFPMath || HasFMAD);
+  // If the addition is not contractable, do not combine.
+  if (!AllowFusionGlobally && !isContractable(N))
+    return SDValue();
+
   const SelectionDAGTargetInfo *STI = DAG.getSubtarget().getSelectionDAGInfo();
-  ;
-  if (AllowFusion && STI && STI->generateFMAsInMachineCombiner(OptLevel))
+  if (STI && STI->generateFMAsInMachineCombiner(OptLevel))
     return SDValue();
 
   // Always prefer FMAD to FMA for precision.
   unsigned PreferredFusedOpcode = HasFMAD ? ISD::FMAD : ISD::FMA;
   bool Aggressive = TLI.enableAggressiveFMAFusion(VT);
   bool LookThroughFPExt = TLI.isFPExtFree(VT);
 
+  // Is the node an FMUL and contractable either due to global flags or
+  // SDNodeFlags.
+  auto isContractableFMUL = [AllowFusionGlobally](SDValue N) {
+    if (N.getOpcode() != ISD::FMUL)
+      return false;
+    return AllowFusionGlobally || isContractable(N.getNode());
+  };
   // If we have two choices trying to fold (fadd (fmul u, v), (fmul x, y)),
   // prefer to fold the multiply with fewer uses.
-  if (Aggressive && N0.getOpcode() == ISD::FMUL &&
-      N1.getOpcode() == ISD::FMUL) {
+  if (Aggressive && isContractableFMUL(N0) && isContractableFMUL(N1)) {
     if (N0.getNode()->use_size() > N1.getNode()->use_size())
       std::swap(N0, N1);
   }
 
   // fold (fadd (fmul x, y), z) -> (fma x, y, z)
-  if (N0.getOpcode() == ISD::FMUL &&
-      (Aggressive || N0->hasOneUse())) {
+  if (isContractableFMUL(N0) && (Aggressive || N0->hasOneUse())) {
     return DAG.getNode(PreferredFusedOpcode, SL, VT,
                        N0.getOperand(0), N0.getOperand(1), N1);
   }
 
   // fold (fadd x, (fmul y, z)) -> (fma y, z, x)
   // Note: Commutes FADD operands.
-  if (N1.getOpcode() == ISD::FMUL &&
-      (Aggressive || N1->hasOneUse())) {
+  if (isContractableFMUL(N1) && (Aggressive || N1->hasOneUse())) {
     return DAG.getNode(PreferredFusedOpcode, SL, VT,
                        N1.getOperand(0), N1.getOperand(1), N0);
   }
 
   // Look through FP_EXTEND nodes to do more combining.
-  if (AllowFusion && LookThroughFPExt) {
+  if (LookThroughFPExt) {
     // fold (fadd (fpext (fmul x, y)), z) -> (fma (fpext x), (fpext y), z)
     if (N0.getOpcode() == ISD::FP_EXTEND) {
       SDValue N00 = N0.getOperand(0);
-      if (N00.getOpcode() == ISD::FMUL)
+      if (isContractableFMUL(N00))
         return DAG.getNode(PreferredFusedOpcode, SL, VT,
                            DAG.getNode(ISD::FP_EXTEND, SL, VT,
                                        N00.getOperand(0)),
                            DAG.getNode(ISD::FP_EXTEND, SL, VT,
                                        N00.getOperand(1)), N1);
     }
 
     // fold (fadd x, (fpext (fmul y, z))) -> (fma (fpext y), (fpext z), x)
     // Note: Commutes FADD operands.
     if (N1.getOpcode() == ISD::FP_EXTEND) {
       SDValue N10 = N1.getOperand(0);
-      if (N10.getOpcode() == ISD::FMUL)
+      if (isContractableFMUL(N10))
         return DAG.getNode(PreferredFusedOpcode, SL, VT,
                            DAG.getNode(ISD::FP_EXTEND, SL, VT,
                                        N10.getOperand(0)),
                            DAG.getNode(ISD::FP_EXTEND, SL, VT,
                                        N10.getOperand(1)), N0);
     }
   }
 
if (Options.UnsafeFPMath &&
       return DAG.getNode(PreferredFusedOpcode, SL, VT,
                          N1.getOperand(0), N1.getOperand(1),
                          DAG.getNode(PreferredFusedOpcode, SL, VT,
                                      N1.getOperand(2).getOperand(0),
                                      N1.getOperand(2).getOperand(1),
                                      N0));
     }
 
-    if (AllowFusion && LookThroughFPExt) {
+    if (/*AllowFusion &&*/ LookThroughFPExt) {
       // fold (fadd (fma x, y, (fpext (fmul u, v))), z)
       //   -> (fma x, y, (fma (fpext u), (fpext v), z))
       auto FoldFAddFMAFPExtFMul = [&] (
           SDValue X, SDValue Y, SDValue U, SDValue V, SDValue Z) {
         return DAG.getNode(PreferredFusedOpcode, SL, VT, X, Y,
                            DAG.getNode(PreferredFusedOpcode, SL, VT,
                                        DAG.getNode(ISD::FP_EXTEND, SL, VT, U),
                                        DAG.getNode(ISD::FP_EXTEND, SL, VT, V),
                                        Z));
       };
       if (N0.getOpcode() == PreferredFusedOpcode) {
         SDValue N02 = N0.getOperand(2);
         if (N02.getOpcode() == ISD::FP_EXTEND) {
           SDValue N020 = N02.getOperand(0);
-          if (N020.getOpcode() == ISD::FMUL)
+          if (isContractableFMUL(N020))
             return FoldFAddFMAFPExtFMul(N0.getOperand(0), N0.getOperand(1),
                                         N020.getOperand(0), N020.getOperand(1),
                                         N1);
         }
       }
 
       // fold (fadd (fpext (fma x, y, (fmul u, v))), z)
       //   -> (fma (fpext x), (fpext y), (fma (fpext u), (fpext v), z))
if (/*AllowFusion &&*/ LookThroughFPExt) {
                                        DAG.getNode(ISD::FP_EXTEND, SL, VT, U),
                                        DAG.getNode(ISD::FP_EXTEND, SL, VT, V),
                                        Z));
       };
       if (N0.getOpcode() == ISD::FP_EXTEND) {
         SDValue N00 = N0.getOperand(0);
         if (N00.getOpcode() == PreferredFusedOpcode) {
           SDValue N002 = N00.getOperand(2);
-          if (N002.getOpcode() == ISD::FMUL)
+          if (isContractableFMUL(N002))
             return FoldFAddFPExtFMAFMul(N00.getOperand(0), N00.getOperand(1),
                                         N002.getOperand(0), N002.getOperand(1),
                                         N1);
         }
       }
 
       // fold (fadd x, (fma y, z, (fpext (fmul u, v)))
       //   -> (fma y, z, (fma (fpext u), (fpext v), x))
       if (N1.getOpcode() == PreferredFusedOpcode) {
         SDValue N12 = N1.getOperand(2);
         if (N12.getOpcode() == ISD::FP_EXTEND) {
           SDValue N120 = N12.getOperand(0);
-          if (N120.getOpcode() == ISD::FMUL)
+          if (isContractableFMUL(N120))
             return FoldFAddFMAFPExtFMul(N1.getOperand(0), N1.getOperand(1),
                                         N120.getOperand(0), N120.getOperand(1),
                                         N0);
         }
       }
 
       // fold (fadd x, (fpext (fma y, z, (fmul u, v)))
       //   -> (fma (fpext y), (fpext z), (fma (fpext u), (fpext v), x))
       // FIXME: This turns two single-precision and one double-precision
       // operation into two double-precision operations, which might not be
       // interesting for all targets, especially GPUs.
       if (N1.getOpcode() == ISD::FP_EXTEND) {
         SDValue N10 = N1.getOperand(0);
         if (N10.getOpcode() == PreferredFusedOpcode) {
           SDValue N102 = N10.getOperand(2);
-          if (N102.getOpcode() == ISD::FMUL)
+          if (isContractableFMUL(N102))
             return FoldFAddFPExtFMAFMul(N10.getOperand(0), N10.getOperand(1),
                                         N102.getOperand(0), N102.getOperand(1),
                                         N0);
         }
       }
     }
   }
 

llvm/trunk/test/CodeGen/AArch64/neon-fma-FMF.ll

+; RUN: llc < %s -verify-machineinstrs -mtriple=aarch64-none-linux-gnu -mattr=+neon | FileCheck %s
+
+define <2 x float> @fma(<2 x float> %A, <2 x float> %B, <2 x float> %C) {
+; CHECK-LABEL: fma:
+; CHECK: fmla {{v[0-9]+}}.2s, {{v[0-9]+}}.2s, {{v[0-9]+}}.2s
+  %tmp1 = fmul contract <2 x float> %A, %B;
+  %tmp2 = fadd contract <2 x float> %C, %tmp1;
+  ret <2 x float> %tmp2
+}
+
+define <2 x float> @no_fma_1(<2 x float> %A, <2 x float> %B, <2 x float> %C) {
+; CHECK-LABEL: no_fma_1:
+; CHECK: fmul
+; CHECK: fadd
+  %tmp1 = fmul contract <2 x float> %A, %B;
+  %tmp2 = fadd <2 x float> %C, %tmp1;
+  ret <2 x float> %tmp2
+}
+
+define <2 x float> @no_fma_2(<2 x float> %A, <2 x float> %B, <2 x float> %C) {
+; CHECK-LABEL: no_fma_2:
+; CHECK: fmul
+; CHECK: fadd
+  %tmp1 = fmul <2 x float> %A, %B;
+  %tmp2 = fadd contract <2 x float> %C, %tmp1;
+  ret <2 x float> %tmp2
+}

llvm/trunk/test/CodeGen/PowerPC/fma-aggr-FMF.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -verify-machineinstrs -mtriple=powerpc64le-linux-gnu | FileCheck %s
+
+define float @can_fma_with_fewer_uses(float %f1, float %f2, float %f3, float %f4) {
+; CHECK-LABEL: can_fma_with_fewer_uses:
+; CHECK:       # BB#0:
+; CHECK-NEXT:    xsmulsp 0, 1, 2
+; CHECK-NEXT:    fmr 1, 0
+; CHECK-NEXT:    xsmaddasp 1, 3, 4
+; CHECK-NEXT:    xsdivsp 1, 0, 1
+; CHECK-NEXT:    blr
+  %mul1 = fmul contract float %f1, %f2
+  %mul2 = fmul contract float %f3, %f4
+  %add = fadd contract float %mul1, %mul2
+  %second_use_of_mul1 = fdiv float %mul1, %add
+  ret float %second_use_of_mul1
+}
+
+; There is no contract on the mul with no extra use so we can't fuse that.
+; Since we are fusing with the mul with an extra use, the fmul needs to stick
+; around beside the fma.
+define float @no_fma_with_fewer_uses(float %f1, float %f2, float %f3, float %f4) {
+; CHECK-LABEL: no_fma_with_fewer_uses:
+; CHECK:       # BB#0:
+; CHECK-NEXT:    xsmulsp 0, 3, 4
+; CHECK-NEXT:    xsmulsp 13, 1, 2
+; CHECK-NEXT:    xsmaddasp 0, 1, 2
+; CHECK-NEXT:    xsdivsp 1, 13, 0
+; CHECK-NEXT:    blr
+  %mul1 = fmul contract float %f1, %f2
+  %mul2 = fmul float %f3, %f4
+  %add = fadd contract float %mul1, %mul2
+  %second_use_of_mul1 = fdiv float %mul1, %add
+  ret float %second_use_of_mul1
+}