New Iteration of subregister liveness patches

Matthias Braun matze at braunis.de
Mon Dec 15 13:19:24 PST 2014


> On Dec 9, 2014, at 6:05 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
> 
> On Dec 9, 2014, at 5:32 PM, Matthias Braun <matze at braunis.de> wrote:
> 
>> Ok,
>> 
>> I just commited the changes to svn trunk. Thanks everyone, esp. Renato and Quentin, for the detailed reviews!
>> 
>> 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.
>> 
>> For the record: I did fix many but not all issues mentioned in the reviews. The following is still missing:
>> 
>> - Manually creating a single linked list in the LiveInterval class. The alternatives to this are:
>>   a) Using the double linked list provided by llvm.
>>   b) Turning the manual single linked list into a full featued ADT in llvm.
>> 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.
> 
> Fair enough.
> 
>> - 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.
> 
> Please file a PR for that.
> 
>> - 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
> 
> SGTM.
r224272 fixes these particular issues in a nicer way by rewriting the code involved to calculate subrange liveness sequentially instead of in parallel avoiding the need for the LiveOutData parameters altogether. This also allowed to reduce code duplication at these places.

Greetings
    Matthias



More information about the llvm-commits mailing list