[PATCH, ARM, PR64208] LRA ICE Fix

[PATCH, ARM, PR64208] LRA ICE Fix

[Yvan Roux]

[-- Attachment #1: Type: text/plain, Size: 1296 bytes --]

Hi,

This is a fix for PR64208 where LRA loops when dealing with
iwmmxt_arm_movdi insn.  As explain in the PR, the issue was introduced
on trunk and 4.9 branch by fix of PR rtl-optimization/60969 and then
workaround by r211798 (-fuse-caller-save enable for ARM).

The changes in IRA cost made by PR60969, changed the register class of
this insn output from GENERAL_REGS to IWMMXT_REGS, and the
redundancies in the insn pattern alternatives description force LRA to
reload the pseudo, which generates the same iwmmxt_arm_movdi insn,
which can't be resolved, and so on ...

Removing the redundancies fixes the issue, as LRA find that
alternative 8 (Uy => y) matches.

This issue is present in 4.9 branch, but latent on trunk (the
clobbering of IP and CC information added during -fuse-caller-save
patch changed the register allocation).

Cross compiled and regression tested on ARM targets (but not on an
IWMMXT one), is it ok for trunk and 4.9 branch ?

Rq: I think that adding IP and CC clobbers to
CALL_INSN_FUNCTION_USAGE, as specified by AAPCS, in 4.9 branch is
something we need too, I've a patch for that if you agree on that.

Thanks,
Yvan

2105-03-17  Yvan Roux  <yvan.roux@linaro.org>

    PR target/64208
    * config/arm/iwmmxt.md ("*iwmmxt_arm_movdi"): Cleanup redundant
alternatives.

[-- Attachment #2: pr64208.diff --]
[-- Type: text/plain, Size: 759 bytes --]

diff --git a/gcc/config/arm/iwmmxt.md b/gcc/config/arm/iwmmxt.md
index fda3c2c..d1a60ff 100644
--- a/gcc/config/arm/iwmmxt.md
+++ b/gcc/config/arm/iwmmxt.md
@@ -107,8 +107,8 @@
 )
 
 (define_insn "*iwmmxt_arm_movdi"
-  [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r, r, r, r, m,y,y,yr,y,yrUy,*w, r,*w,*w, *Uv")
-        (match_operand:DI 1 "di_operand"              "rDa,Db,Dc,mi,r,y,yr,y,yrUy,y, r,*w,*w,*Uvi,*w"))]
+  [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r, r, r, r, m,y,y,r, y,Uy,*w, r,*w,*w, *Uv")
+        (match_operand:DI 1 "di_operand"              "rDa,Db,Dc,mi,r,y,r,y,Uy,y,  r,*w,*w,*Uvi,*w"))]
   "TARGET_REALLY_IWMMXT
    && (   register_operand (operands[0], DImode)
        || register_operand (operands[1], DImode))"

Re: [PATCH, ARM, PR64208] LRA ICE Fix

[Kyrill Tkachov]

Hi Yvan,

On 18/03/15 10:19, Yvan Roux wrote:
> Hi,
>
> This is a fix for PR64208 where LRA loops when dealing with
> iwmmxt_arm_movdi insn.  As explain in the PR, the issue was introduced
> on trunk and 4.9 branch by fix of PR rtl-optimization/60969 and then
> workaround by r211798 (-fuse-caller-save enable for ARM).
>
> The changes in IRA cost made by PR60969, changed the register class of
> this insn output from GENERAL_REGS to IWMMXT_REGS, and the
> redundancies in the insn pattern alternatives description force LRA to
> reload the pseudo, which generates the same iwmmxt_arm_movdi insn,
> which can't be resolved, and so on ...
>
> Removing the redundancies fixes the issue, as LRA find that
> alternative 8 (Uy => y) matches.
>
> This issue is present in 4.9 branch, but latent on trunk (the
> clobbering of IP and CC information added during -fuse-caller-save
> patch changed the register allocation).
>
> Cross compiled and regression tested on ARM targets (but not on an
> IWMMXT one), is it ok for trunk and 4.9 branch ?
>
> Rq: I think that adding IP and CC clobbers to
> CALL_INSN_FUNCTION_USAGE, as specified by AAPCS, in 4.9 branch is
> something we need too, I've a patch for that if you agree on that.
>
> Thanks,
> Yvan
>
> 2105-03-17  Yvan Roux  <yvan.roux@linaro.org>
>
>      PR target/64208
>      * config/arm/iwmmxt.md ("*iwmmxt_arm_movdi"): Cleanup redundant
> alternatives.

As a general note, I think this patch should include the testcase from 
the PR.
I can see that it's fairly self-contained.

Cheers,
Kyrill

Re: [PATCH, ARM, PR64208] LRA ICE Fix

[Yvan Roux]

HI Kyrill,

On 18 March 2015 at 11:24, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> Hi Yvan,
>
>
> On 18/03/15 10:19, Yvan Roux wrote:
>>
>> Hi,
>>
>> This is a fix for PR64208 where LRA loops when dealing with
>> iwmmxt_arm_movdi insn.  As explain in the PR, the issue was introduced
>> on trunk and 4.9 branch by fix of PR rtl-optimization/60969 and then
>> workaround by r211798 (-fuse-caller-save enable for ARM).
>>
>> The changes in IRA cost made by PR60969, changed the register class of
>> this insn output from GENERAL_REGS to IWMMXT_REGS, and the
>> redundancies in the insn pattern alternatives description force LRA to
>> reload the pseudo, which generates the same iwmmxt_arm_movdi insn,
>> which can't be resolved, and so on ...
>>
>> Removing the redundancies fixes the issue, as LRA find that
>> alternative 8 (Uy => y) matches.
>>
>> This issue is present in 4.9 branch, but latent on trunk (the
>> clobbering of IP and CC information added during -fuse-caller-save
>> patch changed the register allocation).
>>
>> Cross compiled and regression tested on ARM targets (but not on an
>> IWMMXT one), is it ok for trunk and 4.9 branch ?
>>
>> Rq: I think that adding IP and CC clobbers to
>> CALL_INSN_FUNCTION_USAGE, as specified by AAPCS, in 4.9 branch is
>> something we need too, I've a patch for that if you agree on that.
>>
>> Thanks,
>> Yvan
>>
>> 2105-03-17  Yvan Roux  <yvan.roux@linaro.org>
>>
>>      PR target/64208
>>      * config/arm/iwmmxt.md ("*iwmmxt_arm_movdi"): Cleanup redundant
>> alternatives.
>
>
> As a general note, I think this patch should include the testcase from the
> PR.
> I can see that it's fairly self-contained.

Yes, I wanted to find one that exhibits the issue on trunk as the PR
testcase doesn't, but don't find one so far.  If it's fine to include
a testcase that don't fail without that patch on trunk, I can include
it.

Cheers,
Yvan

Re: [PATCH, ARM, PR64208] LRA ICE Fix

[Yvan Roux]

[-- Attachment #1: Type: text/plain, Size: 2340 bytes --]

On 18 March 2015 at 12:42, Yvan Roux <yvan.roux@linaro.org> wrote:
> HI Kyrill,
>
> On 18 March 2015 at 11:24, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>> Hi Yvan,
>>
>>
>> On 18/03/15 10:19, Yvan Roux wrote:
>>>
>>> Hi,
>>>
>>> This is a fix for PR64208 where LRA loops when dealing with
>>> iwmmxt_arm_movdi insn.  As explain in the PR, the issue was introduced
>>> on trunk and 4.9 branch by fix of PR rtl-optimization/60969 and then
>>> workaround by r211798 (-fuse-caller-save enable for ARM).
>>>
>>> The changes in IRA cost made by PR60969, changed the register class of
>>> this insn output from GENERAL_REGS to IWMMXT_REGS, and the
>>> redundancies in the insn pattern alternatives description force LRA to
>>> reload the pseudo, which generates the same iwmmxt_arm_movdi insn,
>>> which can't be resolved, and so on ...
>>>
>>> Removing the redundancies fixes the issue, as LRA find that
>>> alternative 8 (Uy => y) matches.
>>>
>>> This issue is present in 4.9 branch, but latent on trunk (the
>>> clobbering of IP and CC information added during -fuse-caller-save
>>> patch changed the register allocation).
>>>
>>> Cross compiled and regression tested on ARM targets (but not on an
>>> IWMMXT one), is it ok for trunk and 4.9 branch ?
>>>
>>> Rq: I think that adding IP and CC clobbers to
>>> CALL_INSN_FUNCTION_USAGE, as specified by AAPCS, in 4.9 branch is
>>> something we need too, I've a patch for that if you agree on that.
>>>
>>> Thanks,
>>> Yvan
>>>
>>> 2105-03-17  Yvan Roux  <yvan.roux@linaro.org>
>>>
>>>      PR target/64208
>>>      * config/arm/iwmmxt.md ("*iwmmxt_arm_movdi"): Cleanup redundant
>>> alternatives.
>>
>>
>> As a general note, I think this patch should include the testcase from the
>> PR.
>> I can see that it's fairly self-contained.
>
> Yes, I wanted to find one that exhibits the issue on trunk as the PR
> testcase doesn't, but don't find one so far.  If it's fine to include
> a testcase that don't fail without that patch on trunk, I can include
> it.

Here is the patch with the testcase.

Cheers,
Yvan

gcc/
2105-03-18  Yvan Roux  <yvan.roux@linaro.org>

    PR target/64208
    * config/arm/iwmmxt.md ("*iwmmxt_arm_movdi"): Cleanup redundant
alternatives.

gcc/testsuite/
2105-03-18  Yvan Roux  <yvan.roux@linaro.org>

    PR target/64208
    * gcc.target/arm/pr64208.c: New test.

[-- Attachment #2: pr64208.diff --]
[-- Type: text/plain, Size: 1776 bytes --]

diff --git a/gcc/config/arm/iwmmxt.md b/gcc/config/arm/iwmmxt.md
index fda3c2c..d1a60ff 100644
--- a/gcc/config/arm/iwmmxt.md
+++ b/gcc/config/arm/iwmmxt.md
@@ -107,8 +107,8 @@
 )
 
 (define_insn "*iwmmxt_arm_movdi"
-  [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r, r, r, r, m,y,y,yr,y,yrUy,*w, r,*w,*w, *Uv")
-        (match_operand:DI 1 "di_operand"              "rDa,Db,Dc,mi,r,y,yr,y,yrUy,y, r,*w,*w,*Uvi,*w"))]
+  [(set (match_operand:DI 0 "nonimmediate_di_operand" "=r, r, r, r, m,y,y,r, y,Uy,*w, r,*w,*w, *Uv")
+        (match_operand:DI 1 "di_operand"              "rDa,Db,Dc,mi,r,y,r,y,Uy,y,  r,*w,*w,*Uvi,*w"))]
   "TARGET_REALLY_IWMMXT
    && (   register_operand (operands[0], DImode)
        || register_operand (operands[1], DImode))"
diff --git a/gcc/testsuite/gcc.target/arm/pr64208.c b/gcc/testsuite/gcc.target/arm/pr64208.c
new file mode 100644
index 0000000..96fd56d
--- /dev/null
+++ b/gcc/testsuite/gcc.target/arm/pr64208.c
@@ -0,0 +1,25 @@
+/* { dg-do compile } */
+/* { dg-skip-if "Test is specific to the iWMMXt" { arm*-*-* } { "-mcpu=*" } { "-mcpu=iwmmxt" } } */
+/* { dg-skip-if "Test is specific to the iWMMXt" { arm*-*-* } { "-mabi=*" } { "-mabi=iwmmxt" } } */
+/* { dg-skip-if "Test is specific to the iWMMXt" { arm*-*-* } { "-march=*" } { "-march=iwmmxt" } } */
+/* { dg-skip-if "Test is specific to ARM mode" { arm*-*-* } { "-mthumb" } { "" } } */
+/* { dg-require-effective-target arm32 } */
+/* { dg-require-effective-target arm_iwmmxt_ok } */
+/* { dg-options "-O1 -mcpu=iwmmxt" } */
+
+long long x6(void);
+void x7(long long, long long);
+void x8(long long);
+
+int x0;
+long long *x1;
+
+void x2(void) {
+  long long *x3 = x1;
+  while (x1) {
+    long long x4 = x0, x5 = x6();
+    x7(x4, x5);
+    x8(x5);
+    *x3 = 0;
+  }
+}

Re: [PATCH, ARM, PR64208] LRA ICE Fix

[Ramana Radhakrishnan]

On Wed, Mar 18, 2015 at 10:19 AM, Yvan Roux <yvan.roux@linaro.org> wrote:
> Hi,
>
> This is a fix for PR64208 where LRA loops when dealing with
> iwmmxt_arm_movdi insn.  As explain in the PR, the issue was introduced
> on trunk and 4.9 branch by fix of PR rtl-optimization/60969 and then
> workaround by r211798 (-fuse-caller-save enable for ARM).
>
> The changes in IRA cost made by PR60969, changed the register class of
> this insn output from GENERAL_REGS to IWMMXT_REGS, and the
> redundancies in the insn pattern alternatives description force LRA to
> reload the pseudo, which generates the same iwmmxt_arm_movdi insn,
> which can't be resolved, and so on ...
>
> Removing the redundancies fixes the issue, as LRA find that
> alternative 8 (Uy => y) matches.
>
> This issue is present in 4.9 branch, but latent on trunk (the
> clobbering of IP and CC information added during -fuse-caller-save
> patch changed the register allocation).
>
> Cross compiled and regression tested on ARM targets (but not on an
> IWMMXT one), is it ok for trunk and 4.9 branch ?


This looks sane. It doesn't look reasonable for alternatives to be
duplicating each other.

Given I have neither the time nor the hardware to test this patch on,
I'd rather someone with an interest in iwMMX possibly folks from
Marvell can pick up testing for this patch.

regards
Ramana

>
> Rq: I think that adding IP and CC clobbers to
> CALL_INSN_FUNCTION_USAGE, as specified by AAPCS, in 4.9 branch is
> something we need too, I've a patch for that if you agree on that.
>
> Thanks,
> Yvan
>
> 2105-03-17  Yvan Roux  <yvan.roux@linaro.org>
>
>     PR target/64208
>     * config/arm/iwmmxt.md ("*iwmmxt_arm_movdi"): Cleanup redundant
> alternatives.

Re: [PATCH, ARM, PR64208] LRA ICE Fix

[Yvan Roux]

Hi,

On 23 March 2015 at 17:08, Ramana Radhakrishnan
<ramana.gcc@googlemail.com> wrote:
> On Wed, Mar 18, 2015 at 10:19 AM, Yvan Roux <yvan.roux@linaro.org> wrote:
>> Hi,
>>
>> This is a fix for PR64208 where LRA loops when dealing with
>> iwmmxt_arm_movdi insn.  As explain in the PR, the issue was introduced
>> on trunk and 4.9 branch by fix of PR rtl-optimization/60969 and then
>> workaround by r211798 (-fuse-caller-save enable for ARM).
>>
>> The changes in IRA cost made by PR60969, changed the register class of
>> this insn output from GENERAL_REGS to IWMMXT_REGS, and the
>> redundancies in the insn pattern alternatives description force LRA to
>> reload the pseudo, which generates the same iwmmxt_arm_movdi insn,
>> which can't be resolved, and so on ...
>>
>> Removing the redundancies fixes the issue, as LRA find that
>> alternative 8 (Uy => y) matches.
>>
>> This issue is present in 4.9 branch, but latent on trunk (the
>> clobbering of IP and CC information added during -fuse-caller-save
>> patch changed the register allocation).
>>
>> Cross compiled and regression tested on ARM targets (but not on an
>> IWMMXT one), is it ok for trunk and 4.9 branch ?
>
>
> This looks sane. It doesn't look reasonable for alternatives to be
> duplicating each other.
>
> Given I have neither the time nor the hardware to test this patch on,
> I'd rather someone with an interest in iwMMX possibly folks from
> Marvell can pick up testing for this patch.

Ok, Thanks Ramana, I'll wait for somebody able to test it. Notice,
that without this patch on the 4.9 branch, building a cross compiler
which default to iWMMXT architectures ICE on that during LRA while
building of libgcc.

Cheers,
Yvan

> regards
> Ramana
>
>>
>> Rq: I think that adding IP and CC clobbers to
>> CALL_INSN_FUNCTION_USAGE, as specified by AAPCS, in 4.9 branch is
>> something we need too, I've a patch for that if you agree on that.
>>
>> Thanks,
>> Yvan
>>
>> 2105-03-17  Yvan Roux  <yvan.roux@linaro.org>
>>
>>     PR target/64208
>>     * config/arm/iwmmxt.md ("*iwmmxt_arm_movdi"): Cleanup redundant
>> alternatives.

Re: [PATCH, ARM, PR64208] LRA ICE Fix

[Yvan Roux]

Hi Xingxing,

do you know if it is possible to test this patch inside Marvell (as it
is a fix for iWMMXT arch.) ?

Thanks a lot
Yvan

On 23 March 2015 at 18:47, Yvan Roux <yvan.roux@linaro.org> wrote:
> Hi,
>
> On 23 March 2015 at 17:08, Ramana Radhakrishnan
> <ramana.gcc@googlemail.com> wrote:
>> On Wed, Mar 18, 2015 at 10:19 AM, Yvan Roux <yvan.roux@linaro.org> wrote:
>>> Hi,
>>>
>>> This is a fix for PR64208 where LRA loops when dealing with
>>> iwmmxt_arm_movdi insn.  As explain in the PR, the issue was introduced
>>> on trunk and 4.9 branch by fix of PR rtl-optimization/60969 and then
>>> workaround by r211798 (-fuse-caller-save enable for ARM).
>>>
>>> The changes in IRA cost made by PR60969, changed the register class of
>>> this insn output from GENERAL_REGS to IWMMXT_REGS, and the
>>> redundancies in the insn pattern alternatives description force LRA to
>>> reload the pseudo, which generates the same iwmmxt_arm_movdi insn,
>>> which can't be resolved, and so on ...
>>>
>>> Removing the redundancies fixes the issue, as LRA find that
>>> alternative 8 (Uy => y) matches.
>>>
>>> This issue is present in 4.9 branch, but latent on trunk (the
>>> clobbering of IP and CC information added during -fuse-caller-save
>>> patch changed the register allocation).
>>>
>>> Cross compiled and regression tested on ARM targets (but not on an
>>> IWMMXT one), is it ok for trunk and 4.9 branch ?
>>
>>
>> This looks sane. It doesn't look reasonable for alternatives to be
>> duplicating each other.
>>
>> Given I have neither the time nor the hardware to test this patch on,
>> I'd rather someone with an interest in iwMMX possibly folks from
>> Marvell can pick up testing for this patch.
>
> Ok, Thanks Ramana, I'll wait for somebody able to test it. Notice,
> that without this patch on the 4.9 branch, building a cross compiler
> which default to iWMMXT architectures ICE on that during LRA while
> building of libgcc.
>
> Cheers,
> Yvan
>
>> regards
>> Ramana
>>
>>>
>>> Rq: I think that adding IP and CC clobbers to
>>> CALL_INSN_FUNCTION_USAGE, as specified by AAPCS, in 4.9 branch is
>>> something we need too, I've a patch for that if you agree on that.
>>>
>>> Thanks,
>>> Yvan
>>>
>>> 2105-03-17  Yvan Roux  <yvan.roux@linaro.org>
>>>
>>>     PR target/64208
>>>     * config/arm/iwmmxt.md ("*iwmmxt_arm_movdi"): Cleanup redundant
>>> alternatives.

Re: [PATCH, ARM, PR64208] LRA ICE Fix

[Yvan Roux]

Hi,

On 23 March 2015 at 18:47, Yvan Roux <yvan.roux@linaro.org> wrote:
> Hi,
>
> On 23 March 2015 at 17:08, Ramana Radhakrishnan
> <ramana.gcc@googlemail.com> wrote:
>> On Wed, Mar 18, 2015 at 10:19 AM, Yvan Roux <yvan.roux@linaro.org> wrote:
>>> Hi,
>>>
>>> This is a fix for PR64208 where LRA loops when dealing with
>>> iwmmxt_arm_movdi insn.  As explain in the PR, the issue was introduced
>>> on trunk and 4.9 branch by fix of PR rtl-optimization/60969 and then
>>> workaround by r211798 (-fuse-caller-save enable for ARM).
>>>
>>> The changes in IRA cost made by PR60969, changed the register class of
>>> this insn output from GENERAL_REGS to IWMMXT_REGS, and the
>>> redundancies in the insn pattern alternatives description force LRA to
>>> reload the pseudo, which generates the same iwmmxt_arm_movdi insn,
>>> which can't be resolved, and so on ...
>>>
>>> Removing the redundancies fixes the issue, as LRA find that
>>> alternative 8 (Uy => y) matches.
>>>
>>> This issue is present in 4.9 branch, but latent on trunk (the
>>> clobbering of IP and CC information added during -fuse-caller-save
>>> patch changed the register allocation).
>>>
>>> Cross compiled and regression tested on ARM targets (but not on an
>>> IWMMXT one), is it ok for trunk and 4.9 branch ?
>>
>>
>> This looks sane. It doesn't look reasonable for alternatives to be
>> duplicating each other.
>>
>> Given I have neither the time nor the hardware to test this patch on,
>> I'd rather someone with an interest in iwMMX possibly folks from
>> Marvell can pick up testing for this patch.
>
> Ok, Thanks Ramana, I'll wait for somebody able to test it. Notice,
> that without this patch on the 4.9 branch, building a cross compiler
> which default to iWMMXT architectures ICE on that during LRA while
> building of libgcc.

I got an access to a cubox with an armada 510 and finally managed to
validate this patch (~ 1week for bootstrap + make check !).  So,
bootstrap is ok and no regession.  is it Ok for trunk and branches
(the issue was observed on 4.9) ? Notice that I've only tested it for
trunk and I don't plan to validate it on the branches ! ;)

Thanks
Yvan


> Cheers,
> Yvan
>
>> regards
>> Ramana
>>
>>>
>>> Rq: I think that adding IP and CC clobbers to
>>> CALL_INSN_FUNCTION_USAGE, as specified by AAPCS, in 4.9 branch is
>>> something we need too, I've a patch for that if you agree on that.
>>>
>>> Thanks,
>>> Yvan
>>>
>>> 2105-03-17  Yvan Roux  <yvan.roux@linaro.org>
>>>
>>>     PR target/64208
>>>     * config/arm/iwmmxt.md ("*iwmmxt_arm_movdi"): Cleanup redundant
>>> alternatives.

Re: [PATCH, ARM, PR64208] LRA ICE Fix

[Ramana Radhakrishnan]

On Thu, Apr 30, 2015 at 6:49 PM, Yvan Roux <yvan.roux@linaro.org> wrote:
> Hi,
>
> On 23 March 2015 at 18:47, Yvan Roux <yvan.roux@linaro.org> wrote:
>> Hi,
>>
>> On 23 March 2015 at 17:08, Ramana Radhakrishnan
>> <ramana.gcc@googlemail.com> wrote:
>>> On Wed, Mar 18, 2015 at 10:19 AM, Yvan Roux <yvan.roux@linaro.org> wrote:
>>>> Hi,
>>>>
>>>> This is a fix for PR64208 where LRA loops when dealing with
>>>> iwmmxt_arm_movdi insn.  As explain in the PR, the issue was introduced
>>>> on trunk and 4.9 branch by fix of PR rtl-optimization/60969 and then
>>>> workaround by r211798 (-fuse-caller-save enable for ARM).
>>>>
>>>> The changes in IRA cost made by PR60969, changed the register class of
>>>> this insn output from GENERAL_REGS to IWMMXT_REGS, and the
>>>> redundancies in the insn pattern alternatives description force LRA to
>>>> reload the pseudo, which generates the same iwmmxt_arm_movdi insn,
>>>> which can't be resolved, and so on ...
>>>>
>>>> Removing the redundancies fixes the issue, as LRA find that
>>>> alternative 8 (Uy => y) matches.
>>>>
>>>> This issue is present in 4.9 branch, but latent on trunk (the
>>>> clobbering of IP and CC information added during -fuse-caller-save
>>>> patch changed the register allocation).
>>>>
>>>> Cross compiled and regression tested on ARM targets (but not on an
>>>> IWMMXT one), is it ok for trunk and 4.9 branch ?
>>>
>>>
>>> This looks sane. It doesn't look reasonable for alternatives to be
>>> duplicating each other.
>>>
>>> Given I have neither the time nor the hardware to test this patch on,
>>> I'd rather someone with an interest in iwMMX possibly folks from
>>> Marvell can pick up testing for this patch.
>>
>> Ok, Thanks Ramana, I'll wait for somebody able to test it. Notice,
>> that without this patch on the 4.9 branch, building a cross compiler
>> which default to iWMMXT architectures ICE on that during LRA while
>> building of libgcc.
>
> I got an access to a cubox with an armada 510 and finally managed to
> validate this patch (~ 1week for bootstrap + make check !).  So,
> bootstrap is ok and no regession.  is it Ok for trunk and branches
> (the issue was observed on 4.9) ? Notice that I've only tested it for
> trunk and I don't plan to validate it on the branches ! ;)

OK for trunk - Thanks for taking the extra effort to get an armada
board to validate this on.

 it's ok for the branches only if you validate it on the branches. If
someone is interested in the bug fix they can always pick it up

Ramana

>
> Thanks
> Yvan
>
>
>> Cheers,
>> Yvan
>>
>>> regards
>>> Ramana
>>>
>>>>
>>>> Rq: I think that adding IP and CC clobbers to
>>>> CALL_INSN_FUNCTION_USAGE, as specified by AAPCS, in 4.9 branch is
>>>> something we need too, I've a patch for that if you agree on that.
>>>>
>>>> Thanks,
>>>> Yvan
>>>>
>>>> 2105-03-17  Yvan Roux  <yvan.roux@linaro.org>
>>>>
>>>>     PR target/64208
>>>>     * config/arm/iwmmxt.md ("*iwmmxt_arm_movdi"): Cleanup redundant
>>>> alternatives.

Re: [PATCH, ARM, PR64208] LRA ICE Fix

[Yvan Roux]

On 6 May 2015 at 11:50, Ramana Radhakrishnan <ramana.gcc@googlemail.com> wrote:
> On Thu, Apr 30, 2015 at 6:49 PM, Yvan Roux <yvan.roux@linaro.org> wrote:
>> Hi,
>>
>> On 23 March 2015 at 18:47, Yvan Roux <yvan.roux@linaro.org> wrote:
>>> Hi,
>>>
>>> On 23 March 2015 at 17:08, Ramana Radhakrishnan
>>> <ramana.gcc@googlemail.com> wrote:
>>>> On Wed, Mar 18, 2015 at 10:19 AM, Yvan Roux <yvan.roux@linaro.org> wrote:
>>>>> Hi,
>>>>>
>>>>> This is a fix for PR64208 where LRA loops when dealing with
>>>>> iwmmxt_arm_movdi insn.  As explain in the PR, the issue was introduced
>>>>> on trunk and 4.9 branch by fix of PR rtl-optimization/60969 and then
>>>>> workaround by r211798 (-fuse-caller-save enable for ARM).
>>>>>
>>>>> The changes in IRA cost made by PR60969, changed the register class of
>>>>> this insn output from GENERAL_REGS to IWMMXT_REGS, and the
>>>>> redundancies in the insn pattern alternatives description force LRA to
>>>>> reload the pseudo, which generates the same iwmmxt_arm_movdi insn,
>>>>> which can't be resolved, and so on ...
>>>>>
>>>>> Removing the redundancies fixes the issue, as LRA find that
>>>>> alternative 8 (Uy => y) matches.
>>>>>
>>>>> This issue is present in 4.9 branch, but latent on trunk (the
>>>>> clobbering of IP and CC information added during -fuse-caller-save
>>>>> patch changed the register allocation).
>>>>>
>>>>> Cross compiled and regression tested on ARM targets (but not on an
>>>>> IWMMXT one), is it ok for trunk and 4.9 branch ?
>>>>
>>>>
>>>> This looks sane. It doesn't look reasonable for alternatives to be
>>>> duplicating each other.
>>>>
>>>> Given I have neither the time nor the hardware to test this patch on,
>>>> I'd rather someone with an interest in iwMMX possibly folks from
>>>> Marvell can pick up testing for this patch.
>>>
>>> Ok, Thanks Ramana, I'll wait for somebody able to test it. Notice,
>>> that without this patch on the 4.9 branch, building a cross compiler
>>> which default to iWMMXT architectures ICE on that during LRA while
>>> building of libgcc.
>>
>> I got an access to a cubox with an armada 510 and finally managed to
>> validate this patch (~ 1week for bootstrap + make check !).  So,
>> bootstrap is ok and no regession.  is it Ok for trunk and branches
>> (the issue was observed on 4.9) ? Notice that I've only tested it for
>> trunk and I don't plan to validate it on the branches ! ;)
>
> OK for trunk - Thanks for taking the extra effort to get an armada
> board to validate this on.
>
>  it's ok for the branches only if you validate it on the branches. If
> someone is interested in the bug fix they can always pick it up

Ok fair enough. Thanks Ramana.

Cheers,
Yvan