[InstCombine] Use SimplifyFMulInst to simplify multiply in fma.

[fhahn]

Summary

This allows us to fold fma's that multiply with 0.0. Also, the
multiply by 1.0 case is handled there as well. The fneg/fabs cases
are not handled by SimplifyFMulInst, so we need to keep them.

[fhahn]

Remove unrelated test-case.

[spatel]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2247–2252 ↗

(On Diff #219329)

I don't understand what this block does. How does an fmul simplify to a different fmul? And then it becomes LHS^2?

2260–2262 ↗

(On Diff #219329)

It would be easier to read if you move the simplify call below these local variables and then re-use those names.

llvm/test/Transforms/InstCombine/fma.ll

185 ↗

(On Diff #219329)

I'm not seeing how these got commuted.

372–375 ↗

(On Diff #219329)

Please add all of the new tests with baseline results as a preliminary commit, so we just have the diffs here.
Add one more test like:
call fast fma(sqrt(X), sqrt(X), Y) --> X*Y

[fhahn]

Address review comments, thanks!

[fhahn]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2247–2252 ↗

(On Diff #219329)

I don't understand what this block does. How does an fmul simplify to a different fmul?

If we simplify to a different fmul, we can update the existing FMA and we do not have to replace it with a new fmul + fadd. Not sure if that happens in practice though. Simplifications like the ones below ( -x * -y => x * y) could result in a different fmul. But currently we do not do this in SimplifyFMulInst. If there is a reason we never do create different fmuls there, then we can drop it.

And then it becomes LHS^2?

That should have been RHS of course.

2260–2262 ↗

(On Diff #219329)

Sure, I've moved it down.

llvm/test/Transforms/InstCombine/fma.ll

185 ↗

(On Diff #219329)

That was noise of update_test_checks.py I think. The way it generates checks for arguments, it will match both fmul %x, %y and fmul %y, %x.

I've stripped the unnecessary changes.

372–375 ↗

(On Diff #219329)

Done

[lebedev.ri]

Looks good to me, but i'm not really good at llvm's FP.

[spatel]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2247–2252 ↗

(On Diff #219329)

I think we should drop this clause...unless there is a test that shows the intended transform?
InstSimplify never creates new instructions because it can be used as an analysis rather than transform pass.
If we fold to a different multiply, then it should be optimized just as well as.
Eg, we have this fold in InstCombiner::visitFMul():

// -X * -Y --> X * Y

(can't do that in instsimplify because it's a new instruction)

[fhahn]

Remove fmul matching.

[fhahn]

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

2247–2252 ↗

(On Diff #219329)

Thanks for clarifying! I've dropped it.

[spatel]

LGTM

[fhahn]

Thanks!

llvm/trunk/lib/Transforms/InstCombine/InstCombineCalls.cpp

case Intrinsic::fma: {
     // fma fabs(x), fabs(x), z -> fma x, x, z
     if (match(Src0, m_FAbs(m_Value(X))) &&
         match(Src1, m_FAbs(m_Specific(X)))) {
       II->setArgOperand(0, X);
       II->setArgOperand(1, X);
       return II;
     }
 
-    // fma x, 1, z -> fadd x, z
-    if (match(Src1, m_FPOne())) {
-      auto *FAdd = BinaryOperator::CreateFAdd(Src0, II->getArgOperand(2));
+    // Try to simplify the underlying FMul.
+    if (Value *V = SimplifyFMulInst(II->getArgOperand(0), II->getArgOperand(1),
+                                    II->getFastMathFlags(),
+                                    SQ.getWithInstruction(II))) {
+      auto *FAdd = BinaryOperator::CreateFAdd(V, II->getArgOperand(2));
       FAdd->copyFastMathFlags(II);
       return FAdd;
     }
 
     break;
   }
   case Intrinsic::fabs: {
     Value *Cond;

llvm/trunk/test/Transforms/InstCombine/fma.ll

 ;
   %fmuladd = call fast float @llvm.fmuladd.f32(float %x, float 1.0, float %z)
   ret float %fmuladd
 }
 
 define <2 x double> @fmuladd_a_0_b(<2 x double> %a, <2 x double> %b) {
 ; CHECK-LABEL: @fmuladd_a_0_b(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[RES:%.*]] = call nnan nsz <2 x double> @llvm.fmuladd.v2f64(<2 x double> [[A:%.*]], <2 x double> zeroinitializer, <2 x double> [[B:%.*]])
-; CHECK-NEXT:    ret <2 x double> [[RES]]
+; CHECK-NEXT:    ret <2 x double> [[B:%.*]]
 ;
 entry:
   %res = call nnan nsz <2 x double> @llvm.fmuladd.v2f64(<2 x double> %a, <2 x double> zeroinitializer, <2 x double> %b)
   ret <2 x double> %res
 }
 
 define <2 x double> @fmuladd_0_a_b(<2 x double> %a, <2 x double> %b) {
 ; CHECK-LABEL: @fmuladd_0_a_b(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[RES:%.*]] = call nnan nsz <2 x double> @llvm.fmuladd.v2f64(<2 x double> [[A:%.*]], <2 x double> zeroinitializer, <2 x double> [[B:%.*]])
-; CHECK-NEXT:    ret <2 x double> [[RES]]
+; CHECK-NEXT:    ret <2 x double> [[B:%.*]]
 ;
 entry:
   %res = call nnan nsz <2 x double> @llvm.fmuladd.v2f64(<2 x double> zeroinitializer, <2 x double> %a, <2 x double> %b)
   ret <2 x double> %res
 }
 
 define <2 x double> @fmuladd_a_0_b_missing_flags(<2 x double> %a, <2 x double> %b) {
 ; CHECK-LABEL: @fmuladd_a_0_b_missing_flags(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[RES:%.*]] = call nnan <2 x double> @llvm.fmuladd.v2f64(<2 x double> [[A:%.*]], <2 x double> zeroinitializer, <2 x double> [[B:%.*]])
 ; CHECK-NEXT:    ret <2 x double> [[RES]]
 ;
 entry:
   %res = call nnan <2 x double> @llvm.fmuladd.v2f64(<2 x double> %a, <2 x double> zeroinitializer, <2 x double> %b)
   ret <2 x double> %res
 }
 
 declare <2 x double> @llvm.fmuladd.v2f64(<2 x double>, <2 x double>, <2 x double>)
 
 define <2 x double> @fma_a_0_b(<2 x double> %a, <2 x double> %b) {
 ; CHECK-LABEL: @fma_a_0_b(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[RES:%.*]] = call nnan nsz <2 x double> @llvm.fma.v2f64(<2 x double> [[A:%.*]], <2 x double> zeroinitializer, <2 x double> [[B:%.*]])
-; CHECK-NEXT:    ret <2 x double> [[RES]]
+; CHECK-NEXT:    ret <2 x double> [[B:%.*]]
 ;
 entry:
   %res = call nnan nsz <2 x double> @llvm.fma.v2f64(<2 x double> %a, <2 x double> zeroinitializer, <2 x double> %b)
   ret <2 x double> %res
 }
 
 define <2 x double> @fma_0_a_b(<2 x double> %a, <2 x double> %b) {
 ; CHECK-LABEL: @fma_0_a_b(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[RES:%.*]] = call nnan nsz <2 x double> @llvm.fma.v2f64(<2 x double> [[A:%.*]], <2 x double> zeroinitializer, <2 x double> [[B:%.*]])
-; CHECK-NEXT:    ret <2 x double> [[RES]]
+; CHECK-NEXT:    ret <2 x double> [[B:%.*]]
 ;
 entry:
   %res = call nnan nsz <2 x double> @llvm.fma.v2f64(<2 x double> zeroinitializer, <2 x double> %a, <2 x double> %b)
   ret <2 x double> %res
 }
 
 define <2 x double> @fma_0_a_b_missing_flags(<2 x double> %a, <2 x double> %b) {
 ; CHECK-LABEL: @fma_0_a_b_missing_flags(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[RES:%.*]] = call nsz <2 x double> @llvm.fma.v2f64(<2 x double> [[A:%.*]], <2 x double> zeroinitializer, <2 x double> [[B:%.*]])
 ; CHECK-NEXT:    ret <2 x double> [[RES]]
 ;
 entry:
   %res = call nsz <2 x double> @llvm.fma.v2f64(<2 x double> zeroinitializer, <2 x double> %a, <2 x double> %b)
   ret <2 x double> %res
 }
 
 define <2 x double> @fma_sqrt(<2 x double> %a, <2 x double> %b) {
 ; CHECK-LABEL: @fma_sqrt(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[SQRT:%.*]] = call fast <2 x double> @llvm.sqrt.v2f64(<2 x double> [[A:%.*]])
-; CHECK-NEXT:    [[RES:%.*]] = call fast <2 x double> @llvm.fma.v2f64(<2 x double> [[SQRT]], <2 x double> [[SQRT]], <2 x double> [[B:%.*]])
+; CHECK-NEXT:    [[RES:%.*]] = fadd fast <2 x double> [[A:%.*]], [[B:%.*]]
 ; CHECK-NEXT:    ret <2 x double> [[RES]]
 ;
 entry:
   %sqrt = call fast <2 x double> @llvm.sqrt.v2f64(<2 x double> %a)
   %res = call fast <2 x double> @llvm.fma.v2f64(<2 x double> %sqrt, <2 x double> %sqrt, <2 x double> %b)
   ret <2 x double> %res
 }
 
 
 declare <2 x double> @llvm.fma.v2f64(<2 x double>, <2 x double>, <2 x double>)
 declare <2 x double> @llvm.sqrt.v2f64(<2 x double>)
 
 
 attributes #0 = { nounwind }
 attributes #1 = { nounwind readnone }