[DAGCombiner] try repeated fdiv divisor transform before building estimate

[spatel]

Summary

This was originally part of D61028, but it's an independent diff.

If we do the repeated divisor reciprocal transform before producing an estimate sequence, then we have an opportunity to use scalar fdiv. On x86, the trade-off is 1 divss vs. 5 vector FP ops in the default estimate sequence. On recent chips (Skylake, Ryzen), the full-precision division is only 3 cycle throughput, so that's probably the better perf default option and avoids problems from x86's inaccurate estimates.

The last 2 tests show that users still have the option to override the defaults by using the function attributes for reciprocal estimates, but we can potentially make those faster by converting vector ops (including ymm ops) to scalar math.

[RKSimon]

LGTM - thanks for splitting this off from D61028

[spatel]

Reopening - I reverted this at rL359695 because it can cause an infinite loop from opposing combines.
There's a proposal to solve that in D61384, so I'll merge that here and try again.

[spatel]

Patch updated:
Add a check for a vector splat of "1.0" when bailing out of the repeated divisor transform. I suspect this bug may be present independently of the original change here (moving the order of the transforms), but I didn't try to create another test case to prove that.

A reduction of the infinite looping test from D61384 was added at rL359709. I don't think we need to add a specific helper to find a splat of "1.0" through a constant pool load as is proposed in D61384 - a simpler use of the existing isConstOrConstSplatFP() avoids the problem.

[nemanjai]

Interestingly enough, I tried this (or some variant of this) and it didn't work for me. Clearly I had done something wrong when trying it. In any case, I've run this against the code we were originally spinning on and it's all good. LGTM and thanks for fixing so quickly.

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

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 (SDValue V = combineRepeatedFPDivisors(N))
+    return V;
+
   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
if (char RHSNeg = isNegatibleForFree(N1, LegalOperations, TLI, &Options,
                            GetNegatedExpression(N0, DAG, LegalOperations,
                                                 ForCodeSize),
                            GetNegatedExpression(N1, DAG, LegalOperations,
                                                 ForCodeSize),
                            Flags);
     }
   }
 
-  if (SDValue CombineRepeatedDivisors = combineRepeatedFPDivisors(N))
-    return CombineRepeatedDivisors;
-
   return SDValue();
 }
 
 SDValue DAGCombiner::visitFREM(SDNode *N) {
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   ConstantFPSDNode *N0CFP = dyn_cast<ConstantFPSDNode>(N0);
   ConstantFPSDNode *N1CFP = dyn_cast<ConstantFPSDNode>(N1);

llvm/trunk/test/CodeGen/X86/fdiv-combine-vec.ll

; AVX-NEXT:    retq
   %splaty = shufflevector <4 x double> %vy, <4 x double> undef, <4 x i32> zeroinitializer
   %r = fdiv arcp <4 x double> %x, %splaty
   ret <4 x double> %r
 }
 
 define <4 x float> @splat_fdiv_v4f32(<4 x float> %x, float %y) {
 ; SSE-LABEL: splat_fdiv_v4f32:
 ; SSE:       # %bb.0:
-; SSE-NEXT:    shufps {{.*#+}} xmm1 = xmm1[0,0,0,0]
-; SSE-NEXT:    rcpps %xmm1, %xmm2
-; SSE-NEXT:    mulps %xmm2, %xmm1
-; SSE-NEXT:    movaps {{.*#+}} xmm3 = [1.0E+0,1.0E+0,1.0E+0,1.0E+0]
-; SSE-NEXT:    subps %xmm1, %xmm3
-; SSE-NEXT:    mulps %xmm2, %xmm3
-; SSE-NEXT:    addps %xmm2, %xmm3
-; SSE-NEXT:    mulps %xmm3, %xmm0
+; SSE-NEXT:    movss {{.*#+}} xmm2 = mem[0],zero,zero,zero
+; SSE-NEXT:    divss %xmm1, %xmm2
+; SSE-NEXT:    shufps {{.*#+}} xmm2 = xmm2[0,0,0,0]
+; SSE-NEXT:    mulps %xmm2, %xmm0
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: splat_fdiv_v4f32:
 ; AVX:       # %bb.0:
+; AVX-NEXT:    vmovss {{.*#+}} xmm2 = mem[0],zero,zero,zero
+; AVX-NEXT:    vdivss %xmm1, %xmm2, %xmm1
 ; AVX-NEXT:    vpermilps {{.*#+}} xmm1 = xmm1[0,0,0,0]
-; AVX-NEXT:    vrcpps %xmm1, %xmm2
-; AVX-NEXT:    vmulps %xmm2, %xmm1, %xmm1
-; AVX-NEXT:    vmovaps {{.*#+}} xmm3 = [1.0E+0,1.0E+0,1.0E+0,1.0E+0]
-; AVX-NEXT:    vsubps %xmm1, %xmm3, %xmm1
-; AVX-NEXT:    vmulps %xmm1, %xmm2, %xmm1
-; AVX-NEXT:    vaddps %xmm1, %xmm2, %xmm1
 ; AVX-NEXT:    vmulps %xmm1, %xmm0, %xmm0
 ; AVX-NEXT:    retq
   %vy = insertelement <4 x float> undef, float %y, i32 0
   %splaty = shufflevector <4 x float> %vy, <4 x float> undef, <4 x i32> zeroinitializer
   %r = fdiv arcp reassoc <4 x float> %x, %splaty
   ret <4 x float> %r
 }
 
 define <8 x float> @splat_fdiv_v8f32(<8 x float> %x, float %y) {
 ; SSE-LABEL: splat_fdiv_v8f32:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movss {{.*#+}} xmm3 = mem[0],zero,zero,zero
 ; SSE-NEXT:    divss %xmm2, %xmm3
 ; SSE-NEXT:    shufps {{.*#+}} xmm3 = xmm3[0,0,0,0]
 ; SSE-NEXT:    mulps %xmm3, %xmm0
 ; SSE-NEXT:    mulps %xmm3, %xmm1
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: splat_fdiv_v8f32:
 ; AVX:       # %bb.0:
+; AVX-NEXT:    vmovss {{.*#+}} xmm2 = mem[0],zero,zero,zero
+; AVX-NEXT:    vdivss %xmm1, %xmm2, %xmm1
 ; AVX-NEXT:    vpermilps {{.*#+}} xmm1 = xmm1[0,0,0,0]
 ; AVX-NEXT:    vinsertf128 $1, %xmm1, %ymm1, %ymm1
-; AVX-NEXT:    vrcpps %ymm1, %ymm2
-; AVX-NEXT:    vmulps %ymm2, %ymm1, %ymm1
-; AVX-NEXT:    vmovaps {{.*#+}} ymm3 = [1.0E+0,1.0E+0,1.0E+0,1.0E+0,1.0E+0,1.0E+0,1.0E+0,1.0E+0]
-; AVX-NEXT:    vsubps %ymm1, %ymm3, %ymm1
-; AVX-NEXT:    vmulps %ymm1, %ymm2, %ymm1
-; AVX-NEXT:    vaddps %ymm1, %ymm2, %ymm1
 ; AVX-NEXT:    vmulps %ymm1, %ymm0, %ymm0
 ; AVX-NEXT:    retq
   %vy = insertelement <8 x float> undef, float %y, i32 0
   %splaty = shufflevector <8 x float> %vy, <8 x float> undef, <8 x i32> zeroinitializer
   %r = fdiv fast <8 x float> %x, %splaty
   ret <8 x float> %r
 }
 
 define <4 x float> @splat_fdiv_v4f32_estimate(<4 x float> %x, float %y) #0 {
 ; SSE-LABEL: splat_fdiv_v4f32_estimate:
 ; SSE:       # %bb.0:
-; SSE-NEXT:    shufps {{.*#+}} xmm1 = xmm1[0,0,0,0]
-; SSE-NEXT:    rcpps %xmm1, %xmm2
-; SSE-NEXT:    mulps %xmm2, %xmm1
-; SSE-NEXT:    movaps {{.*#+}} xmm3 = [1.0E+0,1.0E+0,1.0E+0,1.0E+0]
-; SSE-NEXT:    subps %xmm1, %xmm3
-; SSE-NEXT:    mulps %xmm2, %xmm3
-; SSE-NEXT:    addps %xmm2, %xmm3
+; SSE-NEXT:    rcpss %xmm1, %xmm2
+; SSE-NEXT:    mulss %xmm2, %xmm1
+; SSE-NEXT:    movss {{.*#+}} xmm3 = mem[0],zero,zero,zero
+; SSE-NEXT:    subss %xmm1, %xmm3
+; SSE-NEXT:    mulss %xmm2, %xmm3
+; SSE-NEXT:    addss %xmm2, %xmm3
+; SSE-NEXT:    shufps {{.*#+}} xmm3 = xmm3[0,0,0,0]
 ; SSE-NEXT:    mulps %xmm3, %xmm0
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: splat_fdiv_v4f32_estimate:
 ; AVX:       # %bb.0:
+; AVX-NEXT:    vrcpss %xmm1, %xmm1, %xmm2
+; AVX-NEXT:    vmulss %xmm2, %xmm1, %xmm1
+; AVX-NEXT:    vmovss {{.*#+}} xmm3 = mem[0],zero,zero,zero
+; AVX-NEXT:    vsubss %xmm1, %xmm3, %xmm1
+; AVX-NEXT:    vmulss %xmm1, %xmm2, %xmm1
+; AVX-NEXT:    vaddss %xmm1, %xmm2, %xmm1
 ; AVX-NEXT:    vpermilps {{.*#+}} xmm1 = xmm1[0,0,0,0]
-; AVX-NEXT:    vrcpps %xmm1, %xmm2
-; AVX-NEXT:    vmulps %xmm2, %xmm1, %xmm1
-; AVX-NEXT:    vmovaps {{.*#+}} xmm3 = [1.0E+0,1.0E+0,1.0E+0,1.0E+0]
-; AVX-NEXT:    vsubps %xmm1, %xmm3, %xmm1
-; AVX-NEXT:    vmulps %xmm1, %xmm2, %xmm1
-; AVX-NEXT:    vaddps %xmm1, %xmm2, %xmm1
 ; AVX-NEXT:    vmulps %xmm1, %xmm0, %xmm0
 ; AVX-NEXT:    retq
   %vy = insertelement <4 x float> undef, float %y, i32 0
   %splaty = shufflevector <4 x float> %vy, <4 x float> undef, <4 x i32> zeroinitializer
   %r = fdiv arcp reassoc <4 x float> %x, %splaty
   ret <4 x float> %r
 }
 
 define <8 x float> @splat_fdiv_v8f32_estimate(<8 x float> %x, float %y) #0 {
 ; SSE-LABEL: splat_fdiv_v8f32_estimate:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    rcpss %xmm2, %xmm3
 ; SSE-NEXT:    mulss %xmm3, %xmm2
 ; SSE-NEXT:    movss {{.*#+}} xmm4 = mem[0],zero,zero,zero
 ; SSE-NEXT:    subss %xmm2, %xmm4
 ; SSE-NEXT:    mulss %xmm3, %xmm4
 ; SSE-NEXT:    addss %xmm3, %xmm4
 ; SSE-NEXT:    shufps {{.*#+}} xmm4 = xmm4[0,0,0,0]
 ; SSE-NEXT:    mulps %xmm4, %xmm0
 ; SSE-NEXT:    mulps %xmm4, %xmm1
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: splat_fdiv_v8f32_estimate:
 ; AVX:       # %bb.0:
+; AVX-NEXT:    vrcpss %xmm1, %xmm1, %xmm2
+; AVX-NEXT:    vmulss %xmm2, %xmm1, %xmm1
+; AVX-NEXT:    vmovss {{.*#+}} xmm3 = mem[0],zero,zero,zero
+; AVX-NEXT:    vsubss %xmm1, %xmm3, %xmm1
+; AVX-NEXT:    vmulss %xmm1, %xmm2, %xmm1
+; AVX-NEXT:    vaddss %xmm1, %xmm2, %xmm1
 ; AVX-NEXT:    vpermilps {{.*#+}} xmm1 = xmm1[0,0,0,0]
 ; AVX-NEXT:    vinsertf128 $1, %xmm1, %ymm1, %ymm1
-; AVX-NEXT:    vrcpps %ymm1, %ymm2
-; AVX-NEXT:    vmulps %ymm2, %ymm1, %ymm1
-; AVX-NEXT:    vmovaps {{.*#+}} ymm3 = [1.0E+0,1.0E+0,1.0E+0,1.0E+0,1.0E+0,1.0E+0,1.0E+0,1.0E+0]
-; AVX-NEXT:    vsubps %ymm1, %ymm3, %ymm1
-; AVX-NEXT:    vmulps %ymm1, %ymm2, %ymm1
-; AVX-NEXT:    vaddps %ymm1, %ymm2, %ymm1
 ; AVX-NEXT:    vmulps %ymm1, %ymm0, %ymm0
 ; AVX-NEXT:    retq
   %vy = insertelement <8 x float> undef, float %y, i32 0
   %splaty = shufflevector <8 x float> %vy, <8 x float> undef, <8 x i32> zeroinitializer
   %r = fdiv fast <8 x float> %x, %splaty
   ret <8 x float> %r
 }
 
 attributes #0 = { "reciprocal-estimates"="divf,vec-divf" }