[PATCH 1/5] refactor LiveInterval: introduce new LiveRanges class

[Quentin Colombet]

Hi Matthias,

I have a couple of minor comments:
1. Formatting changes do not help reviewing the patch, things like:
-    const_vni_iterator vni_end() const { return valnos.end(); }
+    const_vni_iterator vni_end() const   { return valnos.end(); }
Are necessary.
Note that I do not know the LLVM policy for this kind of thing, but I am not fond of additional space stuff to align block.

2. Because of the refactoring, the LiveRange class does not have the register field anymore, thus you remove the reference in the print method of the LiveRangeUpdater:
-      OS << "Clean " << PrintReg(LI->reg) << " updater: " << *LI << '\n';
[…]
+      OS << "Clean updater: " << *LR << '\n';

This is fine of course.
However, I think you should also modify LiveInterval::print to print the register to ease debug purpose because now, we have less information (unless I missed something).

Thanks,
-Quentin

On Aug 29, 2013, at 6:07 PM, Matthias Braun <matze at braunis.de> wrote:

> 
> LiveRanges has logic to manage a set list LiveRange instances
> and a list of value numbers which previously was in LiveInterval,
> without having details like spill weight or a fixed register number.
> LiveInterval is now a direct subclass of LiveRanges and simply adds the
> spill weight and the register number.
> ---
> include/llvm/CodeGen/LiveInterval.h  | 143 +++++++++++++++++++++--------------
> lib/CodeGen/LiveInterval.cpp         | 128 +++++++++++++++----------------
> lib/CodeGen/LiveIntervalAnalysis.cpp |  20 ++---
> 3 files changed, 156 insertions(+), 135 deletions(-)
> 
> <0001-refactor-LiveInterval-introduce-new-LiveRanges-class.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130830/0285c8e1/attachment.html>