[PATCH] D12814: RegisterPressure: Move LiveInRegs/LiveOutRegs from RegisterPressure to PressureTracker

[Andrew Trick via llvm-commits]

atrick requested changes to this revision.
atrick added a comment.
This revision now requires changes to proceed.

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.


================
Comment at: lib/CodeGen/RegisterPressure.cpp:573
@@ -575,2 +572,3 @@
       static_cast<RegionPressure&>(P).openBottom(CurrPos);
+    LiveInRegs.clear();
   }
----------------
I think this should be LiveOutRegs.clear(), rather than preserving the old incorrect behavior.


Repository:
  rL LLVM

http://reviews.llvm.org/D12814