Utilize new SDNode flag functionality to expand current support for fdiv

[mcberg2017]

Summary

This patch originated from D46562 and is a proper subset, with some issues addressed.

[mcberg2017]

Note, I will unify the multiple/divide context as as before if needed once one or the other is checked in.

[spatel]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

732–734

See comments in D47911 - we're adding FMF constraints where there were none before?

test/CodeGen/AMDGPU/fdiv.f16.ll

221

I don't know enough about AMDGPU to understand what this diff means.

I'd prefer that we add a test for some target where this kind of test currently produces fdiv asm, and this patch will allow it to become an fmul.

[mcberg2017]

Be advised that I am splitting the remaining fmul, fdiv and fadd reviews out for negation context, it will have its own review shortly. The old reviews will remain but only under visit wrt context.

[mcberg2017]

Removed negation context...

test/CodeGen/AMDGPU/fdiv.f16.ll

221

Working on it, should have a new test in bit...

[arsenm]

test/CodeGen/AMDGPU/fdiv.f16.ll

221

Changing the value of the constant here is pretty suspicious

[mcberg2017]

test/CodeGen/AMDGPU/fdiv.f16.ll

221

So I walked through the APFloat interface, it does return the correct 16bit value, 0x2E66, which is then converted to F32, and is 0x3dccc000. The old value bypassed conversion in visitFDIV and we given the decimal value of 1/10 which is in 32bit is 0x3dcccccd. The new behavior is more correct than what used to happen since we are being asked to rcp divide approximate a 16bit value.

[mcberg2017]

I will see about adding a fdiv const test to the fmf flag tests as well

test/CodeGen/AMDGPU/fdiv.f16.ll

221

Matt and I have discussed this offline noting that the new behavior is correct wrt to the changed constants in the this test.

[mcberg2017]

Added test for fmf path.

[mcberg2017]

Tighter binding of the folded constant to the multiply produced in the check patterns. Please note the long constant 1036828672 is the hex value 0x3DCCC000 and is represented as float 0.0999755859, this shows the same propagation as the AMDGPU test, first folding the constant 1/10 to F16 and then converting it to F32.

[spatel]

LGTM.

test/CodeGen/X86/fmf-flags.ll

12–16

This is an interesting difference. Before this patch, we wouldn't recognize that we could use an estimate from the fdiv node, but we did recognize that we could use an estimate on the square root itself. But at that point, we don't realize that we're actually calculating a reciprocal square root (as opposed to a plain square root estimate)...so we generate the safety check for a 0.0 input.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

static char isNegatibleForFree(SDValue Op, bool LegalOperations,
                                const TargetLowering &TLI,
                                const TargetOptions *Options,
                                unsigned Depth = 0) {
   // fneg is removable even if it has multiple uses.
   if (Op.getOpcode() == ISD::FNEG) return 2;
 
   // Don't allow anything with multiple uses unless we know it is free.
   EVT VT = Op.getValueType();
+  const SDNodeFlags Flags = Op->getFlags();
   if (!Op.hasOneUse())
     if (!(Op.getOpcode() == ISD::FP_EXTEND &&
           TLI.isFPExtFree(VT, Op.getOperand(0).getValueType())))
       return 0;
 
   // Don't recurse exponentially.
   if (Depth > 6) return 0;
 
case ISD::FSUB:
     if (!Options->NoSignedZerosFPMath &&
         !Op.getNode()->getFlags().hasNoSignedZeros())
       return 0;
 
     // fold (fneg (fsub A, B)) -> (fsub B, A)
     return 1;
 
   case ISD::FMUL:
-  case ISD::FDIV:
     if (Options->HonorSignDependentRoundingFPMath()) return 0;
 
     // fold (fneg (fmul X, Y)) -> (fmul (fneg X), Y) or (fmul X, (fneg Y))
     if (char V = isNegatibleForFree(Op.getOperand(0), LegalOperations, TLI,
                                     Options, Depth + 1))
       return V;
 
     return isNegatibleForFree(Op.getOperand(1), LegalOperations, TLI, Options,
                               Depth + 1);
                 
+  case ISD::FDIV:
+    if (Options->HonorSignDependentRoundingFPMathOption && 
+        !Options->UnsafeFPMath && !Flags.hasNoNaNs()) return 0;
+
+    // fold (fneg (fmul X, Y)) -> (fmul (fneg X), Y) or (fmul X, (fneg Y))
+    if (char V = isNegatibleForFree(Op.getOperand(0), LegalOperations, TLI,
+                                    Options, Depth + 1))
+      return V;
+
+    return isNegatibleForFree(Op.getOperand(1), LegalOperations, TLI, Options,
+                              Depth + 1);
+
   case ISD::FP_EXTEND:
   case ISD::FP_ROUND:
   case ISD::FSIN:
     return isNegatibleForFree(Op.getOperand(0), LegalOperations, TLI, Options,
                               Depth + 1);
   }
 }
 
if (ConstantFPSDNode *N0CFP = dyn_cast<ConstantFPSDNode>(Op.getOperand(0)))
         return Op.getOperand(1);
 
     // fold (fneg (fsub A, B)) -> (fsub B, A)
     return DAG.getNode(ISD::FSUB, SDLoc(Op), Op.getValueType(),
                        Op.getOperand(1), Op.getOperand(0), Flags);
 
   case ISD::FMUL:
   case ISD::FDIV:
-    assert(!Options.HonorSignDependentRoundingFPMath());
+    assert(!Options.HonorSignDependentRoundingFPMath() || Flags.hasNoNaNs());
 
     // fold (fneg (fmul X, Y)) -> (fmul (fneg X), Y)
     if (isNegatibleForFree(Op.getOperand(0), LegalOperations,
                            DAG.getTargetLoweringInfo(), &Options, Depth+1))
       return DAG.getNode(Op.getOpcode(), SDLoc(Op), Op.getValueType(),
                          GetNegatedExpression(Op.getOperand(0), DAG,
                                               LegalOperations, Depth+1),
                          Op.getOperand(1), Flags);
SDValue DAGCombiner::visitFDIV(SDNode *N) {
 
   // fold (fdiv c1, c2) -> c1/c2
   if (N0CFP && N1CFP)
     return DAG.getNode(ISD::FDIV, SDLoc(N), VT, N0, N1, Flags);
 
   if (SDValue NewSel = foldBinOpIntoSelect(N))
     return NewSel;
 
-  if (Options.UnsafeFPMath) {
+  if (Options.UnsafeFPMath || Flags.hasAllowReciprocal()) {
     // fold (fdiv X, c2) -> fmul X, 1/c2 if losing precision is acceptable.
     if (N1CFP) {
       // Compute the reciprocal 1.0 / c2.
       const APFloat &N1APF = N1CFP->getValueAPF();
       APFloat Recip(N1APF.getSemantics(), 1); // 1.0
       APFloat::opStatus st = Recip.divide(N1APF, APFloat::rmNearestTiesToEven);
       // Only do the transform if the reciprocal is a legal fp immediate that
       // isn't too nasty (eg NaN, denormal, ...).

test/CodeGen/AMDGPU/fdiv.f16.ll

 define amdgpu_kernel void @div_arcp_2_x_pat_f16(half addrspace(1)* %out) #0 {
   %x = load half, half addrspace(1)* undef
   %rcp = fdiv arcp half %x, 2.0
   store half %rcp, half addrspace(1)* %out, align 4
   ret void
 }
 
 ; FUNC-LABEL: {{^}}div_arcp_k_x_pat_f16:
-; SI: v_mul_f32_e32 v{{[0-9]+}}, 0x3dcccccd, v{{[0-9]+}}
+; SI: v_mul_f32_e32 v{{[0-9]+}}, 0x3dccc000, v{{[0-9]+}}
 
 ; GFX8_9: v_mul_f16_e32 [[MUL:v[0-9]+]], 0x2e66, v{{[0-9]+}}
 ; GFX8_9: buffer_store_short [[MUL]]
 define amdgpu_kernel void @div_arcp_k_x_pat_f16(half addrspace(1)* %out) #0 {
   %x = load half, half addrspace(1)* undef
   %rcp = fdiv arcp half %x, 10.0
   store half %rcp, half addrspace(1)* %out, align 4
   ret void
 }
 
 ; FUNC-LABEL: {{^}}div_arcp_neg_k_x_pat_f16:
-; SI: v_mul_f32_e32 v{{[0-9]+}}, 0xbdcccccd, v{{[0-9]+}}
+; SI: v_mul_f32_e32 v{{[0-9]+}}, 0xbdccc000, v{{[0-9]+}}
 
 ; GFX8_9: v_mul_f16_e32 [[MUL:v[0-9]+]], 0xae66, v{{[0-9]+}}
 ; GFX8_9: buffer_store_short [[MUL]]
 define amdgpu_kernel void @div_arcp_neg_k_x_pat_f16(half addrspace(1)* %out) #0 {
   %x = load half, half addrspace(1)* undef
   %rcp = fdiv arcp half %x, -10.0
   store half %rcp, half addrspace(1)* %out, align 4
   ret void

test/CodeGen/X86/fmf-flags.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-unknown | FileCheck %s -check-prefix=X64
 ; RUN: llc < %s -mtriple=i686-unknown   | FileCheck %s -check-prefix=X86
 
 declare float @llvm.sqrt.f32(float %x);
 
 define float @fast_recip_sqrt(float %x) {
 ; X64-LABEL: fast_recip_sqrt:
 ; X64:       # %bb.0:
 ; X64-NEXT:    rsqrtss %xmm0, %xmm1
-; X64-NEXT:    xorps   %xmm2, %xmm2
-; X64-NEXT:    cmpeqss %xmm0, %xmm2
 ; X64-NEXT:    mulss   %xmm1, %xmm0
-; X64-NEXT:    movss   {{.*}}(%rip), %xmm3
-; X64-NEXT:    mulss   %xmm0, %xmm3
 ; X64-NEXT:    mulss   %xmm1, %xmm0
 ; X64-NEXT:    addss   {{.*}}(%rip), %xmm0
-; X64-NEXT:    mulss   %xmm3, %xmm0
-; X64-NEXT:    andnps  %xmm0, %xmm2
-; X64-NEXT:    movss   {{.*}}(%rip), %xmm0
-; X64-NEXT:    divss   %xmm2, %xmm0
+; X64-NEXT:    mulss   {{.*}}(%rip), %xmm1
+; X64-NEXT:    mulss   %xmm1, %xmm0
 ; X64-NEXT:    retq
 ;
 ; X86-LABEL: fast_recip_sqrt:
 ; X86:       # %bb.0:
 ; X86-NEXT:    flds {{[0-9]+}}(%esp)
 ; X86-NEXT:    fsqrt
 ; X86-NEXT:    fld1
 ; X86-NEXT:    fdivp %st(1)
 ; X86-NEXT:    retl

 
 ; The sqrt is strict.
 
 @sqrt1 = common global float 0.000000e+00, align 4
 
 define float @not_so_fast_recip_sqrt(float %x) {
 ; X64-LABEL: not_so_fast_recip_sqrt:
 ; X64:       # %bb.0:
-; X64-NEXT:    sqrtss %xmm0, %xmm1
-; X64-NEXT:    movss {{.*#+}} xmm0 = mem[0],zero,zero,zero
-; X64-NEXT:    divss %xmm1, %xmm0
-; X64-NEXT:    movss %xmm1, {{.*}}(%rip)
+; X64-NEXT:    rsqrtss %xmm0, %xmm1
+; X64-NEXT:    sqrtss  %xmm0, %xmm2
+; X64-NEXT:    mulss   %xmm1, %xmm0
+; X64-NEXT:    mulss   %xmm1, %xmm0
+; X64-NEXT:    addss   {{.*}}(%rip), %xmm0
+; X64-NEXT:    mulss   {{.*}}(%rip), %xmm1
+; X64-NEXT:    mulss   %xmm1, %xmm0
+; X64-NEXT:    movss   %xmm2, sqrt1(%rip)
 ; X64-NEXT:    retq
 ;
 ; X86-LABEL: not_so_fast_recip_sqrt:
 ; X86:       # %bb.0:
 ; X86-NEXT:    flds {{[0-9]+}}(%esp)
 ; X86-NEXT:    fsqrt
 ; X86-NEXT:    fld1
 ; X86-NEXT:    fdiv %st(1)