Use the 'arcp' fast-math-flag when combining repeated FP divisors

[spatel]

Summary

This is a usage of the IR-level fast-math-flags now that they are propagated to SDNodes (r237046). This was originally part of D8900.

I'm not sure if we need to maintain the global TargetOptions.UnsafeFPMath checks or if we're now free to start deleting those and rely exclusively on the node-level optimization flags. If the latter, I need to update the corresponding fdiv testcases for PowerPC and Aarch64 to use 'arcp'.

[ahatanak]

To preserve backward compatibility, I think we should make sure functions using fast math attributes (e.g., "unsafe-fp-math" in test/CodeGen/PowerPC/fdiv-combine.ll) still get optimized after we migrate to modeling those flags at the node-level (using autoupgrade?).

[spatel]

In D9708#171355, @ahatanak wrote:

To preserve backward compatibility, I think we should make sure functions using fast math attributes (e.g., "unsafe-fp-math" in test/CodeGen/PowerPC/fdiv-combine.ll) still get optimized after we migrate to modeling those flags at the node-level (using autoupgrade?).

Thanks, Akira. If I understand correctly, I should create this autoupgrade patch first then. That would allow us to remove the Options.UnsafeFPMath checks as we update all of the places in the backend to use the node-level flags.

Is a separate mechanism needed to handle the llc command-line flag "-enable-unsafe-fp-math"?

[spatel]

Sorry for the (5 month!) delay...

After several false starts, FMF on DAG nodes appears to be here for good. This will still be the first use of them to enable an optimization from what I can tell.

Patch rebased.

[hfinkel]

This will still be the first use of them to enable an optimization from what I can tell.

:-)

LGTM.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

if (char RHSNeg = isNegatibleForFree(N1, LegalOperations, TLI, &Options)) {
       // negated.
       if (LHSNeg == 2 || RHSNeg == 2)
         return DAG.getNode(ISD::FDIV, SDLoc(N), VT,
                            GetNegatedExpression(N0, DAG, LegalOperations),
                            GetNegatedExpression(N1, DAG, LegalOperations));
     }
   }
 
+  // For reciprocal optimization of a division node, check the division
+  // node's optimization flags as well as the function-wide fast-math setting.
+  bool AllowRecip = false;
+  if (const auto *NodeWithFlags = dyn_cast<BinaryWithFlagsSDNode>(N))
+    AllowRecip = NodeWithFlags->Flags.hasAllowReciprocal();
+  
   // Combine multiple FDIVs with the same divisor into multiple FMULs by the
   // reciprocal.
   // E.g., (a / D; b / D;) -> (recip = 1.0 / D; a * recip; b * recip)
   // Notice that this is not always beneficial. One reason is different target
   // may have different costs for FDIV and FMUL, so sometimes the cost of two
   // FDIVs may be lower than the cost of one FDIV and two FMULs. Another reason
   // is the critical path is increased from "one FDIV" to "one FDIV + one FMUL".
-  if (Options.UnsafeFPMath) {
+  if (Options.UnsafeFPMath || AllowRecip) {
     // Skip if current node is a reciprocal.
     if (N0CFP && N0CFP->isExactlyValue(1.0))
       return SDValue();
 
     SmallVector<SDNode *, 4> Users;
     // Find all FDIV users of the same divisor.
     for (SDNode::use_iterator UI = N1.getNode()->use_begin(),
                               UE = N1.getNode()->use_end();
          UI != UE; ++UI) {
       SDNode *User = UI.getUse().getUser();
       if (User->getOpcode() == ISD::FDIV && User->getOperand(1) == N1)
+        if (const auto *NodeWithFlags = dyn_cast<BinaryWithFlagsSDNode>(User))
+          if (Options.UnsafeFPMath || NodeWithFlags->Flags.hasAllowReciprocal())
             Users.push_back(User);
     }
 
     if (TLI.combineRepeatedFPDivisors(Users.size())) {
       SDLoc DL(N);
       SDValue FPOne = DAG.getConstantFP(1.0, DL, VT); // floating point 1.0
       SDValue Reciprocal = DAG.getNode(ISD::FDIV, DL, VT, FPOne, N1);
 
       // Dividend / Divisor -> Dividend * Reciprocal

test/CodeGen/X86/fdiv-combine.ll

 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown | FileCheck %s
 
-; Anything more than one division using a single divisor operand
+; More than one 'arcp' division using a single divisor operand
 ; should be converted into a reciprocal and multiplication.
 
-define float @div1_arcp(float %x, float %y, float %z) #0 {
+; Don't do anything for just one division.
+;
+define float @div1_arcp(float %x, float %y, float %z) {
 ; CHECK-LABEL: div1_arcp:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    divss %xmm1, %xmm0
 ; CHECK-NEXT:    retq
   %div1 = fdiv arcp float %x, %y
   ret float %div1
 }
 
-define float @div2_arcp(float %x, float %y, float %z) #0 {
-; CHECK-LABEL: div2_arcp:
+; All math instructions are 'arcp', so optimize.
+;
+define float @div2_arcp_all(float %x, float %y, float %z) {
+; CHECK-LABEL: div2_arcp_all:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    movss {{.*#+}} xmm3 = mem[0],zero,zero,zero
 ; CHECK-NEXT:    divss %xmm2, %xmm3
 ; CHECK-NEXT:    mulss %xmm3, %xmm0
 ; CHECK-NEXT:    mulss %xmm1, %xmm0
 ; CHECK-NEXT:    mulss %xmm3, %xmm0
 ; CHECK-NEXT:    retq
   %div1 = fdiv arcp float %x, %z
   %mul = fmul arcp float %div1, %y
   %div2 = fdiv arcp float %mul, %z
   ret float %div2
 }
 
-; FIXME: If the backend understands 'arcp', then this attribute is unnecessary.
-attributes #0 = { "unsafe-fp-math"="true" }
+; The first division is not 'arcp', so do not optimize.
+;
+define float @div2_arcp_partial1(float %x, float %y, float %z) {
+; CHECK-LABEL: div2_arcp_partial1:
+; CHECK:       # BB#0:
+; CHECK-NEXT:    divss %xmm2, %xmm0
+; CHECK-NEXT:    mulss %xmm1, %xmm0
+; CHECK-NEXT:    divss %xmm2, %xmm0
+; CHECK-NEXT:    retq
+  %div1 = fdiv float %x, %z
+  %mul = fmul arcp float %div1, %y
+  %div2 = fdiv arcp float %mul, %z
+  ret float %div2
+}
+
+; The second division is not 'arcp', so do not optimize.
+;
+define float @div2_arcp_partial2(float %x, float %y, float %z) {
+; CHECK-LABEL: div2_arcp_partial2:
+; CHECK:       # BB#0:
+; CHECK-NEXT:    divss %xmm2, %xmm0
+; CHECK-NEXT:    mulss %xmm1, %xmm0
+; CHECK-NEXT:    divss %xmm2, %xmm0
+; CHECK-NEXT:    retq
+  %div1 = fdiv arcp float %x, %z
+  %mul = fmul arcp float %div1, %y
+  %div2 = fdiv float %mul, %z
+  ret float %div2
+}
+
+; The multiply is not 'arcp', but that does not prevent optimizing the divisions.
+;
+define float @div2_arcp_partial3(float %x, float %y, float %z) {
+; CHECK-LABEL: div2_arcp_partial3:
+; CHECK:       # BB#0:
+; CHECK-NEXT:    movss {{.*#+}} xmm3 = mem[0],zero,zero,zero
+; CHECK-NEXT:    divss %xmm2, %xmm3
+; CHECK-NEXT:    mulss %xmm3, %xmm0
+; CHECK-NEXT:    mulss %xmm1, %xmm0
+; CHECK-NEXT:    mulss %xmm3, %xmm0
+; CHECK-NEXT:    retq
+  %div1 = fdiv arcp float %x, %z
+  %mul = fmul float %div1, %y
+  %div2 = fdiv arcp float %mul, %z
+  ret float %div2
+}
+