[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