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

Kai Luo via llvm-commits llvm-commits at lists.llvm.org
Sat Aug 1 07:03:49 PDT 2026


================
@@ -1263,304 +1263,283 @@ void MachineCopyPropagation::backwardCopyPropagateBlock(
   Tracker.clear();
 }
 
-[[maybe_unused]] static void printSpillReloadChain(
-    DenseMap<MachineInstr *, SmallVector<MachineInstr *>> &SpillChain,
-    DenseMap<MachineInstr *, SmallVector<MachineInstr *>> &ReloadChain,
-    MachineInstr *Leader) {
-  auto &SC = SpillChain[Leader];
-  auto &RC = ReloadChain[Leader];
-  for (auto I = SC.rbegin(), E = SC.rend(); I != E; ++I)
-    (*I)->dump();
-  for (MachineInstr *MI : RC)
-    MI->dump();
-}
+/// StructReloadPair defines a pair of Machine Instructions where the Spill is
+/// the Instruction spilling the physical register, and the associated reload
+/// for that physical register. Take the following example
+/// r2 = COPY x5 // Spill
+/// ....
+/// r5 = COPY r2 // Reload
+struct SpillReloadPair {
+  MachineInstr *Spill = nullptr;
+  MachineInstr *Reload = nullptr;
+};
 
-// 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) {
+/// Defines a Spill Reload Chain that is present in a MachineBasicBlock. This
+/// will describe one single chain, and consist of the relevant
+/// SpillReloadPair's within the chain. A Spill Chain will, in Machine
+/// Instructions look as such: 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 The
+/// Chain will continue until a Scratch Register is not used, and the register
+/// used at Chain[i] is being spilled/reloaded at Chain [i+1]
+struct SpillReloadChain {
+  SmallVector<SpillReloadPair> Pairs;
+
+  SpillReloadChain(MachineInstr *Spill, MachineInstr *Reload) {
+    append(Spill, Reload);
+  }
 
-  // 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;
+  /// Is MI Spilled or Reloaded within the Chain?
+  bool contains(const MachineInstr *MI) {
+    return any_of(Pairs, [MI](const SpillReloadPair &Pair) {
+      return Pair.Spill == MI || Pair.Reload == MI;
+    });
   }
-  if (CopyCount < 6)
-    return;
+  bool containsReload(const MachineInstr *MI) {
+    return any_of(
+        Pairs, [MI](const SpillReloadPair &Pair) { return Pair.Reload == MI; });
+  }
+  void append(MachineInstr *Spill, MachineInstr *Reload) {
+    Pairs.push_back({Spill, Reload});
+  }
+};
 
-  // ChainLeader maps MI inside a spill-reload chain to its innermost reload COPY.
-  // Thus we can track if a MI belongs to an existing spill-reload chain.
-  DenseMap<MachineInstr *, MachineInstr *> ChainLeader;
-  // SpillChain maps innermost reload COPY of a spill-reload chain to a sequence
-  // of COPYs that forms spills of a spill-reload chain.
-  // ReloadChain maps innermost reload COPY of a spill-reload chain to a
-  // sequence of COPYs that forms reloads of a spill-reload chain.
-  DenseMap<MachineInstr *, SmallVector<MachineInstr *>> SpillChain, ReloadChain;
-  // If a COPY's Source has use or def until next COPY defines the Source,
-  // we put the COPY in this set to keep property#2.
+class SpillageCopyEliminator {
+  const TargetRegisterInfo &TRI;
+  const TargetInstrInfo &TII;
+  CopyTracker &Tracker;
+  bool UseCopyInstr;
+  // Maps the innermost reload COPY of a spill-reload chain to the COPYs that
+  // form the chain.
+  SmallVector<SpillReloadChain> Chains;
+  // If a COPY's source has a use or def before the next COPY defines that
+  // source, record it here to preserve property#2.
   DenseSet<const MachineInstr *> CopySourceInvalid;
 
-  auto TryFoldSpillageCopies =
-      [&, this](const SmallVectorImpl<MachineInstr *> &SC,
-                const SmallVectorImpl<MachineInstr *> &RC) {
-        assert(SC.size() == RC.size() && "Spill-reload should be paired");
-
-        // We need at least 3 pairs of copies for the transformation to apply,
-        // because the first outermost pair cannot be removed since we don't
-        // recolor outside of the chain and that we need at least one temporary
-        // spill slot to shorten the chain. If we only have a chain of two
-        // pairs, we already have the shortest sequence this code can handle:
-        // the outermost pair for the temporary spill slot, and the pair that
-        // use that temporary spill slot for the other end of the chain.
-        // TODO: We might be able to simplify to one spill-reload pair if collecting
-        // more infomation about the outermost COPY.
-        if (SC.size() <= 2)
-          return;
-
-        // If violate property#2, we don't fold the chain.
-        for (const MachineInstr *Spill : drop_begin(SC))
-          if (CopySourceInvalid.count(Spill))
-            return;
-
-        for (const MachineInstr *Reload : drop_end(RC))
-          if (CopySourceInvalid.count(Reload))
-            return;
-
-        auto CheckCopyConstraint = [this](Register Dst, Register Src) {
-          return TRI->getCommonMinimalPhysRegClass(Dst, Src);
-        };
-
-        auto UpdateReg = [](MachineInstr *MI, const MachineOperand *Old,
-                            const MachineOperand *New) {
-          for (MachineOperand &MO : MI->operands()) {
-            if (&MO == Old)
-              MO.setReg(New->getReg());
-          }
-        };
-
-        DestSourcePair InnerMostSpillCopy =
-            *isCopyInstr(*SC[0], *TII, UseCopyInstr);
-        DestSourcePair OuterMostSpillCopy =
-            *isCopyInstr(*SC.back(), *TII, UseCopyInstr);
-        DestSourcePair InnerMostReloadCopy =
-            *isCopyInstr(*RC[0], *TII, UseCopyInstr);
-        DestSourcePair OuterMostReloadCopy =
-            *isCopyInstr(*RC.back(), *TII, UseCopyInstr);
-        if (!CheckCopyConstraint(getSrcMCReg(OuterMostSpillCopy),
-                                 getSrcMCReg(InnerMostSpillCopy)) ||
-            !CheckCopyConstraint(getDstMCReg(InnerMostReloadCopy),
-                                 getDstMCReg(OuterMostReloadCopy)))
-          return;
-
-        SpillageChainsLength += SC.size() + RC.size();
-        NumSpillageChains += 1;
-        UpdateReg(SC[0], InnerMostSpillCopy.Destination,
-                  OuterMostSpillCopy.Source);
-        UpdateReg(RC[0], InnerMostReloadCopy.Source,
-                  OuterMostReloadCopy.Destination);
-
-        for (size_t I = 1; I < SC.size() - 1; ++I) {
-          SC[I]->eraseFromParent();
-          RC[I]->eraseFromParent();
-          NumDeletes += 2;
-        }
-      };
-
-  auto GetFoldableCopy =
-      [this](const MachineInstr &MaybeCopy) -> std::optional<DestSourcePair> {
-    if (MaybeCopy.getNumImplicitOperands() > 0)
+  std::optional<DestSourcePair> getFoldableCopy(const MachineInstr &MI) const {
+    if (MI.getNumImplicitOperands() > 0)
       return std::nullopt;
     std::optional<DestSourcePair> CopyOperands =
-        isCopyInstr(MaybeCopy, *TII, UseCopyInstr);
+        isCopyInstr(MI, TII, UseCopyInstr);
     if (!CopyOperands)
       return std::nullopt;
     auto [Dst, Src] = getDstSrcMCRegs(*CopyOperands);
-    if (Src && Dst && !TRI->regsOverlap(Src, Dst) &&
+    if (Src && Dst && !TRI.regsOverlap(Src, Dst) &&
         CopyOperands->Source->isRenamable() &&
         CopyOperands->Destination->isRenamable())
       return CopyOperands;
 
     return std::nullopt;
-  };
+  }
 
-  auto IsSpillReloadPair = [&](const MachineInstr &Spill,
-                               const MachineInstr &Reload) {
-    std::optional<DestSourcePair> FoldableSpillCopy = GetFoldableCopy(Spill);
+  bool isSpillReloadPair(const MachineInstr &Spill,
+                         const MachineInstr &Reload) const {
+    std::optional<DestSourcePair> FoldableSpillCopy = getFoldableCopy(Spill);
     if (!FoldableSpillCopy)
       return false;
-    std::optional<DestSourcePair> FoldableReloadCopy = GetFoldableCopy(Reload);
+    std::optional<DestSourcePair> FoldableReloadCopy = getFoldableCopy(Reload);
     if (!FoldableReloadCopy)
       return false;
     return FoldableSpillCopy->Source->getReg() ==
                FoldableReloadCopy->Destination->getReg() &&
            FoldableSpillCopy->Destination->getReg() ==
                FoldableReloadCopy->Source->getReg();
-  };
+  }
 
-  auto IsChainedCopy = [&](const MachineInstr &Prev,
-                           const MachineInstr &Current) {
-    std::optional<DestSourcePair> FoldablePrevCopy = GetFoldableCopy(Prev);
+  bool isChainedCopy(const MachineInstr &Prev,
+                     const MachineInstr &Current) const {
+    std::optional<DestSourcePair> FoldablePrevCopy = getFoldableCopy(Prev);
     if (!FoldablePrevCopy)
       return false;
     std::optional<DestSourcePair> FoldableCurrentCopy =
-        GetFoldableCopy(Current);
+        getFoldableCopy(Current);
     if (!FoldableCurrentCopy)
       return false;
     return FoldablePrevCopy->Source->getReg() ==
            FoldableCurrentCopy->Destination->getReg();
-  };
+  }
 
-  for (MachineInstr &MI : llvm::make_early_inc_range(MBB)) {
-    std::optional<DestSourcePair> CopyOperands =
-        isCopyInstr(MI, *TII, UseCopyInstr);
+  SpillReloadChain *findChainContainingReload(const MachineInstr *MI) {
+    for (SpillReloadChain &Chain : Chains) {
+      if (Chain.containsReload(MI))
+        return &Chain;
+    }
+    return nullptr;
+  }
+
+  bool isInAnyChain(MachineInstr *MI) {
+    return any_of(Chains,
+                  [MI](SpillReloadChain &Chain) { return Chain.contains(MI); });
+  }
+
+  bool canRewriteChainEndpoints(const DestSourcePair &InnerMostSpillCopy,
+                                const DestSourcePair &OuterMostSpillCopy,
+                                const DestSourcePair &InnerMostReloadCopy,
+                                const DestSourcePair &OuterMostReloadCopy) {
+    return TRI.getCommonMinimalPhysRegClass(getSrcMCReg(OuterMostSpillCopy),
+                                            getSrcMCReg(InnerMostSpillCopy)) &&
+           TRI.getCommonMinimalPhysRegClass(getDstMCReg(InnerMostReloadCopy),
+                                            getDstMCReg(OuterMostReloadCopy));
+  }
+
+  static void rewriteRegisterOperand(MachineInstr *MI,
+                                     const MachineOperand *Old,
+                                     const MachineOperand *New) {
+    for (MachineOperand &MO : MI->operands()) {
+      if (&MO == Old)
+        MO.setReg(New->getReg());
+    }
+  }
+
+  [[maybe_unused]] void printSpillReloadChain(SpillReloadChain *Chain) {
+    for (SpillReloadPair &Pair : Chain->Pairs) {
+      Pair.Spill->dump();
+      Pair.Reload->dump();
+    }
+  }
+
+  void tryFoldChain(SmallVector<SpillReloadPair> &Pairs) {
+
+    assert(any_of(Pairs,
+                  [](const SpillReloadPair &Pair) {
+                    return Pair.Spill != nullptr && Pair.Reload != nullptr;
+                  }) &&
+           "Spill-reload should be paired");
+
+    // We need at least 3 pairs of copies for the transformation to apply,
+    // because the first outermost pair cannot be removed since we don't
+    // recolor outside of the chain and that we need at least one temporary
+    // spill slot to shorten the chain. If we only have a chain of two
+    // pairs, we already have the shortest sequence this code can handle:
+    // the outermost pair for the temporary spill slot, and the pair that
+    // use that temporary spill slot for the other end of the chain.
+    // TODO: We might be able to simplify to one spill-reload pair if collecting
+    // more infomation about the outermost COPY.
+    if (Pairs.size() <= 2)
+      return;
+
+    // If violate property#2, we don't fold the chain.
+    for (SpillReloadPair &Pair : drop_begin(Pairs)) {
+      if (CopySourceInvalid.count(Pair.Spill))
+        return;
+    }
+    for (SpillReloadPair &Pair : drop_end(Pairs)) {
+      if (CopySourceInvalid.count(Pair.Reload))
+        return;
+    }
+
+    DestSourcePair InnerMostSpillCopy =
+        *isCopyInstr(*Pairs.front().Spill, TII, UseCopyInstr);
+    DestSourcePair OuterMostSpillCopy =
+        *isCopyInstr(*Pairs.back().Spill, TII, UseCopyInstr);
+    DestSourcePair InnerMostReloadCopy =
+        *isCopyInstr(*Pairs.front().Reload, TII, UseCopyInstr);
+    DestSourcePair OuterMostReloadCopy =
+        *isCopyInstr(*Pairs.back().Reload, TII, UseCopyInstr);
+    if (!canRewriteChainEndpoints(InnerMostSpillCopy, OuterMostSpillCopy,
+                                  InnerMostReloadCopy, OuterMostReloadCopy))
+      return;
+
+    SpillageChainsLength += Pairs.size() * 2;
+    NumSpillageChains += 1;
+    rewriteRegisterOperand(Pairs.front().Spill, InnerMostSpillCopy.Destination,
+                           OuterMostSpillCopy.Source);
+    rewriteRegisterOperand(Pairs.front().Reload, InnerMostReloadCopy.Source,
+                           OuterMostReloadCopy.Destination);
+
+    for (size_t I = 1; I < Pairs.size() - 1; ++I) {
+      Pairs[I].Spill->eraseFromParent();
+      Pairs[I].Reload->eraseFromParent();
+      NumDeletes += 2;
+    }
+  }
 
-    // Update track information via non-copy instruction.
+  void processNonCopy(MachineInstr &MI) {
     SmallSet<Register, 8> RegsToClobber;
-    if (!CopyOperands) {
-      for (const MachineOperand &MO : MI.operands()) {
-        if (MO.isRegMask()) {
-          BitVector &PreservedRegUnits = Tracker.getPreservedRegUnits(MO, *TRI);
-          Tracker.clobberNonPreservedRegs(PreservedRegUnits, *TRI, *TII);
-          continue;
-        }
-        if (!MO.isReg())
-          continue;
-        Register Reg = MO.getReg();
-        if (!Reg)
-          continue;
-        MachineInstr *LastUseCopy =
-            Tracker.findLastSeenUseInCopy(Reg.asMCReg(), *TRI);
-        if (LastUseCopy) {
-          LLVM_DEBUG(dbgs() << "MCP: Copy source of\n");
-          LLVM_DEBUG(LastUseCopy->dump());
-          LLVM_DEBUG(dbgs() << "might be invalidated by\n");
-          LLVM_DEBUG(MI.dump());
-          CopySourceInvalid.insert(LastUseCopy);
-        }
-        // Must be noted Tracker.clobberRegister(Reg, ...) removes tracking of
-        // Reg, i.e, COPY that defines Reg is removed from the mapping as well
-        // as marking COPYs that uses Reg unavailable.
-        // We don't invoke CopyTracker::clobberRegister(Reg, ...) if Reg is not
-        // defined by a previous COPY, since we don't want to make COPYs uses
-        // Reg unavailable.
-        if (Tracker.findLastSeenDefInCopy(MI, Reg.asMCReg(), *TRI, *TII,
-                                    UseCopyInstr))
-          // Thus we can keep the property#1.
-          RegsToClobber.insert(Reg);
+    for (const MachineOperand &MO : MI.operands()) {
+      if (MO.isRegMask()) {
+        BitVector &PreservedRegUnits = Tracker.getPreservedRegUnits(MO, TRI);
+        Tracker.clobberNonPreservedRegs(PreservedRegUnits, TRI, TII);
+        continue;
       }
-      for (Register Reg : RegsToClobber) {
-        Tracker.clobberRegister(Reg, *TRI, *TII, UseCopyInstr);
-        LLVM_DEBUG(dbgs() << "MCP: Removed tracking of " << printReg(Reg, TRI)
-                          << "\n");
+      if (!MO.isReg())
+        continue;
+      Register Reg = MO.getReg();
+      if (!Reg)
+        continue;
+      MachineInstr *LastUseCopy =
+          Tracker.findLastSeenUseInCopy(Reg.asMCReg(), TRI);
+      if (LastUseCopy) {
+        LLVM_DEBUG(dbgs() << "MCP: Copy source of\n");
+        LLVM_DEBUG(LastUseCopy->dump());
+        LLVM_DEBUG(dbgs() << "might be invalidated by\n");
+        LLVM_DEBUG(MI.dump());
+        CopySourceInvalid.insert(LastUseCopy);
       }
-      continue;
+      // Must be noted Tracker.clobberRegister(Reg, ...) removes tracking of
+      // Reg, i.e, COPY that defines Reg is removed from the mapping as well
+      // as marking COPYs that uses Reg unavailable.
+      // We don't invoke CopyTracker::clobberRegister(Reg, ...) if Reg is not
+      // defined by a previous COPY, since we don't want to make COPYs uses
+      // Reg unavailable.
+      if (Tracker.findLastSeenDefInCopy(MI, Reg.asMCReg(), TRI, TII,
+                                        UseCopyInstr))
+        // Thus we can keep the property#1.
+        RegsToClobber.insert(Reg);
     }
+    for (Register Reg : RegsToClobber) {
+      Tracker.clobberRegister(Reg, TRI, TII, UseCopyInstr);
+      LLVM_DEBUG(dbgs() << "MCP: Removed tracking of " << printReg(Reg, &TRI)
+                        << "\n");
+    }
+  }
 
-    auto [Dst, Src] = getDstSrcMCRegs(*CopyOperands);
+  SpillReloadChain *getReloadChain(MachineInstr *Reload, MCRegister Dst) {
+    // We found a spill-reload pair:
+    //   L2: r2 = COPY r3
+    //   L5: r3 = COPY r2
+    // Look for a valid COPY before L5 which uses r3.
+    MachineInstr *MaybePrevReload = Tracker.findLastSeenUseInCopy(Dst, TRI);
+    SpillReloadChain *Chain = findChainContainingReload(MaybePrevReload);
+    if (!Chain ||
+        (MaybePrevReload && !isChainedCopy(*MaybePrevReload, *Reload)))
+      return nullptr;
+
+    assert(MaybePrevReload &&
+           "Found a valid leader through nullptr should not happend");
+    assert(Chain->Pairs.size() > 0 &&
+           "Existing chain's length should be larger than zero");
+    return Chain;
+  }
+
+  void addSpillReloadPairToChain(MachineInstr *Reload, MachineInstr *Spill,
+                                 MCRegister Dst) {
+    LLVM_DEBUG(dbgs() << "MCP: Found spill: ");
+    LLVM_DEBUG(Spill->dump());
+
+    SpillReloadChain *Chain = getReloadChain(Reload, Dst);
+    if (!Chain) {
+      SpillReloadChain NewChain = {Spill, Reload};
+      Chains.push_back(NewChain);
+      Chain = &Chains.back();
+    } else {
+      Chain->Pairs.push_back({Spill, Reload});
+    }
+    LLVM_DEBUG(dbgs() << "MCP: Chain " << Chain->Pairs.front().Reload
+                      << " now is:\n");
+    LLVM_DEBUG(printSpillReloadChain(Chain));
+  }
+
+  void processCopy(MachineInstr &MI, const DestSourcePair &CopyOperands) {
+    auto [Dst, Src] = getDstSrcMCRegs(CopyOperands);
     // Check if we can find a pair spill-reload copy.
     LLVM_DEBUG(dbgs() << "MCP: Searching paired spill for reload: ");
     LLVM_DEBUG(MI.dump());
     MachineInstr *MaybeSpill =
-        Tracker.findAvailCopy(MI, Src, *TRI, *TII, UseCopyInstr);
-    bool MaybeSpillIsChained = ChainLeader.count(MaybeSpill);
+        Tracker.findAvailCopy(MI, Src, TRI, TII, UseCopyInstr);
----------------
bzEq wrote:

I think this part can also be simplified. If we have found a valid Spill(the spill must not be contained in any chain) for this reload, the pair of spill/reload should be either a chain member or a chain leader.
If the the most rencent use of the reload's dst is a copy's src and the copy is already in the chain, then this pair should be a chain member, otherwise a chain leader.

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


More information about the llvm-commits mailing list