This is an archive of the discontinued LLVM Phabricator instance.

RegisterPressure: Move LiveInRegs/LiveOutRegs from RegisterPressure to PressureTracker
ClosedPublic

Authored by MatzeB on Sep 11 2015, 1:47 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 34581.Sep 11 2015, 1:47 PM
MatzeB retitled this revision from to RegisterPressure: Move LiveInRegs/LiveOutRegs from RegisterPressure to PressureTracker.
MatzeB updated this object.
MatzeB added a reviewer: atrick.
MatzeB set the repository for this revision to rL LLVM.
MatzeB added a subscriber: llvm-commits.
atrick requested changes to this revision.Sep 11 2015, 11:42 PM
atrick edited edge metadata.

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

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

This revision now requires changes to proceed.Sep 11 2015, 11:42 PM
This revision was automatically updated to reflect the committed changes.

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