[RegScavenger] Remove support for forwards register scavenging

Differential Revision: https://reviews.llvm.org/D156987

Author: jayfoad

llvm/docs/ReleaseNotes.rst

@@ -127,6 +127,11 @@ Changes to the CodeGen infrastructure
   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
 ---------------------------------
 

llvm/include/llvm/CodeGen/RegisterScavenging.h

@@ -37,7 +37,6 @@ class RegScavenger {
   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;
@@ -62,11 +61,6 @@ class RegScavenger {
 
   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;
 
@@ -94,24 +88,11 @@ class RegScavenger {
   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.
@@ -181,25 +162,6 @@ class RegScavenger {
   /// 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);
 

llvm/lib/CodeGen/RegisterScavenging.cpp

@@ -59,16 +59,6 @@ void RegScavenger::init(MachineBasicBlock &MBB) {
   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) {
@@ -95,135 +85,6 @@ void RegScavenger::enterBasicBlockEnd(MachineBasicBlock &MBB) {
   }
 }
 
-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");