[PATCH] SubRegister Liveness fixups

Matthias Braun matze at braunis.de
Wed Nov 19 16:46:57 PST 2014


> On Nov 19, 2014, at 4:01 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
> Hi Matthias,
> 
> Thanks for all the investigations you have done to track down all the regressions.
> 
> This is amazing!
> 
> For now, I would focus on having something correct but disabled by default. That way we could fix the performance problems we found more easily, like what you saw with the widening of moves logic.
> 
> To this end, I’ll focus into the review of the other thread, so that the ground work is in place and can be tested/enhanced by everybody.
> Anyhow, here are my comments for this set of patches.
> 
>> 0002-Do-not-unnecessarily-test-for-kill-flags.patch
> 
> Why is this required?
> X86 does not use the sub register liveness, is it?
Oh this ended up in the wrong patchset. I have another commit that enables subreg liveness tracking on X86. Together with this patch there are no regressions in the X86 testsuite anymore. I'll submit that another time together with the commit that enables x86 subregister liveness tracking.

> 
>> 003-ARM-Rework-widen-VMOVS-transformation.patch
> 
> This one is a refactoring of the previous copy widening mechanism. As the diffs in the updated test cases show, this does not seem to be equivalent to the old stuff. Still it seems to me that it could work in both modes (i.e., with and without sub register liveness), therefore it sounds like a good generalization.
> Could you start a different thread with this patch?
> That is fine if you think it is too early.
As that new version does not depend on implicit def operands being present anymore it triggers in a few extra cases, that's why the additional testcases are necessary. Should indeed be easy to push that patch independently of subregister liveness. I'll start a separate thread.

> 
>> 0004-Adapt-testcases-to-different-schedules-after-subreg-.patch
> 
> Could the changes be implied by a different ordering in the coalescing decisions?
My suspicion is that the missing superregister implicit def/use operands somehow end up in different tie-breaks in the scheduler, but I haven't really looked into the code yet to see if that is true.

- Matthias



More information about the llvm-commits mailing list