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

Matthias Braun matze at braunis.de
Thu Jul 16 13:40:27 PDT 2015


MatzeB added a comment.

In http://reviews.llvm.org/D10745#204649, @john.brawn wrote:

> 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.


I haven't heard of any architecture that fuses unrelated operations. If such a thing comes up then we can still add an option to the Mutator or the respective target can simply create a mutator of its own.


================
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;
+}
+
----------------
john.brawn wrote:
> 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.
Unfortunately MachineInstr::defs() only includes the explicit register defs, there may be additional implicit defs later in the operand list.


Repository:
  rL LLVM

http://reviews.llvm.org/D10745







More information about the llvm-commits mailing list