Utilize new SDNode flag functionality to expand current support for fmul

[mcberg2017]

Summary

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

[spatel]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

10540

No tests? Please rebase after rL334308.

[mcberg2017]

Updated as requested with some additional coverage.

[spatel]

LGTM.

[spatel]

test/CodeGen/X86/fp-fold.ll

104 ↗

(On Diff #150549)

Can remove the TODO now.

[spatel]

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

724–726

Hang on - I missed this change. Why does this require nsz?

[mcberg2017]

Good catch, it should be the same as fdiv, nnan for that case.

[spatel]

In D47911#1126817, @mcberg2017 wrote:

Good catch, it should be the save as fdiv, nnan for that case.

Hmm...how does NaN change anything?

Instcombine moves fneg without any FMF:

// Sink negation: -X * Y --> -(X * Y)

[mcberg2017]

So Options.HonorSignDependentRoundingFPMath is sufficient to guard fmul negate and we can drop the unsafe context, is that only true for multiply? What about divide?

[spatel]

In D47911#1126828, @mcberg2017 wrote:

So Options.HonorSignDependentRoundingFPMath is sufficient to guard fmul negate and we can drop the unsafe context, is that only true for multiply? What about divide?

I've never looked at HonorSignDependentRoundingFPMath:

/// If this is enabled (set to true), the code generator must
/// assume that the rounding mode may dynamically change.

That would seem to preclude a bunch of transforms, but I don't know why that's guarding this particular transform.

We need to split this diff into its own patch and find the existing tests and/or add new tests, so we know what's going on here.

fmul and fdiv should be treated the same with regard to fneg transforms (although instcombine probably gets this wrong).

[mcberg2017]

Removed negation context...

[mcberg2017]

fmul updates...

[spatel]

LGTM.

test/CodeGen/X86/fp-fold.ll

104 ↗

(On Diff #150549)

Stale TODO still here.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

case ISD::FSUB:
 
     // 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::FP_EXTEND:
   case ISD::FP_ROUND:
   case ISD::FSIN:
SDValue DAGCombiner::visitFMUL(SDNode *N) {
   if (SDValue NewSel = foldBinOpIntoSelect(N))
     return NewSel;
 
   if (Options.UnsafeFPMath) {
     // fold (fmul A, 0) -> 0
     if (N1CFP && N1CFP->isZero())
       return N1;
 
     // fmul (fmul X, C1), X2 -> fmul X, C1 * C2
     if (N0.getOpcode() == ISD::FMUL) {
       // Fold scalars or any vector constants (not just splats).
       // This fold is done in general by InstCombine, but extra fmul insts
       // may have been generated during lowering.
       SDValue N00 = N0.getOperand(0);
       SDValue N01 = N0.getOperand(1);
       auto *BV1 = dyn_cast<BuildVectorSDNode>(N1);
       auto *BV00 = dyn_cast<BuildVectorSDNode>(N00);
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)

 
 ; 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)