[RegScavenger] Remove support for forwards register scavenging

[arsenm]

Should get mentioned in the release notes

[foad]

Release notes

[qcolombet]

Yay!

llvm/docs/ReleaseNotes.rst

 
 Changes to the CodeGen infrastructure
 -------------------------------------
 
 * ``PrologEpilogInserter`` no longer supports register scavenging
   during forwards frame index elimination. Targets should use
   backwards frame index elimination instead.
 
+* ``RegScavenger`` no longer supports forwards register
+  scavenging. Clients should use backwards register scavenging
+  instead, which is preferred because it does not depend on accurate
+  kill flags.
+
 Changes to the Metadata Info
 ---------------------------------
 
 Changes to the Debug Info
 ---------------------------------
 
 Changes to the LLVM tools
 ---------------------------------

llvm/include/llvm/CodeGen/RegisterScavenging.h

 class TargetRegisterInfo;
 
 class RegScavenger {
   const TargetRegisterInfo *TRI = nullptr;
   const TargetInstrInfo *TII = nullptr;
   MachineRegisterInfo *MRI = nullptr;
   MachineBasicBlock *MBB = nullptr;
   MachineBasicBlock::iterator MBBI;
-  unsigned NumRegUnits = 0;
 
   /// True if RegScavenger is currently tracking the liveness of registers.
   bool Tracking = false;
 
   /// Information on scavenged registers (held in a spill slot).
   struct ScavengedInfo {
     ScavengedInfo(int FI = -1) : FrameIndex(FI) {}
 
     /// A spill slot used for scavenging a register post register allocation.
     int FrameIndex;
 
     /// If non-zero, the specific register is currently being
     /// scavenged. That is, it is spilled to this scavenging stack slot.
     Register Reg;
 
     /// The instruction that restores the scavenged register from stack.
     const MachineInstr *Restore = nullptr;
   };
 
   /// A vector of information on scavenged registers.
   SmallVector<ScavengedInfo, 2> Scavenged;
 
   LiveRegUnits LiveUnits;
 
-  // These BitVectors are only used internally to forward(). They are members
-  // to avoid frequent reallocations.
-  BitVector KillRegUnits, DefRegUnits;
-  BitVector TmpRegUnits;
-
 public:
   RegScavenger() = default;
 
   /// Record that \p Reg is in use at scavenging index \p FI. This is for
   /// targets which need to directly manage the spilling process, and need to
   /// update the scavenger's internal state.  It's expected this be called a
   /// second time with \p Restore set to a non-null value, so that the
   /// externally inserted restore instruction resets the scavenged slot
void assignRegToScavengingIndex(int FI, Register Reg,
 
     llvm_unreachable("did not find scavenging index");
   }
 
   /// Start tracking liveness from the begin of basic block \p MBB.
   void enterBasicBlock(MachineBasicBlock &MBB);
 
   /// Start tracking liveness from the end of basic block \p MBB.
-  /// Use backward() to move towards the beginning of the block. This is
-  /// preferred to enterBasicBlock() and forward() because it does not depend
-  /// on the presence of kill flags.
+  /// Use backward() to move towards the beginning of the block.
   void enterBasicBlockEnd(MachineBasicBlock &MBB);
 
-  /// Move the internal MBB iterator and update register states.
-  void forward();
-
-  /// Move the internal MBB iterator and update register states until
-  /// it has processed the specific iterator.
-  void forward(MachineBasicBlock::iterator I) {
-    while (!Tracking || MBBI != I)
-      forward();
-  }
-
-  /// Update internal register state and move MBB iterator backwards.
-  /// Contrary to unprocess() this method gives precise results even in the
-  /// absence of kill flags.
+  /// Update internal register state and move MBB iterator backwards. This
+  /// method gives precise results even in the absence of kill flags.
   void backward();
 
   /// Call backward() as long as the internal iterator does not point to \p I.
   void backward(MachineBasicBlock::iterator I) {
     while (MBBI != I)
       backward();
   }
 
public:
 
   /// Tell the scavenger a register is used.
   void setRegUsed(Register Reg, LaneBitmask LaneMask = LaneBitmask::getAll());
 
 private:
   /// Returns true if a register is reserved. It is never "unused".
   bool isReserved(Register Reg) const { return MRI->isReserved(Reg); }
 
-  /// setUsed / setUnused - Mark the state of one or a number of register units.
-  ///
-  void setUsed(const BitVector &RegUnits) {
-    LiveUnits.addUnits(RegUnits);
-  }
-  void setUnused(const BitVector &RegUnits) {
-    LiveUnits.removeUnits(RegUnits);
-  }
-
-  /// Processes the current instruction and fill the KillRegUnits and
-  /// DefRegUnits bit vectors.
-  void determineKillsAndDefs();
-
-  /// Add all Reg Units that Reg contains to BV.
-  void addRegUnits(BitVector &BV, MCRegister Reg);
-
-  /// Remove all Reg Units that \p Reg contains from \p BV.
-  void removeRegUnits(BitVector &BV, MCRegister Reg);
-
   /// Initialize RegisterScavenger.
   void init(MachineBasicBlock &MBB);
 
   /// Spill a register after position \p After and reload it before position
   /// \p UseMI.
   ScavengedInfo &spill(Register Reg, const TargetRegisterClass &RC, int SPAdj,
                        MachineBasicBlock::iterator Before,
                        MachineBasicBlock::iterator &UseMI);

llvm/lib/CodeGen/RegisterScavenging.cpp

 
 void RegScavenger::init(MachineBasicBlock &MBB) {
   MachineFunction &MF = *MBB.getParent();
   TII = MF.getSubtarget().getInstrInfo();
   TRI = MF.getSubtarget().getRegisterInfo();
   MRI = &MF.getRegInfo();
   LiveUnits.init(*TRI);
 
-  assert((NumRegUnits == 0 || NumRegUnits == TRI->getNumRegUnits()) &&
-         "Target changed?");
-
-  // Self-initialize.
-  if (!this->MBB) {
-    NumRegUnits = TRI->getNumRegUnits();
-    KillRegUnits.resize(NumRegUnits);
-    DefRegUnits.resize(NumRegUnits);
-    TmpRegUnits.resize(NumRegUnits);
-  }
   this->MBB = &MBB;
 
   for (ScavengedInfo &SI : Scavenged) {
     SI.Reg = 0;
     SI.Restore = nullptr;
   }
 
   Tracking = false;
void RegScavenger::enterBasicBlockEnd(MachineBasicBlock &MBB) {
 
   // Move internal iterator at the last instruction of the block.
   if (!MBB.empty()) {
     MBBI = std::prev(MBB.end());
     Tracking = true;
   }
 }
 
-void RegScavenger::addRegUnits(BitVector &BV, MCRegister Reg) {
-  for (MCRegUnit Unit : TRI->regunits(Reg))
-    BV.set(Unit);
-}
-
-void RegScavenger::removeRegUnits(BitVector &BV, MCRegister Reg) {
-  for (MCRegUnit Unit : TRI->regunits(Reg))
-    BV.reset(Unit);
-}
-
-void RegScavenger::determineKillsAndDefs() {
-  assert(Tracking && "Must be tracking to determine kills and defs");
-
-  MachineInstr &MI = *MBBI;
-  assert(!MI.isDebugInstr() && "Debug values have no kills or defs");
-
-  // Find out which registers are early clobbered, killed, defined, and marked
-  // def-dead in this instruction.
-  KillRegUnits.reset();
-  DefRegUnits.reset();
-  for (const MachineOperand &MO : MI.operands()) {
-    if (MO.isRegMask()) {
-      TmpRegUnits.reset();
-      for (unsigned RU = 0, RUEnd = TRI->getNumRegUnits(); RU != RUEnd; ++RU) {
-        for (MCRegUnitRootIterator RURI(RU, TRI); RURI.isValid(); ++RURI) {
-          if (MO.clobbersPhysReg(*RURI)) {
-            TmpRegUnits.set(RU);
-            break;
-          }
-        }
-      }
-
-      // Apply the mask.
-      KillRegUnits |= TmpRegUnits;
-    }
-    if (!MO.isReg())
-      continue;
-    if (!MO.getReg().isPhysical() || isReserved(MO.getReg()))
-      continue;
-    MCRegister Reg = MO.getReg().asMCReg();
-
-    if (MO.isUse()) {
-      // Ignore undef uses.
-      if (MO.isUndef())
-        continue;
-      if (MO.isKill())
-        addRegUnits(KillRegUnits, Reg);
-    } else {
-      assert(MO.isDef());
-      if (MO.isDead())
-        addRegUnits(KillRegUnits, Reg);
-      else
-        addRegUnits(DefRegUnits, Reg);
-    }
-  }
-}
-
-void RegScavenger::forward() {
-  // Move ptr forward.
-  if (!Tracking) {
-    MBBI = MBB->begin();
-    Tracking = true;
-  } else {
-    assert(MBBI != MBB->end() && "Already past the end of the basic block!");
-    MBBI = std::next(MBBI);
-  }
-  assert(MBBI != MBB->end() && "Already at the end of the basic block!");
-
-  MachineInstr &MI = *MBBI;
-
-  for (ScavengedInfo &I : Scavenged) {
-    if (I.Restore != &MI)
-      continue;
-
-    I.Reg = 0;
-    I.Restore = nullptr;
-  }
-
-  if (MI.isDebugOrPseudoInstr())
-    return;
-
-  determineKillsAndDefs();
-
-  // Verify uses and defs.
-#ifndef NDEBUG
-  for (const MachineOperand &MO : MI.operands()) {
-    if (!MO.isReg())
-      continue;
-    Register Reg = MO.getReg();
-    if (!Reg.isPhysical() || isReserved(Reg))
-      continue;
-    if (MO.isUse()) {
-      if (MO.isUndef())
-        continue;
-      if (!isRegUsed(Reg)) {
-        // Check if it's partial live: e.g.
-        // D0 = insert_subreg undef D0, S0
-        // ... D0
-        // The problem is the insert_subreg could be eliminated. The use of
-        // D0 is using a partially undef value. This is not *incorrect* since
-        // S1 is can be freely clobbered.
-        // Ideally we would like a way to model this, but leaving the
-        // insert_subreg around causes both correctness and performance issues.
-        if (none_of(TRI->subregs(Reg),
-                    [&](MCPhysReg SR) { return isRegUsed(SR); }) &&
-            none_of(TRI->superregs(Reg),
-                    [&](MCPhysReg SR) { return isRegUsed(SR); })) {
-          MBB->getParent()->verify(nullptr, "In Register Scavenger");
-          llvm_unreachable("Using an undefined register!");
-        }
-      }
-    } else {
-      assert(MO.isDef());
-#if 0
-      // FIXME: Enable this once we've figured out how to correctly transfer
-      // implicit kills during codegen passes like the coalescer.
-      assert((KillRegs.test(Reg) || isUnused(Reg) ||
-              isLiveInButUnusedBefore(Reg, MI, MBB, TRI, MRI)) &&
-             "Re-defining a live register!");
-#endif
-    }
-  }
-#endif // NDEBUG
-
-  // Commit the changes.
-  setUnused(KillRegUnits);
-  setUsed(DefRegUnits);
-}
-
 void RegScavenger::backward() {
   assert(Tracking && "Must be tracking to determine kills and defs");
 
   const MachineInstr &MI = *MBBI;
   LiveUnits.stepBackward(MI);
 
   // Expire scavenge spill frameindex uses.
   for (ScavengedInfo &I : Scavenged) {