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

10542

No tests? Please rebase after rL334308.

[mcberg2017]

Updated as requested with some additional coverage.

[spatel]

LGTM.

[spatel]

test/CodeGen/X86/fp-fold.ll

104

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

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) {
 
   // fold (fmul A, 1.0) -> A
   if (N1CFP && N1CFP->isExactlyValue(1.0))
     return N0;
 
   if (SDValue NewSel = foldBinOpIntoSelect(N))
     return NewSel;
 
-  if (Options.UnsafeFPMath) {
+  if (Options.UnsafeFPMath || 
+      (Flags.hasNoNaNs() && Flags.hasNoSignedZeros())) {
     // fold (fmul A, 0) -> 0
     if (N1CFP && N1CFP->isZero())
       return N1;
+  } 
 
-    // fmul (fmul X, C1), X2 -> fmul X, C1 * C2
+  if (Options.UnsafeFPMath || Flags.hasAllowReassociation()) {
+    // fmul (fmul X, C1), C2 -> 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);

test/CodeGen/X86/fmul-combines.ll

 }
 
 ; Node-level FMF and no function-level attributes.
 
 define <4 x float> @fmul_v4f32_two_consts_no_splat_reassoc(<4 x float> %x) {
 ; CHECK-LABEL: fmul_v4f32_two_consts_no_splat_reassoc:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    mulps {{.*}}(%rip), %xmm0
-; CHECK-NEXT:    mulps {{.*}}(%rip), %xmm0
 ; CHECK-NEXT:    retq
   %y = fmul <4 x float> %x, <float 1.0, float 2.0, float 3.0, float 4.0>
   %z = fmul reassoc <4 x float> %y, <float 5.0, float 6.0, float 7.0, float 8.0>
   ret <4 x float> %z
 }
 
 ; Multiplication by 2.0 is a special case because that gets converted to fadd x, x.
 
 define <4 x float> @fmul_v4f32_two_consts_no_splat_reassoc_2(<4 x float> %x) {
 ; CHECK-LABEL: fmul_v4f32_two_consts_no_splat_reassoc_2:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    addps %xmm0, %xmm0
 ; CHECK-NEXT:    mulps {{.*}}(%rip), %xmm0
 ; CHECK-NEXT:    retq
   %y = fadd <4 x float> %x, %x
   %z = fmul reassoc <4 x float> %y, <float 5.0, float 6.0, float 7.0, float 8.0>
   ret <4 x float> %z
 }
 
 ; CHECK: float 6

test/CodeGen/X86/fp-fold.ll

 define float @fsub_negzero_nsz(float %x) {
 ; ANY-LABEL: fsub_negzero_nsz:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    retq
   %r = fsub nsz float %x, -0.0
   ret float %r
 }
 
 ; TODO: handle x*0 for fast flags the same as unsafe
 define float @fmul_zero(float %x) {
-; STRICT-LABEL: fmul_zero:
-; STRICT:       # %bb.0:
-; STRICT-NEXT:    xorps %xmm1, %xmm1
-; STRICT-NEXT:    mulss %xmm1, %xmm0
-; STRICT-NEXT:    retq
-;
-; UNSAFE-LABEL: fmul_zero:
-; UNSAFE:       # %bb.0:
-; UNSAFE-NEXT:    xorps %xmm0, %xmm0
-; UNSAFE-NEXT:    retq
+; ANY-LABEL: fmul_zero:
+; ANY:       # %bb.0:
+; ANY-NEXT:    xorps %xmm0, %xmm0
+; ANY-NEXT:    retq
   %r = fmul nnan nsz float %x, 0.0
   ret float %r
 }
 
 define float @fmul_one(float %x) {
 ; ANY-LABEL: fmul_one:
 ; ANY:       # %bb.0:
 ; ANY-NEXT:    retq
   %r = fmul float %x, 1.0
   ret float %r
 }
+
+define float @fmul_x_const_const(float %x) {
+; ANY-LABEL: fmul_x_const_const:
+; ANY:       # %bb.0:
+; ANY-NEXT:    mulss {{.*}}(%rip), %xmm0
+; ANY-NEXT:    retq
+  %mul = fmul reassoc float %x, 9.0
+  %r = fmul reassoc float %mul, 4.0
+  ret float %r
+}