[PATCH] D132837: [ISel] Enable generating more fma instructions.

[Jay Foad via Phabricator via llvm-commits]

foad added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14158
+  // fadd E, (fma A, B, (fmul C, D)) --> fma C, D, (fma A, B, E)
+  // This also works on with nested fma instructions:
+  // fadd (fma A, B, (fma (C, D, (fmul (E, F))))), G --> fma E, F, (fma C, D,
----------------
Remove "on"?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14159-14161
+  // fadd (fma A, B, (fma (C, D, (fmul (E, F))))), G --> fma E, F, (fma C, D,
+  // fma (A, B, G)) fadd (G, (fma A, B, (fma (C, D, (fmul (E, F)))))) --> fma E,
+  // F, (fma C, D, fma (A, B, G)).
----------------
Looks like clang-format has mangled this comment!


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14163-14164
   // This requires reassociation because it changes the order of operations.
-  SDValue FMA, E;
-  if (CanReassociate && isFusedOp(N0) &&
-      N0.getOperand(2).getOpcode() == ISD::FMUL && N0.hasOneUse() &&
-      N0.getOperand(2).hasOneUse()) {
-    FMA = N0;
-    E = N1;
-  } else if (CanReassociate && isFusedOp(N1) &&
-             N1.getOperand(2).getOpcode() == ISD::FMUL && N1.hasOneUse() &&
-             N1.getOperand(2).hasOneUse()) {
-    FMA = N1;
-    E = N0;
-  }
-  if (FMA && E) {
-    SDValue A = FMA.getOperand(0);
-    SDValue B = FMA.getOperand(1);
-    SDValue C = FMA.getOperand(2).getOperand(0);
-    SDValue D = FMA.getOperand(2).getOperand(1);
-    SDValue CDE = DAG.getNode(PreferredFusedOpcode, SL, VT, C, D, E);
-    return DAG.getNode(PreferredFusedOpcode, SL, VT, A, B, CDE);
+  // Moving the outermost FMA operands to the innermost FMA in the chain can
+  // help with eliminating a final copy to an output register.
+  if (CanReassociate) {
----------------
Can you explain this? It is not at all obvious. Are you sure it's not just random? Perhaps it helps your case, but could harm other cases?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132837