Page MenuHomePhabricator

LiveIntervalAnalysis: Cleanup handleMove{Down|Up}() functions, NFC
ClosedPublic

Authored by MatzeB on Jan 20 2016, 4:07 PM.

Details

Summary

These two functions are hard to reason about. This commit makes
the code more comprehensible:

  • Use four distinct variables (OldIdxIn, OldIdxOut, NewIdxIn, NewIdxOut) with a fixed value instead of a changing iterator I that points to different things during the function.
  • Remove the early explanation before the function in favor of more detailed comments inside the function. Should have more/clearer comments now stating which conditions are tested and which invariants hold at different points in the functions.

The behaviour of the code was not changed.

I hope that this will make it easier to review the changes in
http://reviews.llvm.org/D9067 which I will adapt next.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 45459.Jan 20 2016, 4:07 PM
MatzeB retitled this revision from to LiveIntervalAnalysis: Cleanup handleMove{Down|Up}() functions, NFC.
MatzeB updated this object.
MatzeB added reviewers: atrick, qcolombet, stoklund.
MatzeB set the repository for this revision to rL LLVM.
MatzeB added a subscriber: llvm-commits.

Would it make sense for Lang to review this?

MatzeB updated this revision to Diff 45751.Jan 22 2016, 2:42 PM
MatzeB edited edge metadata.

Some more non function changes in order to minimze the changes in the upcoming patch.

lhames accepted this revision.Jan 22 2016, 3:35 PM
lhames edited edge metadata.

Looks like a good cleanup to me. Thanks Matthias.

This revision is now accepted and ready to land.Jan 22 2016, 3:35 PM

Thanks for the review

This revision was automatically updated to reflect the committed changes.