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

[Evan Cheng]

LGTM then.

Evan

On Oct 4, 2013, at 4:05 PM, Quentin Colombet <qcolombet at apple.com> wrote:

> On Oct 4, 2013, at 12:59 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> 
>> Good point.
>> 
>> Let me gather some numbers.
>> 
>> -Quentin
>> 
>> On Oct 4, 2013, at 12:50 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>> 
>>> The patch looks fine to me. Is there any measurable compile time impact?
> No, nothing measurable.
> 
> I benchmarked the compiler with and without the patch in release mode on the whole llvm suite + SPEC and did not see any measurable impact.
> 
> -Quentin
> 
>>> 
>>> Evan
>>> 
>>> On Sep 9, 2013, at 9:20 AM, Quentin Colombet <qcolombet at apple.com> wrote:
>>> 
>>>> Ping^2?
>>>> 
>>>> -Quentin
>>>> 
>>>> On Sep 3, 2013, at 9:41 AM, Quentin Colombet <qcolombet at apple.com> wrote:
>>>> 
>>>>> Ping?
>>>>> -Quentin
>>>>> 
>>>>> On Aug 28, 2013, at 5:29 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>>>>> 
>>>>>> Hi Evan,
>>>>>> 
>>>>>> Eric suggested that you may be the right person to review this patch.
>>>>>> Could you have a look, please?
>>>>>> 
>>>>>> This fixes <rdar://problem/14742333>.
>>>>>> 
>>>>>> Thanks,
>>>>>> -Quentin
>>>>>> <PeepholeCopy.svndiff>
>>>>>> 
>>>>>> On Aug 28, 2013, at 5:25 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>>>>>> 
>>>>>>> 
>>>>>>> On Aug 28, 2013, at 4:43 PM, Eric Christopher <echristo at gmail.com> wrote:
>>>>>>> 
>>>>>>>>> 
>>>>>>>>> The patch is not triggered by any of the tests that are in the llvm test
>>>>>>>>> suite (tested on both ARM and X86).
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> *nod* I figured, otherwise you'd have said it. :)
>>>>>>>> 
>>>>>>>>> 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.
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> *nod*
>>>>>>>> 
>>>>>>>>> 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.
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> Pretty nice. Can you give a general idea of the type of code that this
>>>>>>>> is triggering on?
>>>>>>> Basically it was some code using a tone of intrinsics, in particular mixing vector, integer, and floating point code.
>>>>>>> For isel, this means, tone of bitcast, vector insert, and vector extract, across different basic blocks.
>>>>>>> 
>>>>>>>> 
>>>>>>>>> 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?
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> Helps. I was hoping for more code details, but the comment provides
>>>>>>>> some helpful bit.
>>>>>>> Do you think I should put more comments?
>>>>>>> 
>>>>>>>> 
>>>>>>>>> I have moved the assignment inside the loop.
>>>>>>>>> Is it better?
>>>>>>>>> (I do like predicated code!)
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> Looks better to me from a readability perspective.
>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 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!)
>>>>>>>>> 
>>>>>>>> 
>>>>>>>> Ultimately Jakob or Evan. It looks pretty simple so unless you get an
>>>>>>>> objection by tomorrow morning I guess submit?
>>>>>>> I will do that.
>>>>>>> 
>>>>>>> Thanks,
>>>>>>> Quentin
>>>>>>>> 
>>>>>>>> -eric
>>>>>>>> 
>>>>>>>>> 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
>>>>>> 
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>> 
>>>>> _______________________________________________
>>>>> llvm-commits mailing list
>>>>> llvm-commits at cs.uiuc.edu
>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>> 
>>> 
>> 
>> _______________________________________________
>> 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/20131007/13498d47/attachment.html>