| Message ID | CAD57uCfHZuBqWbMiqGT+qDXQj37NCa9wKcEN+RpcUS3xpurYhQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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
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.
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.
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.
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.
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.
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
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))"
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.