[PATCH] D35816: [Greedy RegAlloc] Add logic to greedy reg alloc to avoid bad eviction chains

[Quentin Colombet via llvm-commits]

Hi Marina,

> On Jul 27, 2017, at 5:16 AM, Marina Yatsina via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> myatsina added a comment.
> 
> In https://reviews.llvm.org/D35816#819501, @qcolombet wrote:
> 
>> Hi,
>> 
>> The greedy allocator is already very complicated and I am not sure the additional complexity of the eviction track is worth it.
>> Is it something that could be cleaned up in machine copy propagation? The problem is very local so that sounds doable.
>> 
>> I will have a closer look to the patch because fixing the problem from the start is obviously better that patching up later, but given how rare that problem is I really believe exploring other, less complex avenue is interesting.
>> 
>> Cheers,
>> Quentin
> 
> 
> Thank you for suggesting the machine copy propagation, I've started working on this direction, it definitely seems easier to implement it there.
> On the other hand, if I understood correctly, one of the issues with the old llvm register allocator (linear scan) was that that it did a lot of decisions that the rewriter had to clean up afterwards, and it was intended that greedy will try to avoid such decisions.

You’re right.

> I'm not sure if this eviction chain falls under this category or not.

Same here, but given how complex it seems to be to handle it here, I feel like this is not the right trade off, thus I suggested the machine copy propagation approach.

Thanks,
-Quentin

> 
> Thanks,
> Marina
> 
> 
> 
> ================
> Comment at: test/CodeGen/X86/bug26810.ll:1
> +; RUN: llc < %s -march=x86 -regalloc=greedy | FileCheck %s
> +; Make sure bad eviction sequence doesnt occur
> ----------------
> qcolombet wrote:
>> Could you use a .mir test to make the test more robust?
> Will do.
> 
> 
> ================
> Comment at: test/CodeGen/X86/bug26810.ll:3
> +; Make sure bad eviction sequence doesnt occur
> +; XFAIL: *
> +
> ----------------
> qcolombet wrote:
>> That sounds wrong for a new test.
>> 
>> Testing should be positive as much as possible IMO.
> I wasn't very satisfied with this check as well.
> I'll make it into a positive test indeed.
> 
> 
> Repository:
>  rL LLVM
> 
> https://reviews.llvm.org/D35816
> 
> 
>