[PATCH] D25347: [VirtRegRewriter] Eliminate COPYs before re-writing by renaming.

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 7 10:24:32 PDT 2016


Hi Geoff,

> On Oct 7, 2016, at 8:36 AM, Geoff Berry <gberry at codeaurora.org> wrote:
> 
> gberry added a comment.
> 
> In https://reviews.llvm.org/D25347#563996, @qcolombet wrote:
> 
>> Hi Geoff,
>> 
>> Disclaimer: I haven't looked at the patch, just a comment.
>> I would rather fix the MachineCopyPropagation pass than adding this logic to this pass.
>> 
>> Why aren't we catching those cases in the MachineCopyPropagation pass?
> 
> 
> Hi Quentin,
> 
> I believe this was discussed before here: https://reviews.llvm.org/D20531.  In this change, the COPYs only become removable as the result of renaming/recoloring their destination registers.

I see, thanks for checking.

>  I thought the consensus was that this recoloring was only safe to do with virtual registers to avoid violating ABI/other register constraints.

That is correct.

>  The COPYs that this change catches are mostly only removable after register allocation since they are only removable once live range splitting/spilling has occurred.

In that case, we would refactor the code so that both the MachineCopyPropagation and this addition uses common code. Again I haven’t looked at the patch itself, but I am guessing a lot of the core logic to look similar.

Let me know if I am mistaken.

Cheers,
-Quentin

> 
> In https://reviews.llvm.org/D25347#563996, @qcolombet wrote:
> 
>> Hi Geoff,
>> 
>> Disclaimer: I haven't looked at the patch, just a comment.
>> I would rather fix the MachineCopyPropagation pass than adding this logic to this pass.
>> 
>> Why aren't we catching those cases in the MachineCopyPropagation pass?
>> 
>> Cheers,
>> -Quentin
> 
> 
> 
> 
> 
> https://reviews.llvm.org/D25347
> 
> 
> 



More information about the llvm-commits mailing list