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

Andrew Trick via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 11 23:42:57 PDT 2015

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 @@
+    LiveInRegs.clear();
I think this should be LiveOutRegs.clear(), rather than preserving the old incorrect behavior.



More information about the llvm-commits mailing list