[PATCH] D80801: [DAGCombiner] allow more folding of fadd + fmul into fma

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri accepted this revision.
lebedev.ri added a comment.
This revision is now accepted and ready to land.

Seems reasonable to me, but please wait for someone else.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11830
 
+  // fold (fadd (fma x, y, (fmul u, v)), z) -> (fma x, y (fma u, v, z))
+  if (CanFuse &&
----------------
Comma missing.

I understand that it is preexisting style here, but i find it to more hurt readability to name operands.
I'd think
```
  // fold (fadd (fma N00, N01, (fmul N020, N021)), N1) -> (fma N00, N01, (fma N020, N021, N1))
```
is more readable given the code.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11831
+  // fold (fadd (fma x, y, (fmul u, v)), z) -> (fma x, y (fma u, v, z))
+  if (CanFuse &&
+      N0.getOpcode() == PreferredFusedOpcode &&
----------------
We only care if the root node `N` allows producing FMA, we don't care about leafs?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:11843
+
+  // fold (fadd x, (fma y, z, (fmul u, v)) -> (fma y, z (fma u, v, x))
+  if (CanFuse &&
----------------
Comma missing,

```
  // fold (fadd N0, (fma N10, N11, (fmul N120, N121)) -> (fma N10, N11, (fma N120, N121, N0))
```



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

https://reviews.llvm.org/D80801