Patchwork RFA: patch to fix 2 testsuite failures for LRA on PPC

login
register
mail settings
Submitter Vladimir Makarov
Date Dec. 6, 2013, 7:02 p.m.
Message ID <52A21F61.5080308@redhat.com>
Download mbox | patch
Permalink /patch/298201/
State New
Headers show

Comments

Vladimir Makarov - Dec. 6, 2013, 7:02 p.m.
On 12/6/2013, 12:30 PM, Vladimir Makarov wrote:
> On 12/6/2013, 11:28 AM, Michael Meissner wrote:
>> On Thu, Dec 05, 2013 at 12:40:17PM -0500, Vladimir Makarov wrote:
>>> The following patch fixes two GCC testsuite failures for LRA.  The
>>> patch makes swap through registers instead of memory for the test
>>> cases when LRA is used.
>>>
>>> There are differences in reload and LRA constraint matching
>>> algorithm which results in different alternative choices when the
>>> original pattern is used.
>>>
>>> Actually my first proposed solution variant used one pattern which
>>> is now for LRA in this patch.  But some doubt arose that it may
>>> affect reload pass in some bad way.
>>>
>>> Ok to commit?
>>
>> I must admit to not remembering why I used ??&r.  I know I wanted it
>> to prefer
>> doing the memory patterns.  I would think we should try just the pattern
>> without the ??.
>>
>
>    I tried it about 2 months ago.  I did not see any problems of such
> change for reload and LRA.  There were no regressions on GCC testsuite.
>
>    So, Mike, if you don't see any compelling reason to keep ??, probably
> we should remove them.
>
> If you don't mind, I'll make the patch and test again and after that
> submit it for approval.
>

Here is the patch.

Tested and bootstrapped on gcc110.fsffrance.org.

Ok to commit?

2013-12-05  Vladimir Makarov  <vmakarov@redhat.com>

         * config/rs6000/rs600.md (*bswapdi2_64bit): Remove ?? from the
         constraint.
David Edelsohn - Dec. 6, 2013, 7:40 p.m.
On Fri, Dec 6, 2013 at 2:02 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
> On 12/6/2013, 12:30 PM, Vladimir Makarov wrote:
>>
>> On 12/6/2013, 11:28 AM, Michael Meissner wrote:
>>>
>>> On Thu, Dec 05, 2013 at 12:40:17PM -0500, Vladimir Makarov wrote:
>>>>
>>>> The following patch fixes two GCC testsuite failures for LRA.  The
>>>> patch makes swap through registers instead of memory for the test
>>>> cases when LRA is used.
>>>>
>>>> There are differences in reload and LRA constraint matching
>>>> algorithm which results in different alternative choices when the
>>>> original pattern is used.
>>>>
>>>> Actually my first proposed solution variant used one pattern which
>>>> is now for LRA in this patch.  But some doubt arose that it may
>>>> affect reload pass in some bad way.
>>>>
>>>> Ok to commit?
>>>
>>>
>>> I must admit to not remembering why I used ??&r.  I know I wanted it
>>> to prefer
>>> doing the memory patterns.  I would think we should try just the pattern
>>> without the ??.
>>>
>>
>>    I tried it about 2 months ago.  I did not see any problems of such
>> change for reload and LRA.  There were no regressions on GCC testsuite.
>>
>>    So, Mike, if you don't see any compelling reason to keep ??, probably
>> we should remove them.
>>
>> If you don't mind, I'll make the patch and test again and after that
>> submit it for approval.
>>
>
> Here is the patch.
>
> Tested and bootstrapped on gcc110.fsffrance.org.
>
>
> Ok to commit?
>
> 2013-12-05  Vladimir Makarov  <vmakarov@redhat.com>
>
>         * config/rs6000/rs600.md (*bswapdi2_64bit): Remove ?? from the
>         constraint.

Okay, let's just remove the "??" modifier from the constraint.

Thanks for your patience, explanations, and work on this, Vlad.

Thanks, David
Vladimir Makarov - Dec. 6, 2013, 10:23 p.m.
On 12/6/2013, 2:40 PM, David Edelsohn wrote:
> On Fri, Dec 6, 2013 at 2:02 PM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>> Here is the patch.
>>
>> Tested and bootstrapped on gcc110.fsffrance.org.
>>
>>
>> Ok to commit?
>>
>> 2013-12-05  Vladimir Makarov  <vmakarov@redhat.com>
>>
>>          * config/rs6000/rs600.md (*bswapdi2_64bit): Remove ?? from the
>>          constraint.
>
> Okay, let's just remove the "??" modifier from the constraint.
>
> Thanks for your patience, explanations, and work on this, Vlad.
>

Thanks, David.

Committed as rev. 205765.

I'd like to work more on LRA for power.  If you find any issue with LRA 
generated code performance, it could help me a lot.

I know you are busy with power8 stuff but if you see some LRA generated 
code examples can be improved, please let me know.

I'd like to work on spilling general regs into VSX regs too as it is 
done for intel x86-64 (spilling into SSE).  I guess it would help power8 
performance.  I am going to start this work when i have access to 
power8.  Although I should say the LRA code for this is not good right 
now - there are few opportunities for spilling into SSE on x86-64.  So 
probably, I rewrite this code in LRA.

Patch

Index: config/rs6000/rs6000.md
===================================================================
--- config/rs6000/rs6000.md     (revision 205753)
+++ config/rs6000/rs6000.md     (working copy)
@@ -2379,7 +2379,7 @@ 

  ;; Non-power7/cell, fall back to use lwbrx/stwbrx
  (define_insn "*bswapdi2_64bit"
-  [(set (match_operand:DI 0 "reg_or_mem_operand" "=&r,Z,??&r")
+  [(set (match_operand:DI 0 "reg_or_mem_operand" "=&r,Z,&r")
         (bswap:DI (match_operand:DI 1 "reg_or_mem_operand" "Z,r,r")))
     (clobber (match_scratch:DI 2 "=&b,&b,&r"))
     (clobber (match_scratch:DI 3 "=&r,&r,&r"))