[PATCH] D10745: MachineScheduler: Avoid pointless macroop fusion edges.

[John Brawn]

john.brawn added a subscriber: john.brawn.
john.brawn added a comment.

Looks generally OK to me, with one comment. Arguably the HasDataDep check should be done inside of shouldScheduleAdjacent as it is potentially possibly that there's some target that does instruction fusion on instructions without a data dependency, but neither of the two targets that currently define shouldScheduleAdjacent do, and it would be a rather strange thing to do as well, so I think it's OK.


================
Comment at: lib/CodeGen/MachineScheduler.cpp:1362-1382
@@ -1359,1 +1361,23 @@
 
+/// Returns true if \p MI reads a register written by \p Other.
+static bool HasDataDep(const TargetRegisterInfo &TRI, const MachineInstr &MI,
+                       const MachineInstr &Other) {
+  for (const MachineOperand &MO : MI.uses()) {
+    if (!MO.isReg() || !MO.readsReg())
+      continue;
+    unsigned MOReg = MO.getReg();
+
+    for (const MachineOperand &OtherMO : Other.operands()) {
+      if (!OtherMO.isReg() || !OtherMO.isDef())
+        continue;
+      unsigned OtherReg = OtherMO.getReg();
+      if (MOReg == OtherReg ||
+          (TargetRegisterInfo::isPhysicalRegister(MOReg) &&
+           TargetRegisterInfo::isPhysicalRegister(OtherReg) &&
+           TRI.isSuperRegisterEq(MOReg, OtherReg)))
+        return true;
+    }
+  }
+  return false;
+}
+
----------------
According to MCInstrDesc.h def operands are always registers, so inverting this to loop over the defs of Other would simplify things. You can also replace the inner loop with MachineInstr::readsRegister, which looks like it does the subregister checking that you want when passed a TargetRegisterInfo.


Repository:
  rL LLVM

http://reviews.llvm.org/D10745