Re: [PATCH] Relax one gcc_assert in lra-eliminate for fixed register

[Andrew Pinski <pinskia at gmail dot com>]

On Mon, Nov 17, 2014 at 12:36 PM, Jeff Law <law@redhat.com> wrote:
> On 11/17/14 02:26, Jiong Wang wrote:
>>
>> as Pinski reported at
>>
>>    https://gcc.gnu.org/ml/gcc-patches/2014-11/msg01967.html
>>
>> the previosu LR free patch on AArch64 cause one gcc_assert in
>> lra-elimination.c
>>
>> one of the problem is that gcc_assert is too strict and overkilled some
>> valid cases.
>>
>> the purpose of that assert is described here
>>
>>    https://gcc.gnu.org/ml/gcc-patches/2013-12/msg00151.html
>>
>> and I checked the code, it's purpose is to prohibit the following
>> scenario:
>>
>>    FP decided to be not required earlier, and then be required later.
>>
>> that assert prohibit above situation, because it will cause bad code
>> generation. FP
>> may be allocated earlier while because later it identified to be
>> required as fixed purpose, then
>> all those pseudo allocated to FP need to be spilled.
>>
>> so, this check will only make sense when that reg is not fixed itself.
>>
>> this patch relax that gcc_assert for fixed registers.
>>
>> the problem reported by Pinski fixed also, because FP of AArch64 is fixed.
>>
>> bootstrap ok on AArch64 and X86-64.
>> no regression on X86-64.
>> Pinskia's testcase passed on AArch64 on his code branch.
>>
>> ok for trunk?
>>
>> gcc/
>>    * lra-eliminations.c (update_reg_eliminate): Relax gcc_assert for
>> fixed registers.
>
> OK for the trunk.  It'd be useful to have a test, but my understanding is
> you need Pinski's modifications to trigger the failure.  Correct?

Yes you need to my modifications to trigger the failure.  Also I could
never get a small enough testcase that would have been useful for the
testsuite either due to any random changes did not expose the bug.

Now I have tested the patch and it works for my modified branch too.

Thanks,
Andrew

>
> Jeff
>