[PATCH] Refactor and enhance FMA combine

Mehdi AMINI mehdi.amini at apple.com
Tue Mar 3 18:42:18 PST 2015


These are very interesting transformation! See comments inline.

Thanks.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:6982
@@ +6981,3 @@
+
+    // fold (fadd x, (fpext (fmul y, z)), z) -> (fma (fpext y), (fpext z), x)
+    // Note: Commutes FADD operands.
----------------
 // fold (fadd x, (fpext (fmul y, z)), z)

-> isn't there an extra , z)

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7107
@@ -6995,2 +7106,3 @@
                                        bool Aggressive,
+                                       bool ArrangeFPExt,
                                        SDNode *N,
----------------
The existing function is already not document, but adding a new argument only exhibits how lacking it is.
Can you add a comment on top of this function. I'd rather see defined what is the meaning of at least the three first arguments. 

It should also be specified if this function can be called outside of fast-math and what combination of parameter is allowed in this case.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7175
@@ +7174,3 @@
+
+    // fold (fsub (fpext (fneg (fmul, x, y))), z)
+    //   -> (fma (fneg (fpext x)), (fpext y), (fneg z))
----------------
I wonder if this is canonical?
Couldn't it be transformed to:

```
fneg (fadd (fpext (fmul, x, y)), z)
```

================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:7193
@@ +7192,3 @@
+
+    // fold (fsub (fneg (fpext (fmul, x, y))), z)
+    //   -> (fma (fneg (fpext x)), (fpext y), (fneg z))
----------------
Same canonicalization question, seems redundant with the one before.

http://reviews.llvm.org/D8050

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list