[PATCH] D67351: [InstCombine] Use SimplifyFMulInst to simplify multiply in fma.

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:2247-2252
+      Value *RHS, *LHS;
+      if (match(V, m_FMul(m_Value(LHS), m_Value(RHS)))) {
+        II->setArgOperand(0, LHS);
+        II->setArgOperand(1, LHS);
+        return II;
+      }
----------------
I don't understand what this block does. How does an fmul simplify to a different fmul? And then it becomes LHS^2?


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:2260-2262
     Value *Src0 = II->getArgOperand(0);
     Value *Src1 = II->getArgOperand(1);
     Value *X, *Y;
----------------
It would be easier to read if you move the simplify call below these local variables and then re-use those names.


================
Comment at: llvm/test/Transforms/InstCombine/fma.ll:185
 ; 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:%.*]]
----------------
I'm not seeing how these got commuted.


================
Comment at: llvm/test/Transforms/InstCombine/fma.ll:372-375
+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:%.*]]
----------------
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


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67351/new/

https://reviews.llvm.org/D67351