[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

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

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

I'm not seeing how these got commuted.

372–375

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

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

Sure, I've moved it down.

llvm/test/Transforms/InstCombine/fma.ll

185

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

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

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

Thanks for clarifying! I've dropped it.

[spatel]

LGTM

[fhahn]

Thanks!

llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

case Intrinsic::fmuladd: {
     }
 
     LLVM_FALLTHROUGH;
   }
   case Intrinsic::fma: {
     if (Instruction *I = canonicalizeConstantArg0ToArg1(CI))
       return I;
 
+    // Try to simplify the underlying FMul.
+    if (Value *V = SimplifyFMulInst(II->getArgOperand(0), II->getArgOperand(1),
+                                    II->getFastMathFlags(),
+                                    SQ.getWithInstruction(II))) {
+      Value *RHS, *LHS;
+      if (match(V, m_FMul(m_Value(LHS), m_Value(RHS)))) {
+        II->setArgOperand(0, LHS);
+        II->setArgOperand(1, LHS);
+        return II;
+      }
+
+      auto *FAdd = BinaryOperator::CreateFAdd(V, II->getArgOperand(2));
+      FAdd->copyFastMathFlags(II);
+      return FAdd;
+    }
+
     // fma fneg(x), fneg(y), z -> fma x, y, z
     Value *Src0 = II->getArgOperand(0);
     Value *Src1 = II->getArgOperand(1);
     Value *X, *Y;
     if (match(Src0, m_FNeg(m_Value(X))) && match(Src1, m_FNeg(m_Value(Y)))) {
       II->setArgOperand(0, X);
       II->setArgOperand(1, Y);
       return II;
     }
 
     // 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));
-      FAdd->copyFastMathFlags(II);
-      return FAdd;
-    }
-
     break;
   }
+
   case Intrinsic::fabs: {
     Value *Cond;
     Constant *LHS, *RHS;
     if (match(II->getArgOperand(0),
               m_Select(m_Value(Cond), m_Constant(LHS), m_Constant(RHS)))) {
       CallInst *Call0 = Builder.CreateCall(II->getCalledFunction(), {LHS});
       CallInst *Call1 = Builder.CreateCall(II->getCalledFunction(), {RHS});
       return SelectInst::Create(Cond, Call0, Call1);

llvm/test/Transforms/InstCombine/fma.ll

;
   %x.fneg = fneg float %x
   %y.fneg = fneg float %y
   %fmuladd = call float @llvm.fmuladd.f32(float %x.fneg, float %y.fneg, float %z)
   ret float %fmuladd
 }
 
 define float @fmuladd_fneg_x_fneg_y_fast(float %x, float %y, float %z) {
 ; CHECK-LABEL: @fmuladd_fneg_x_fneg_y_fast(
-; CHECK-NEXT:    [[TMP1:%.*]] = fmul fast float [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = fmul fast float [[Y:%.*]], [[X:%.*]]
 ; CHECK-NEXT:    [[FMULADD:%.*]] = fadd fast float [[TMP1]], [[Z:%.*]]
 ; CHECK-NEXT:    ret float [[FMULADD]]
 ;
   %x.fneg = fsub float -0.0, %x
   %y.fneg = fsub float -0.0, %y
   %fmuladd = call fast float @llvm.fmuladd.f32(float %x.fneg, float %y.fneg, float %z)
   ret float %fmuladd
 }
 
 define float @fmuladd_unary_fneg_x_unary_fneg_y_fast(float %x, float %y, float %z) {
 ; CHECK-LABEL: @fmuladd_unary_fneg_x_unary_fneg_y_fast(
-; CHECK-NEXT:    [[TMP1:%.*]] = fmul fast float [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    [[TMP1:%.*]] = fmul fast float [[Y:%.*]], [[X:%.*]]
 ; CHECK-NEXT:    [[FMULADD:%.*]] = fadd fast float [[TMP1]], [[Z:%.*]]
 ; CHECK-NEXT:    ret float [[FMULADD]]
 ;
   %x.fneg = fneg float %x
   %y.fneg = fneg float %y
   %fmuladd = call fast float @llvm.fmuladd.f32(float %x.fneg, float %y.fneg, float %z)
   ret float %fmuladd
 }

 ; CHECK-LABEL: @fmuladd_x_1_z_fast(
 ; CHECK-NEXT:    [[FMULADD:%.*]] = fadd fast float [[X:%.*]], [[Z:%.*]]
 ; CHECK-NEXT:    ret float [[FMULADD]]
 ;
   %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:    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:    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:    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:    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
+}
+
+declare <2 x double> @llvm.fma.v2f64(<2 x double>, <2 x double>, <2 x double>)
+
+
 attributes #0 = { nounwind }
 attributes #1 = { nounwind readnone }