[PATCH] D36704: [CodeGen] Improve the consistency of instruction fusion

[Evandro Menezes via Phabricator via llvm-commits]

evandro marked 2 inline comments as done.
evandro added inline comments.


================
Comment at: llvm/lib/CodeGen/MacroFusion.cpp:38-46
+  // Check that neither instr is already paired with another along the edge
+  // between them.
+  for (SDep &SI : FirstSU.Succs)
+    if (SI.isCluster())
+      return false;
+
+  for (SDep &SI : SecondSU.Preds)
----------------
MatzeB wrote:
> Given the code below that adds all succs of first as succs of second, and vice versa for preds of seconds. How can the situation you are checking here even happen without us failing the cycle check in addEdge anyway?
> 
It can happen that an instr can be eligible for fusion for more than one other instruction.  This check limits the fusion to the first seen eligible one.


================
Comment at: llvm/lib/CodeGen/MacroFusion.cpp:82-94
+  // Make the FirstSU also dependent on the dependencies of the SecondSU to
+  // prevent them from being scheduled between the FirstSU and the SecondSU.
+  for (const SDep &SI : SecondSU.Preds) {
+    SUnit *SU = SI.getSUnit();
+    if (SI.isWeak() || (SI.getKind() != SDep::Order && SI.isCtrl()) ||
+        SU == &DAG.ExitSU || &FirstSU == &DAG.ExitSU ||
+        &FirstSU == SU || FirstSU.isSucc(SU))
----------------
MatzeB wrote:
> This seems unnecessary for SecondSU == ExitSU, maybe shortcut it?
Methinks that it is then, to avoid the predecessors of the ExitSU from being scheduled between it and the FirstSU.


Repository:
  rL LLVM

https://reviews.llvm.org/D36704