[PATCH][Peephole] Rewrite copies to avoid cross register banks copies.

[Quentin Colombet]

Hi Eric,

Thanks for the review.
Comments inlined and new patch attached.


On Aug 28, 2013, at 2:54 PM, Eric Christopher <echristo at gmail.com> wrote:

> Interesting patch. It could definitely use some numbers.
The patch is not triggered by any of the tests that are in the llvm test suite (tested on both ARM and X86).

Like I said in my original email, I could rewrite/eliminate more aggressively copies (and in that case it does kick in in the llvm test suite) but then, the register allocator does in a few cases a poor coalescing/live-range splitting job.

That said, as it is, when the patch kicks in, we observed with our internal tests: no regressions and up to 25% runtime speed-up.

> Perhaps an
> example of what you're optimizing as a comment in the code?
I have added one in the file header. What do you think?

> 
> Silly nits:
> 
> couple (RegisterClass, SubReg)
> 
> maybe "pair”?
Done (and thanks ;)).

> 
> // if
> 
> Capitalize.
Done (and good catch :)).

> 
> A few more things:
> 
> +  } while (!ShouldRewrite && (Copy = MRI->getVRegDef(Src)) && Copy->isCopy());
> 
> I really dislike assignments in this sort of conditional.
I have moved the assignment inside the loop.
Is it better?
(I do like predicated code!)

> 
> I'm not necessarily comfortable approving, but the code looks reasonable.

What do you miss to be comfortable?
Could you point me someone that may be comfortable to approve this, please? (So that I can through him under the bus!)

Thanks again.

-Quentin
> 
> -eric
> 
> 
> On Fri, Aug 23, 2013 at 1:04 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>> Hi,
>> 
>> Here is a patch to rewrite copies to avoid cross register banks copies when possible.
>> Thanks for your reviews.
>> 
>> ** Context **
>> 
>> By definition copies across register bank are not coalescable. Still, it may be possible to get rid of such a copy when the value is available in another register of the same register file.
>> Consider the following example, where capital and lower letters denote different register file:
>> b = copy A <-- cross-bank copy
>> …
>> C = copy b <-- cross-bank copy
>> 
>> This could have been optimized this way:
>> b = copy A  <-- cross-bank copy
>> …
>> C = copy A <-- same-bank copy
>> 
>> Note: b and C's definitions may be in different basic blocks.
>> 
>> 
>> ** Proposed Solution **
>> 
>> Add a peephole optimization that looks through a chain of copies leading to a cross-bank copy and reuses a source that is on the same register file if available.
>> 
>> This solution could also be used to get rid of some copies (e.g., A could have been used instead of C). However, we do not do so because:
>> - It may over constrain the coloring of the source register for coalescing.
>> - The register allocator may not be able to find a nice split point for the longer live-range, leading to more spill.
>> 
>> 
>> ** Testcase?! **
>> 
>> This patch does not include a test case, because it is very difficult to reproduce this behavior with a reasonably small input.
>> 
>> Cheers,
>> -Quentin
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130828/91aa74a8/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: PeepholeCopy.svndiff
Type: application/octet-stream
Size: 6312 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130828/91aa74a8/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130828/91aa74a8/attachment-0001.html>