[DAGCombiner] scale repeated FP divisor by splat factor

[spatel]

Summary

If we have a vector FP division with a splatted divisor, we can use the existing transform that converts 'x/y' into 'x * (1.0/y)' to allow more conversions. This can then potentially be converted into a scalar FP division by existing combines (rL358984) as seen in the tests here.

That can be a potentially big perf difference if scalar fdiv has better timing (including avoiding possible frequency throttling for vector ops).

There's another diff here in the ordering of the transforms - I'm proposing to move the repeated divisor transform ahead of the reciprocal estimate transform because that seems more likely to produce the best results. For default x86, we don't turn fdiv f32 into an estimate because the estimate accuracy is too poor for most code. That's probably the right perf choice for current and future CPUs since divss throughput is down to the 3-4 cycle range (Skylake/Ryzen).

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

11987 ↗

(On Diff #196272)

Does this move need to be a separate patch?

[spatel]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

11987 ↗

(On Diff #196272)

Yes, let me change that back. I noticed that potential diff while looking at the other part of this patch, so I thought it would be good to see the 2 changes together, but it should stand independently assuming it makes sense.

[spatel]

Patch updated:
Remove reordering of reciprocal estimate and repeated divisor transforms. We still see a diff for the SSE target with an illegal type because we don't try to generate the reciprocal estimate sequence until the types are legal.

[RKSimon]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

11906 ↗

(On Diff #196470)

We probably want to not use this when optsize is enabled - add a TODO?

[spatel]

Patch updated:
Add a TODO comment about optimizing for size. That's an existing concern even for scalar code. But there may not be a clear answer for optsize because, for example, we may be able to hoist fdiv out of a loop by doing this transform.

[RKSimon]

LGTM

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

 // 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 targets
 // 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".
 SDValue DAGCombiner::combineRepeatedFPDivisors(SDNode *N) {
+  // TODO: Limit this transform based on optsize/minsize - it always creates at
+  //       least 1 extra instruction. But the perf win may be substantial enough
+  //       that only minsize should restrict this.
   bool UnsafeMath = DAG.getTarget().Options.UnsafeFPMath;
   const SDNodeFlags Flags = N->getFlags();
   if (!UnsafeMath && !Flags.hasAllowReciprocal())
     return SDValue();
 
   // Skip if current node is a reciprocal.
   SDValue N0 = N->getOperand(0);
   ConstantFPSDNode *N0CFP = dyn_cast<ConstantFPSDNode>(N0);
   if (N0CFP && N0CFP->isExactlyValue(1.0))
     return SDValue();
 
   // Exit early if the target does not want this transform or if there can't
   // possibly be enough uses of the divisor to make the transform worthwhile.
   SDValue N1 = N->getOperand(1);
   unsigned MinUses = TLI.combineRepeatedFPDivisors();
-  if (!MinUses || N1->use_size() < MinUses)
+
+  // For splat vectors, scale the number of uses by the splat factor. If we can
+  // convert the division into a scalar op, that will likely be much faster.
+  unsigned NumElts = 1;
+  EVT VT = N->getValueType(0);
+  if (VT.isVector() && DAG.isSplatValue(N1))
+    NumElts = VT.getVectorNumElements();
+
+  if (!MinUses || (N1->use_size() * NumElts) < MinUses)
     return SDValue();
 
   // Find all FDIV users of the same divisor.
   // Use a set because duplicates may be present in the user list.
   SetVector<SDNode *> Users;
   for (auto *U : N1->uses()) {
     if (U->getOpcode() == ISD::FDIV && U->getOperand(1) == N1) {
       // This division is eligible for optimization only if global unsafe math
       // is enabled or if this division allows reciprocal formation.
       if (UnsafeMath || U->getFlags().hasAllowReciprocal())
         Users.insert(U);
     }
   }
 
   // Now that we have the actual number of divisor uses, make sure it meets
   // the minimum threshold specified by the target.
-  if (Users.size() < MinUses)
+  if ((Users.size() * NumElts) < MinUses)
     return SDValue();
 
-  EVT VT = N->getValueType(0);
   SDLoc DL(N);
   SDValue FPOne = DAG.getConstantFP(1.0, DL, VT);
   SDValue Reciprocal = DAG.getNode(ISD::FDIV, DL, VT, FPOne, N1, Flags);
 
   // Dividend / Divisor -> Dividend * Reciprocal
   for (auto *U : Users) {
     SDValue Dividend = U->getOperand(0);
     if (Dividend != FPOne) {

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

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=x86_64-- -mattr=sse2 | FileCheck %s --check-prefix=SSE
 ; RUN: llc < %s -mtriple=x86_64-- -mattr=avx  | FileCheck %s --check-prefix=AVX
 
 define <2 x double> @splat_fdiv_v2f64(<2 x double> %x, double %y) {
 ; SSE-LABEL: splat_fdiv_v2f64:
 ; SSE:       # %bb.0:
-; SSE-NEXT:    unpcklpd {{.*#+}} xmm1 = xmm1[0,0]
-; SSE-NEXT:    divpd %xmm1, %xmm0
+; SSE-NEXT:    movsd {{.*#+}} xmm2 = mem[0],zero
+; SSE-NEXT:    divsd %xmm1, %xmm2
+; SSE-NEXT:    unpcklpd {{.*#+}} xmm2 = xmm2[0,0]
+; SSE-NEXT:    mulpd %xmm2, %xmm0
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: splat_fdiv_v2f64:
 ; AVX:       # %bb.0:
+; AVX-NEXT:    vmovsd {{.*#+}} xmm2 = mem[0],zero
+; AVX-NEXT:    vdivsd %xmm1, %xmm2, %xmm1
 ; AVX-NEXT:    vmovddup {{.*#+}} xmm1 = xmm1[0,0]
-; AVX-NEXT:    vdivpd %xmm1, %xmm0, %xmm0
+; AVX-NEXT:    vmulpd %xmm1, %xmm0, %xmm0
 ; AVX-NEXT:    retq
   %vy = insertelement <2 x double> undef, double %y, i32 0
   %splaty = shufflevector <2 x double> %vy, <2 x double> undef, <2 x i32> zeroinitializer
   %r = fdiv fast <2 x double> %x, %splaty
   ret <2 x double> %r
 }
 
 define <4 x double> @splat_fdiv_v4f64(<4 x double> %x, double %y) {
 ; SSE-LABEL: splat_fdiv_v4f64:
 ; SSE:       # %bb.0:
 ; SSE-NEXT:    movsd {{.*#+}} xmm3 = mem[0],zero
 ; SSE-NEXT:    divsd %xmm2, %xmm3
 ; SSE-NEXT:    unpcklpd {{.*#+}} xmm3 = xmm3[0,0]
 ; SSE-NEXT:    mulpd %xmm3, %xmm0
 ; SSE-NEXT:    mulpd %xmm3, %xmm1
 ; SSE-NEXT:    retq
 ;
 ; AVX-LABEL: splat_fdiv_v4f64:
 ; AVX:       # %bb.0:
+; AVX-NEXT:    vmovsd {{.*#+}} xmm2 = mem[0],zero
+; AVX-NEXT:    vdivsd %xmm1, %xmm2, %xmm1
 ; AVX-NEXT:    vmovddup {{.*#+}} xmm1 = xmm1[0,0]
 ; AVX-NEXT:    vinsertf128 $1, %xmm1, %ymm1, %ymm1
-; AVX-NEXT:    vdivpd %ymm1, %ymm0, %ymm0
+; AVX-NEXT:    vmulpd %ymm1, %ymm0, %ymm0
 ; AVX-NEXT:    retq
   %vy = insertelement <4 x double> undef, double %y, i32 0
   %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) {
; AVX-NEXT:    retq
   %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:    shufps {{.*#+}} xmm2 = xmm2[0,0,0,0]
-; SSE-NEXT:    rcpps %xmm2, %xmm3
-; SSE-NEXT:    mulps %xmm3, %xmm2
-; SSE-NEXT:    movaps {{.*#+}} xmm4 = [1.0E+0,1.0E+0,1.0E+0,1.0E+0]
-; SSE-NEXT:    subps %xmm2, %xmm4
-; SSE-NEXT:    mulps %xmm3, %xmm4
-; SSE-NEXT:    addps %xmm3, %xmm4
-; SSE-NEXT:    mulps %xmm4, %xmm0
-; SSE-NEXT:    mulps %xmm4, %xmm1
+; 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:    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