<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Dec 9, 2014 at 5:32 PM, Matthias Braun <span dir="ltr"><<a href="mailto:matze@braunis.de" target="_blank">matze@braunis.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Ok,<br>
<br>
I just commited the changes to svn trunk. Thanks everyone, esp. Renato and Quentin, for the detailed reviews!<br>
<br>
SubRegisterLiveness is available as a subtarget switch now, but not enabled by any of the targets yet, as most of them need some additional fixing/testcase changes which I'll address in subsequent commits.<br>
<br>
For the record: I did fix many but not all issues mentioned in the reviews. The following is still missing:<br>
<br>
- Manually creating a single linked list in the LiveInterval class. The alternatives to this are:<br>
    a) Using the double linked list provided by llvm.<br>
    b) Turning the manual single linked list into a full featued ADT in llvm.<br></blockquote><div><br>Why not std::forward_list ?<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  Using a double linked list where a single linked list is wasteful, so I'm against a). Turning the thing into a full-blown ADT template class seemed like too much for a single user in the whole llvm codebase, but if others would prefer that over the current code I will do that.<br>
- Sharing the LiveInterval::verify code with the MachineVerifier code. This is not as easy as it looks as you need a working MachineVerifier instance if you want to use the MachineVerifier elsewhere. Additionally that problem isn't introduced by my commits but already existed before... I may take a look into how to make the MachineVerifier more flexible to allow this, but this will be another patch series.<br>
- I clarified the comment in the updateLiveIns() function to explain the LiveOutData parameter, but did not add explanation to the other LiveOutData parameters yet. It seems like a better explanation at the point where the LiveOutData class is defined is the way to go instead of explaining the intention over and over again for each function having such a parameter. Anyway this is still TODO<br>
- The code duplication at places that match uses/defs to subranges and split the subranges on demand is still duplicated at several places in the register coalescer and I think even at other places. This is definitely TODO although not easy as all the appearances vary slightly.<br>
<br>
Greetings<br>
<span class="HOEnZb"><font color="#888888">    Matthias<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> On Dec 5, 2014, at 1:23 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br>
><br>
><br>
>> On Dec 5, 2014, at 1:20 PM, Renato Golin <<a href="mailto:renato.golin@linaro.org">renato.golin@linaro.org</a>> wrote:<br>
>><br>
>> On 5 December 2014 at 19:19, Tom Stellard <<a href="mailto:tom@stellard.net">tom@stellard.net</a>> wrote:<br>
>>> I tested these patches a few months ago and there were some regressions<br>
>>> on R600.  I'm not sure when I'll get a chance to test them again, but I<br>
>>> am in favor of merging these patches as soon as possible, since sub-reg<br>
>>> liveness is hidden behind a subtarget flag and it's easier to fully test<br>
>>> once it is in ToT.<br>
>><br>
>> If this is not the default behaviour, and it can be turned on by just<br>
>> adding a flag, then I don't see why not, after addressing Quentin's<br>
>> comments. I agree it'll be much easier to test this once it lands.<br>
><br>
> Same here. And I would add, my concerns can be addressed by subsequent patches.<br>
><br>
> Cheers,<br>
> Q.<br>
><br>
>><br>
>> cheers,<br>
>> --renato<br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>