[llvm] [MCP][NFC] Refactor eliminateSpillageCopies (PR #208210)

[Jack Styles via llvm-commits]

================
@@ -1578,20 +1545,88 @@ void MachineCopyPropagation::eliminateSpillageCopies(MachineBasicBlock &MBB) {
       LLVM_DEBUG(dbgs() << "MCP: Not paired spill-reload:\n");
       LLVM_DEBUG(MaybeSpill->dump());
       LLVM_DEBUG(MI.dump());
-      Tracker.clobberRegister(Src, *TRI, *TII, UseCopyInstr);
-      LLVM_DEBUG(dbgs() << "MCP: Removed tracking of " << printReg(Src, TRI)
+      Tracker.clobberRegister(Src, TRI, TII, UseCopyInstr);
+      LLVM_DEBUG(dbgs() << "MCP: Removed tracking of " << printReg(Src, &TRI)
                         << "\n");
     }
-    Tracker.trackCopy(&MI, *TRI, *TII, UseCopyInstr);
   }
 
-  for (auto I = SpillChain.begin(), E = SpillChain.end(); I != E; ++I) {
-    auto &SC = I->second;
-    assert(ReloadChain.count(I->first) &&
-           "Reload chain of the same leader should exist");
-    auto &RC = ReloadChain[I->first];
-    TryFoldSpillageCopies(SC, RC);
+public:
+  SpillageCopyEliminator(const TargetRegisterInfo &TRI,
+                         const TargetInstrInfo &TII, CopyTracker &Tracker,
+                         bool UseCopyInstr)
+      : TRI(TRI), TII(TII), Tracker(Tracker), UseCopyInstr(UseCopyInstr) {}
+
+  void run(MachineBasicBlock &MBB) {
+    for (MachineInstr &MI : llvm::make_early_inc_range(MBB)) {
+      std::optional<DestSourcePair> CopyOperands =
+          isCopyInstr(MI, TII, UseCopyInstr);
+      if (!CopyOperands) {
+        processNonCopy(MI);
+        continue;
+      }
+
+      processCopy(MI, *CopyOperands);
+      Tracker.trackCopy(&MI, TRI, TII, UseCopyInstr);
+    }
+
+    for (auto &Chain : Chains)
+      tryFoldChain(Chain.Pairs);
+  }
+};
+
+// Remove spill-reload like copy chains. For example
+// r0 = COPY r1
+// r1 = COPY r2
+// r2 = COPY r3
+// r3 = COPY r4
+// <def-use r4>
+// r4 = COPY r3
+// r3 = COPY r2
+// r2 = COPY r1
+// r1 = COPY r0
+// will be folded into
+// r0 = COPY r1
+// r1 = COPY r4
+// <def-use r4>
+// r4 = COPY r1
+// r1 = COPY r0
+// TODO: Currently we don't track usage of r0 outside the chain, so we
+// conservatively keep its value as it was before the rewrite.
+//
+// The algorithm is trying to keep
+// property#1: No Dst of spill COPY in the chain is used or defined until the
+// paired reload COPY in the chain uses the Dst.
+//
+// property#2: NO Source of COPY in the chain is used or defined until the next
+// COPY in the chain defines the Source, except the innermost spill-reload
+// pair.
+//
+// The algorithm is conducted by checking every COPY inside the MBB, assuming
+// the COPY is a reload COPY, then try to find paired spill COPY by searching
+// the COPY defines the Src of the reload COPY backward. If such pair is found,
+// it either belongs to an existing chain or a new chain depends on
+// last available COPY uses the Dst of the reload COPY.
+// Implementation notes, we use CopyTracker::findLastDefCopy(Reg, ...) to find
+// out last COPY that defines Reg; we use CopyTracker::findLastUseCopy(Reg, ...)
+// to find out last COPY that uses Reg. When we are encountered with a Non-COPY
+// instruction, we check registers in the operands of this instruction. If this
+// Reg is defined by a COPY, we untrack this Reg via
+// CopyTracker::clobberRegister(Reg, ...).
+void MachineCopyPropagation::eliminateSpillageCopies(MachineBasicBlock &MBB) {
+
+  // Perform some cost modelling to ensure that only MBB's with more
+  // than 6 copies are checked. To create a chain that can be optimised,
+  // 6 copies are needed.
+  unsigned CopyCount = 0;
+  for (const MachineInstr &MI : MBB) {
+    if (isCopyInstr(MI, *TII, UseCopyInstr) && ++CopyCount > 6)
+      break;
   }
+  if (CopyCount < 6)
----------------
Stylie777 wrote:

I missed a comment here when I initially added this. `6` is the magic number as it's the number of `COPY`'s in which we can reliably form a chain that can be simplified. Previously we would do alot of intensive work on MBB's that could never include a chain which can be optimised.

Adding it as a `cl::opt` I am open to as it would make it more strict, but the user has an option to disable this pass all together too. I'm open to either approach but I have added a comment here to explain why `6` is the magic number for this.

https://github.com/llvm/llvm-project/pull/208210