[PATCH] D28489: [CodeGen] Move MacroFusion to the target

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:2054-2063
+  ScheduleDAGMI *DAG = static_cast<ScheduleDAGMI*>(DAGInstrs);
+
+  // For each of the SUnits in the scheduling block, try to fuse the instruction
+  // in it with one in its successors.
+  for (SUnit &ASU : DAG->SUnits)
+    scheduleAdjacentImpl(*this, Subtarget, DAG, &ASU, ASU.Succs, false);
+
----------------
This code could be put directly in `AArch64MacroFusion::apply()`. The same comment applies to the X86 version.


================
Comment at: llvm/lib/Target/AArch64/AArch64MacroFusion.h:26-35
+class AArch64MacroFusion : public ScheduleDAGMutation {
+  const AArch64InstrInfo &TII;
+public:
+  AArch64MacroFusion(const AArch64InstrInfo &TII)
+    : TII(TII) {}
+
+  void apply(ScheduleDAGInstrs *DAGInstrs) override {
----------------
This can stay private to the .cpp file where createMacroFusionDAGMutation() is defined and doesn't need to go into a header.

The same comment applies to the X86 version.


================
Comment at: llvm/test/CodeGen/AArch64/misched-fusion.ll:10-19
-; CHECK: add w[[ADDRES:[0-9]+]], w1, #7
 ; CHECK: sub w[[SUBRES:[0-9]+]], w0, #13
-; CHECK-NEXT: cbnz w[[SUBRES]], [[SKIPBLOCK:LBB[0-9_]+]]
-; CHECK: mov [[REGTY:[x,w]]]0, [[REGTY]][[ADDRES]]
-; CHECK: mov [[REGTY]]1, [[REGTY]][[SUBRES]]
-; CHECK: bl _foobar
-; CHECK: [[SKIPBLOCK]]:
----------------
Why is there a testcase change, shoulnd't this be NFC?


Repository:
  rL LLVM

https://reviews.llvm.org/D28489