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

[Ramana Radhakrishnan <ramana.gcc@googlemail.com>]

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.