[DAGCombine] Add node level checks for fp-contract and fp-ninf in visitFMULForFMADistributiveCombine().

[RKSimon]

This patch has no description summary at all :( What's the background to this? Is it related to PR51346 / D74436?

[abinavpp]

In D107551#2928034, @RKSimon wrote:

This patch has no description summary at all :( What's the background to this? Is it related to PR51346 / D74436?

I'm sorry about that. I believe @arsenm would know the background to this. If I understood the issue raised by @arsenm correctly, we need DAG-combine APIs to not just check for fp-model specific flags, but to have finer SDNode level checks. The new test in AMDGPU/fma.ll is an example I could think of as a motivation for this.

[arsenm]

In D107551#2928034, @RKSimon wrote:

This patch has no description summary at all :( What's the background to this? Is it related to PR51346 / D74436?

This is migrating to using the instruction granularity fast math flags instead of the function level fast math attributes, it's unrelated to PR51346

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

13031–13033

I swear we had a utility for this already somewhere

llvm/test/CodeGen/AMDGPU/fma.ll

149

Probably should add check lines for the other targets, and add a comment describing the test

[abinavpp]

Rebased; Addressed review comments.

[abinavpp]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

13031–13033

I couldn't find it :(

[spatel]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

13073–13077

I think this is the existing code that we're remembering. This seems to be pulling in one more clause for FMA/FMAD - probably want to do that too for this patch?

llvm/test/CodeGen/X86/fma-scalar-combine.ll

567

Not sure if you manually created these check lines or removed the CHECK for a 'ret' line, but it's better to leave that in for easier updating. If you can pre-commit this test with the current (baseline) CHECKs, that would be even better.

[abinavpp]

Rebased; Addressed review comments.

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

13073–13077

Oh, I thought we were talking about hasNoInfs(), the isContractibleFMUL() is copied from this lambda-expression. I removed the dependence on HasFMAD since it's checking if a target supports FMAD. HasFMA does the same for FMA. I feel isContractibleFMUL() should strictly check for FP-environment and not worry about target specific details.

[abinavpp]

Rebased.

[abinavpp]

I don't have a commit access. @arsenm, can you commit this on my behalf? Here are my details:
user.name=Abinav Puthan Purayil
user.email=[e-mail]

llvm/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);
 }
 
+// Returns true if floating point contraction is allowed on the FMUL-SDValue
+// `N`
+static bool isContractableFMUL(const TargetOptions &Options, SDValue N) {
+  assert(N.getOpcode() == ISD::FMUL);
+
+  return Options.AllowFPOpFusion == FPOpFusion::Fast || Options.UnsafeFPMath ||
+         N->getFlags().hasAllowContract();
+}
+
+// Return true if `N` can assume no infinities involved in it's computation.
+static bool hasNoInfs(const TargetOptions &Options, SDValue N) {
+  return Options.NoInfsFPMath || N.getNode()->getFlags().hasNoInfs();
+}
+
 /// 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;
if (TLI.generateFMAsInMachineCombiner(VT, OptLevel))
     return SDValue();
 
   // Always prefer FMAD to FMA for precision.
   unsigned PreferredFusedOpcode = HasFMAD ? ISD::FMAD : ISD::FMA;
   bool Aggressive = TLI.enableAggressiveFMAFusion(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 || N->getFlags().hasAllowContract();
   };
   // 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 && 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)
SDValue DAGCombiner::visitFMULForFMADistributiveCombine(SDNode *N) {
   SDLoc SL(N);
 
   assert(N->getOpcode() == ISD::FMUL && "Expected FMUL Operation");
 
   const TargetOptions &Options = DAG.getTarget().Options;
 
   // The transforms below are incorrect when x == 0 and y == inf, because the
   // intermediate multiplication produces a nan.
-  if (!Options.NoInfsFPMath)
+  SDValue FAdd = N0.getOpcode() == ISD::FADD ? N0 : N1;
+  if (!hasNoInfs(Options, FAdd))
     return SDValue();
 
   // Floating-point multiply-add without intermediate rounding.
   bool HasFMA =
-      (Options.AllowFPOpFusion == FPOpFusion::Fast || Options.UnsafeFPMath) &&
+      isContractableFMUL(Options, SDValue(N, 0)) &&
       TLI.isFMAFasterThanFMulAndFAdd(DAG.getMachineFunction(), VT) &&
       (!LegalOperations || TLI.isOperationLegalOrCustom(ISD::FMA, VT));
 
   // Floating-point multiply-add with intermediate rounding. This can result
   // in a less precise result due to the changed rounding order.
   bool HasFMAD = Options.UnsafeFPMath &&
                  (LegalOperations && TLI.isFMADLegal(DAG, N));
 

llvm/test/CodeGen/AMDGPU/fma.ll

bb:
   %tmp6 = fadd contract float %tmp1, %tmp1
   %tmp7 = fmul contract float %tmp6, %tmp
   %tmp8 = fsub contract float 1.0, %tmp7
   %tmp9 = fmul contract float %tmp8, 8.0
   %tmp10 = fadd contract float %tmp5, %tmp9
   store float %tmp10, float addrspace(1)* %gep.out
   ret void
 }
+
+; Fold (fmul (fadd x, 1.0), y) -> (fma x, y, y) without FP specific command-line
+; options.
+; FUNC-LABEL: {{^}}fold_fmul_distributive:
+; GFX906: v_fmac_f32_e32 v0, v1, v0
+define float @fold_fmul_distributive(float %x, float %y) {
+  %fadd = fadd ninf float %y, 1.0
+  %fmul = fmul contract float %fadd, %x
+  ret float %fmul
+}

llvm/test/CodeGen/X86/fma-scalar-combine.ll

 ; CHECK-NEXT:    # fixup A - offset: 5, value: {{\.?LCPI[0-9]+_[0-9]+}}-4, kind: reloc_riprel_4byte
 ; CHECK-NEXT:    # xmm0 = (xmm0 * mem) + xmm1
 ; CHECK-NEXT:    retq # encoding: [0xc3]
   %mul1 = fmul contract float %x, 10.0
   %mul2 = fmul contract float %x, 11.0
   %add1 = fadd contract float %mul1, %mul2
   ret float %add1
 }
+
+; Fold (fmul (fadd x, 1.0), y) -> (fma x, y, y) without FP specific command-line
+; options.
+define float @combine_fmul_distributive(float %x, float %y) {
+; CHECK-LABEL: combine_fmul_distributive:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    vfmadd231ss %xmm0, %xmm1, %xmm0 # EVEX TO VEX Compression encoding: [0xc4,0xe2,0x71,0xb9,0xc0]
+; CHECK-NEXT:    # xmm0 = (xmm1 * xmm0) + xmm0
+; CHECK-NEXT:    retq # encoding: [0xc3]
+  %fadd = fadd ninf float %y, 1.0
+  %fmul = fmul contract float %fadd, %x
+  ret float %fmul
+}