[PATCH] D12588: PeepholeOptimizer: Look through simple extracts of reg_sequence

[Quentin Colombet via llvm-commits]

> On Sep 3, 2015, at 4:27 PM, Matt Arsenault <Matthew.Arsenault at amd.com> wrote:
> 
> arsenm added inline comments.
> 
> ================
> Comment at: lib/CodeGen/PeepholeOptimizer.cpp:1252
> @@ +1251,3 @@
> +  //
> +  //  => vreg3 = COPY vreg0
> +  //
> ----------------
> qcolombet wrote:
>> The design of the copy rewriter is to find alternative sources within the value tracker.
>> This change does not fit that scheme and looks like an ad hoc solution.
>> I believe what it is missing is to teach the value tracker about sub register.
>> Could you look into that direction or explain why do you think we should do it like you propose in this patch?
> This all seemed like it was solving a different problem at first.
> 
> This already does seem to track sub registers correctly, it just stops the search for a better source too early.
> 
> If in findNextSource I change
>      ShouldRewrite = shareSameRegisterFile(*TRI, DefRC, SubReg, SrcRC,
> 
> to only do this if the subregister index is the same, it fixes my cases. This however breaks a few AArch64 tests. The search stopping condition needs to change, but I'm not sure exactly what it should be. It's true in this case that shareSameRegisterFile, but we also want to reduce the register size by looking through the reg_sequence. 

Ok, I see what you want to do.

The thing is this peephole optimization is supposed to remove cross register bank copies, not reducing the width of registers.
The rational is that the register coalescer is supposed to do the right thing with non-cross-register-file copies…
Apparently this is not the case for you :).

That being said, we could reuse indeed the same logic for that and instead just have a different predicate (i.e., something different than shareSameRegisterFile). Maybe we can provide a target hook to tell the peephole optimizer what predicate it should satisfy. That should be pretty easy.

Could you look into that direction?

As an aside, does it really matter to have a smaller register when sub-register liveness is enabled?
I thought this would fix this kind of problems.

Cheers,
-Quentin

> 
> 
> https://urldefense.proofpoint.com/v2/url?u=http-3A__reviews.llvm.org_D12588&d=BQIFaQ&c=eEvniauFctOgLOKGJOplqw&r=Kar_gr4lUTX9iRgFLHyIPCR7VD3VlB3-02h_Q5v9oWk&m=q32rYatajb6G22gcqZno_GtECCuqPDZbBuI51KB-774&s=P2u2zOZr2HmwehluSho3dwSyO7mP1iGWW_l9m6hGxjE&e= 
> 
> 
>