RegisterPressure: Move LiveInRegs/LiveOutRegs from RegisterPressure to PressureTracker

[MatzeB]

Summary

This is part of a series to get rid of the RegisterPressure class.

[atrick]

tl;dr LGTM

In general, it is important to separate result state from analysis state, which is why RegisterPressure and RegisterPressureTracker should have separate classes. Their lifetimes should not be tied together. The RegisterPressure interfaces were designed to support a variety of schedulers and expected to be used in various MI passes. However, this code has been around a while, and it still isn't used outside the one main scheduler driver, ScheduleDAGMILive. Since the current incarnation of the scheduler is stable now and does not need this modularity, I'd say that it is over-engineered. I'm fine consolidating things in the interest of readability.

So, just address the one possible correctness issue.

lib/CodeGen/RegisterPressure.cpp

573 ↗

(On Diff #34581)

I think this should be LiveOutRegs.clear(), rather than preserving the old incorrect behavior.

[MatzeB]

Sorry this was an accidental commit which I reverted now. The code review was of course not finished yet.

llvm/trunk/include/llvm/CodeGen/RegisterPressure.h

 class RegisterClassInfo;
 class MachineInstr;
 
 /// Base class for register pressure results.
 struct RegisterPressure {
   /// Map of max reg pressure indexed by pressure set ID, not class ID.
   std::vector<unsigned> MaxSetPressure;
 
-  /// List of live in virtual registers or physical register units.
-  SmallVector<unsigned,8> LiveInRegs;
-  SmallVector<unsigned,8> LiveOutRegs;
-
   void dump(const TargetRegisterInfo *TRI) const;
 };
 
 /// RegisterPressure computed within a region of instructions delimited by
 /// TopIdx and BottomIdx.  During pressure computation, the maximum pressure per
 /// register pressure set is increased. Once pressure within a region is fully
 /// computed, the live-in and live-out sets are recorded.
 ///
class RegPressureTracker {
   /// Pressure map indexed by pressure set ID, not class ID.
   std::vector<unsigned> CurrSetPressure;
 
   /// Set of live registers.
   LiveRegSet LiveRegs;
 
   /// Set of vreg defs that start a live range.
   SparseSet<unsigned, VirtReg2IndexFunctor> UntiedDefs;
+  /// List of live in virtual registers or physical register units.
+  SmallVector<unsigned,8> LiveInRegs;
+  /// List of live out virtual registers or physical register units.
+  SmallVector<unsigned,8> LiveOutRegs;
   /// Live-through pressure.
   std::vector<unsigned> LiveThruPressure;
 
 public:
   RegPressureTracker(IntervalPressure &rp) :
     MF(nullptr), TRI(nullptr), RCI(nullptr), LIS(nullptr), MBB(nullptr), P(rp),
     RequireIntervals(true), TrackUntiedDefs(false) {}
 
public:
   /// RPTracker.
   void initLiveThru(const RegPressureTracker &RPTracker);
 
   /// Copy an existing live thru pressure result.
   void initLiveThru(ArrayRef<unsigned> PressureSet) {
     LiveThruPressure.assign(PressureSet.begin(), PressureSet.end());
   }
 
+  ArrayRef<unsigned> getLiveIn() const { return LiveInRegs; }
+  ArrayRef<unsigned> getLiveOut() const { return LiveOutRegs; }
   ArrayRef<unsigned> getLiveThru() const { return LiveThruPressure; }
 
   /// Get the resulting register pressure over the traversed region.
   /// This result is complete if either advance() or recede() has returned true,
   /// or if closeRegion() was explicitly invoked.
   RegisterPressure &getPressure() { return P; }
   const RegisterPressure &getPressure() const { return P; }
 

llvm/trunk/lib/CodeGen/MachineScheduler.cpp

void ScheduleDAGMILive::initRegPressure() {
   BotRPTracker.init(&MF, RegClassInfo, LIS, BB, LiveRegionEnd);
 
   // Close the RPTracker to finalize live ins.
   RPTracker.closeRegion();
 
   DEBUG(RPTracker.dump());
 
   // Initialize the live ins and live outs.
-  TopRPTracker.addLiveRegs(RPTracker.getPressure().LiveInRegs);
-  BotRPTracker.addLiveRegs(RPTracker.getPressure().LiveOutRegs);
+  TopRPTracker.addLiveRegs(RPTracker.getLiveIn());
+  BotRPTracker.addLiveRegs(RPTracker.getLiveOut());
 
   // Close one end of the tracker so we can call
   // getMaxUpward/DownwardPressureDelta before advancing across any
   // instructions. This converts currently live regs into live ins/outs.
   TopRPTracker.closeTop();
   BotRPTracker.closeBottom();
 
   BotRPTracker.initLiveThru(RPTracker);
   if (!BotRPTracker.getLiveThru().empty()) {
     TopRPTracker.initLiveThru(BotRPTracker.getLiveThru());
     DEBUG(dbgs() << "Live Thru: ";
           dumpRegSetPressure(BotRPTracker.getLiveThru(), TRI));
   };
 
   // For each live out vreg reduce the pressure change associated with other
   // uses of the same vreg below the live-out reaching def.
-  updatePressureDiffs(RPTracker.getPressure().LiveOutRegs);
+  updatePressureDiffs(RPTracker.getLiveOut());
 
   // Account for liveness generated by the region boundary.
   if (LiveRegionEnd != RegionEnd) {
     SmallVector<unsigned, 8> LiveUses;
     BotRPTracker.recede(&LiveUses);
     updatePressureDiffs(LiveUses);
   }
 

 /// assuming LiveIntervals.
 unsigned ScheduleDAGMILive::computeCyclicCriticalPath() {
   // This only applies to single block loop.
   if (!BB->isSuccessor(BB))
     return 0;
 
   unsigned MaxCyclicLatency = 0;
   // Visit each live out vreg def to find def/use pairs that cross iterations.
-  ArrayRef<unsigned> LiveOuts = RPTracker.getPressure().LiveOutRegs;
+  ArrayRef<unsigned> LiveOuts = RPTracker.getLiveOut();
   for (ArrayRef<unsigned>::iterator RI = LiveOuts.begin(), RE = LiveOuts.end();
        RI != RE; ++RI) {
     unsigned Reg = *RI;
     if (!TRI->isVirtualRegister(Reg))
         continue;
     const LiveInterval &LI = LIS->getInterval(Reg);
     const VNInfo *DefVNI = LI.getVNInfoBefore(LIS->getMBBEndIdx(BB));
     if (!DefVNI)

llvm/trunk/lib/CodeGen/RegisterPressure.cpp

void llvm::dumpRegSetPressure(ArrayRef<unsigned> SetPressure,
   if (Empty)
     dbgs() << "\n";
 }
 
 LLVM_DUMP_METHOD
 void RegisterPressure::dump(const TargetRegisterInfo *TRI) const {
   dbgs() << "Max Pressure: ";
   dumpRegSetPressure(MaxSetPressure, TRI);
-  dbgs() << "Live In: ";
-  for (unsigned i = 0, e = LiveInRegs.size(); i < e; ++i)
-    dbgs() << PrintVRegOrUnit(LiveInRegs[i], TRI) << " ";
-  dbgs() << '\n';
-  dbgs() << "Live Out: ";
-  for (unsigned i = 0, e = LiveOutRegs.size(); i < e; ++i)
-    dbgs() << PrintVRegOrUnit(LiveOutRegs[i], TRI) << " ";
-  dbgs() << '\n';
 }
 
 LLVM_DUMP_METHOD
 void RegPressureTracker::dump() const {
   if (!isTopClosed() || !isBottomClosed()) {
     dbgs() << "Curr Pressure: ";
     dumpRegSetPressure(CurrSetPressure, TRI);
   }
   P.dump(TRI);
+  dbgs() << "Live In: ";
+  for (unsigned i = 0, e = LiveInRegs.size(); i < e; ++i)
+    dbgs() << PrintVRegOrUnit(LiveInRegs[i], TRI) << " ";
+  dbgs() << '\n';
+  dbgs() << "Live Out: ";
+  for (unsigned i = 0, e = LiveOutRegs.size(); i < e; ++i)
+    dbgs() << PrintVRegOrUnit(LiveOutRegs[i], TRI) << " ";
+  dbgs() << '\n';
 }
 
 void PressureDiff::dump(const TargetRegisterInfo &TRI) const {
   for (const PressureChange &Change : *this) {
     if (!Change.isValid() || Change.getUnitInc() == 0)
       continue;
     dbgs() << "    " << TRI.getRegPressureSetName(Change.getPSet())
            << " " << Change.getUnitInc();
void RegPressureTracker::decreaseRegPressure(ArrayRef<unsigned> RegUnits) {
   for (unsigned I = 0, E = RegUnits.size(); I != E; ++I)
     decreaseSetPressure(CurrSetPressure, MRI->getPressureSets(RegUnits[I]));
 }
 
 /// Clear the result so it can be used for another round of pressure tracking.
 void IntervalPressure::reset() {
   TopIdx = BottomIdx = SlotIndex();
   MaxSetPressure.clear();
-  LiveInRegs.clear();
-  LiveOutRegs.clear();
 }
 
 /// Clear the result so it can be used for another round of pressure tracking.
 void RegionPressure::reset() {
   TopPos = BottomPos = MachineBasicBlock::const_iterator();
   MaxSetPressure.clear();
-  LiveInRegs.clear();
-  LiveOutRegs.clear();
 }
 
 /// If the current top is not less than or equal to the next index, open it.
 /// We happen to need the SlotIndex for the next top for pressure update.
 void IntervalPressure::openTop(SlotIndex NextTop) {
   if (TopIdx <= NextTop)
     return;
   TopIdx = SlotIndex();
-  LiveInRegs.clear();
 }
 
 /// If the current top is the previous instruction (before receding), open it.
 void RegionPressure::openTop(MachineBasicBlock::const_iterator PrevTop) {
   if (TopPos != PrevTop)
     return;
   TopPos = MachineBasicBlock::const_iterator();
-  LiveInRegs.clear();
 }
 
 /// If the current bottom is not greater than the previous index, open it.
 void IntervalPressure::openBottom(SlotIndex PrevBottom) {
   if (BottomIdx > PrevBottom)
     return;
   BottomIdx = SlotIndex();
-  LiveInRegs.clear();
 }
 
 /// If the current bottom is the previous instr (before advancing), open it.
 void RegionPressure::openBottom(MachineBasicBlock::const_iterator PrevBottom) {
   if (BottomPos != PrevBottom)
     return;
   BottomPos = MachineBasicBlock::const_iterator();
-  LiveInRegs.clear();
 }
 
 const LiveRange *RegPressureTracker::getLiveRange(unsigned Reg) const {
   if (TargetRegisterInfo::isVirtualRegister(Reg))
     return &LIS->getInterval(Reg);
   return LIS->getCachedRegUnit(Reg);
 }
 
 void RegPressureTracker::reset() {
   MBB = nullptr;
   LIS = nullptr;
 
   CurrSetPressure.clear();
   LiveThruPressure.clear();
   P.MaxSetPressure.clear();
 
   if (RequireIntervals)
     static_cast<IntervalPressure&>(P).reset();
   else
     static_cast<RegionPressure&>(P).reset();
+  LiveInRegs.clear();
+  LiveOutRegs.clear();
 
   LiveRegs.PhysRegs.clear();
   LiveRegs.VirtRegs.clear();
   UntiedDefs.clear();
 }
 
 /// Setup the RegPressureTracker.
 ///

 
 /// Set the boundary for the top of the region and summarize live ins.
 void RegPressureTracker::closeTop() {
   if (RequireIntervals)
     static_cast<IntervalPressure&>(P).TopIdx = getCurrSlot();
   else
     static_cast<RegionPressure&>(P).TopPos = CurrPos;
 
-  assert(P.LiveInRegs.empty() && "inconsistent max pressure result");
-  P.LiveInRegs.reserve(LiveRegs.PhysRegs.size() + LiveRegs.VirtRegs.size());
-  P.LiveInRegs.append(LiveRegs.PhysRegs.begin(), LiveRegs.PhysRegs.end());
-  P.LiveInRegs.append(LiveRegs.VirtRegs.begin(), LiveRegs.VirtRegs.end());
+  assert(LiveInRegs.empty() && "inconsistent max pressure result");
+  LiveInRegs.reserve(LiveRegs.PhysRegs.size() + LiveRegs.VirtRegs.size());
+  LiveInRegs.append(LiveRegs.PhysRegs.begin(), LiveRegs.PhysRegs.end());
+  LiveInRegs.append(LiveRegs.VirtRegs.begin(), LiveRegs.VirtRegs.end());
 }
 
 /// Set the boundary for the bottom of the region and summarize live outs.
 void RegPressureTracker::closeBottom() {
   if (RequireIntervals)
     static_cast<IntervalPressure&>(P).BottomIdx = getCurrSlot();
   else
     static_cast<RegionPressure&>(P).BottomPos = CurrPos;
 
-  assert(P.LiveOutRegs.empty() && "inconsistent max pressure result");
-  P.LiveOutRegs.reserve(LiveRegs.PhysRegs.size() + LiveRegs.VirtRegs.size());
-  P.LiveOutRegs.append(LiveRegs.PhysRegs.begin(), LiveRegs.PhysRegs.end());
-  P.LiveOutRegs.append(LiveRegs.VirtRegs.begin(), LiveRegs.VirtRegs.end());
+  assert(LiveOutRegs.empty() && "inconsistent max pressure result");
+  LiveOutRegs.reserve(LiveRegs.PhysRegs.size() + LiveRegs.VirtRegs.size());
+  LiveOutRegs.append(LiveRegs.PhysRegs.begin(), LiveRegs.PhysRegs.end());
+  LiveOutRegs.append(LiveRegs.VirtRegs.begin(), LiveRegs.VirtRegs.end());
 }
 
 /// Finalize the region boundaries and record live ins and live outs.
 void RegPressureTracker::closeRegion() {
   if (!isTopClosed() && !isBottomClosed()) {
     assert(LiveRegs.PhysRegs.empty() && LiveRegs.VirtRegs.empty() &&
            "no region boundary");
     return;
   }
   if (!isBottomClosed())
     closeBottom();
   else if (!isTopClosed())
     closeTop();
   // If both top and bottom are closed, do nothing.
 }
 
 /// The register tracker is unaware of global liveness so ignores normal
 /// live-thru ranges. However, two-address or coalesced chains can also lead
 /// to live ranges with no holes. Count these to inform heuristics that we
 /// can never drop below this pressure.
 void RegPressureTracker::initLiveThru(const RegPressureTracker &RPTracker) {
   LiveThruPressure.assign(TRI->getNumRegPressureSets(), 0);
   assert(isBottomClosed() && "need bottom-up tracking to intialize.");
-  for (unsigned i = 0, e = P.LiveOutRegs.size(); i < e; ++i) {
-    unsigned Reg = P.LiveOutRegs[i];
+  for (unsigned i = 0, e = LiveOutRegs.size(); i < e; ++i) {
+    unsigned Reg = LiveOutRegs[i];
     if (TargetRegisterInfo::isVirtualRegister(Reg)
         && !RPTracker.hasUntiedDef(Reg)) {
       increaseSetPressure(LiveThruPressure, MRI->getPressureSets(Reg));
     }
   }
 }
 
 /// \brief Convenient wrapper for checking membership in RegisterOperands.
for (unsigned i = 0, e = Regs.size(); i != e; ++i) {
     if (LiveRegs.insert(Regs[i]))
       increaseRegPressure(Regs[i]);
   }
 }
 
 /// Add Reg to the live in set and increase max pressure.
 void RegPressureTracker::discoverLiveIn(unsigned Reg) {
   assert(!LiveRegs.contains(Reg) && "avoid bumping max pressure twice");
-  if (containsReg(P.LiveInRegs, Reg))
+  if (containsReg(LiveInRegs, Reg))
     return;
 
   // At live in discovery, unconditionally increase the high water mark.
-  P.LiveInRegs.push_back(Reg);
+  LiveInRegs.push_back(Reg);
   increaseSetPressure(P.MaxSetPressure, MRI->getPressureSets(Reg));
 }
 
 /// Add Reg to the live out set and increase max pressure.
 void RegPressureTracker::discoverLiveOut(unsigned Reg) {
   assert(!LiveRegs.contains(Reg) && "avoid bumping max pressure twice");
-  if (containsReg(P.LiveOutRegs, Reg))
+  if (containsReg(LiveOutRegs, Reg))
     return;
 
   // At live out discovery, unconditionally increase the high water mark.
-  P.LiveOutRegs.push_back(Reg);
+  LiveOutRegs.push_back(Reg);
   increaseSetPressure(P.MaxSetPressure, MRI->getPressureSets(Reg));
 }
 
 /// Recede across the previous instruction. If LiveUses is provided, record any
 /// RegUnits that are made live by the current instruction's uses. This includes
 /// registers that are both defined and used by the instruction.  If a pressure
 /// difference pointer is provided record the changes is pressure caused by this
 /// instruction independent of liveness.
if (CurrPos->isDebugValue()) {
     closeRegion();
     return false;
   }
   SlotIndex SlotIdx;
   if (RequireIntervals)
     SlotIdx = LIS->getInstructionIndex(CurrPos).getRegSlot();
 
   // Open the top of the region using slot indexes.
-  if (RequireIntervals && isTopClosed())
+  if (isTopClosed()) {
+    if (RequireIntervals)
       static_cast<IntervalPressure&>(P).openTop(SlotIdx);
+    LiveInRegs.clear();
+  }
 
   RegisterOperands RegOpers(TRI, MRI);
   collectOperands(CurrPos, RegOpers);
 
   if (PDiff)
     collectPDiff(*PDiff, RegOpers, MRI);
 
   // Boost pressure for all dead defs together.
if (RequireIntervals)
     SlotIdx = getCurrSlot();
 
   // Open the bottom of the region using slot indexes.
   if (isBottomClosed()) {
     if (RequireIntervals)
       static_cast<IntervalPressure&>(P).openBottom(SlotIdx);
     else
       static_cast<RegionPressure&>(P).openBottom(CurrPos);
+    LiveOutRegs.clear();
   }
 
   RegisterOperands RegOpers(TRI, MRI);
   collectOperands(CurrPos, RegOpers);
 
   for (unsigned i = 0, e = RegOpers.Uses.size(); i < e; ++i) {
     unsigned Reg = RegOpers.Uses[i];
     // Discover live-ins.