[PATCH] Refactor and enhance FMA combine

Mehdi AMINI mehdi.amini at apple.com
Thu Mar 5 11:20:24 PST 2015

The code seems better after refactoring, thanks :)

See inline comments.

Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7060
@@ +7059,3 @@
+      // fold (fadd (fpext (fma x, y, (fmul u, v))), z)
+      //   -> (fma (fpext x), (fpext y), (fma (fpext u), (fpext v), z))
+      if (N0.getOpcode() == ISD::FP_EXTEND) {
This turns two "low precision" and one "high precision" operations into two "high precision" operation. 
I'm not saying it is necessarily bad, but I'm not convinced it is always beneficial.
The other FMA combines don't have the same behavior.

Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7100
@@ +7099,3 @@
+      // fold (fadd x, (fpext (fma y, z, (fmul u, v)))
+      //   -> (fma (fpext y), (fpext z), (fma (fpext u), (fpext v), x))
+      if (N1.getOpcode() == ISD::FP_EXTEND) {
The only change between this one and the one two level above is that the operand of the outer fadd are reversed. Is there an elegant way of not duplicating the code? A lambda maybe?

Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7214
@@ +7213,3 @@
+    // fold (fsub (fpext (fneg (fmul, x, y))), z)
+    //   -> (fneg (fma (fpext x), (fpext y), z))
+    if (N0.getOpcode() == ISD::FP_EXTEND) {
I come back with the canonicalization problem. Wouldn't it be more canonical to have:
  fneg (fadd (fpext (fmul, x, y)), z)

In which case this combine would be useless.



More information about the llvm-commits mailing list