<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">Hi Matthias,<div><br></div><div>I have a couple of minor comments:</div><div>1. Formatting changes do not help reviewing the patch, things like:</div><div><div>- const_vni_iterator vni_end() const { return valnos.end(); }</div><div>+ const_vni_iterator vni_end() const { return valnos.end(); }</div></div><div>Are necessary.</div><div>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.</div><div><br></div><div>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:</div><div><div>- OS << "Clean " << PrintReg(LI->reg) << " updater: " << *LI << '\n';</div><div>[…]</div><div>+ OS << "Clean updater: " << *LR << '\n';</div></div><div><br></div><div>This is fine of course.</div><div>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).</div><div><br></div><div>Thanks,</div><div><div apple-content-edited="true">
<div style="color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">-Quentin</div>
</div>
<br><div><div>On Aug 29, 2013, at 6:07 PM, Matthias Braun <<a href="mailto:matze@braunis.de">matze@braunis.de</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><br>LiveRanges has logic to manage a set list LiveRange instances<br>and a list of value numbers which previously was in LiveInterval,<br>without having details like spill weight or a fixed register number.<br>LiveInterval is now a direct subclass of LiveRanges and simply adds the<br>spill weight and the register number.<br>---<br> include/llvm/CodeGen/LiveInterval.h | 143 +++++++++++++++++++++--------------<br> lib/CodeGen/LiveInterval.cpp | 128 +++++++++++++++----------------<br> lib/CodeGen/LiveIntervalAnalysis.cpp | 20 ++---<br> 3 files changed, 156 insertions(+), 135 deletions(-)<br><br><span><0001-refactor-LiveInterval-introduce-new-LiveRanges-class.patch></span>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote></div><br></div></body></html>