Patchwork patch to solve PR49936

On 08/20/2011 06:13 AM, Richard Sandiford wrote:
> Hi Vlad,
>
> Vladimir Makarov<vmakarov@redhat.com>  writes:
>> The following patch makes gcc4.7 behaving as gcc4.6 for the case
>> described on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49936.
>>
>> The patch was successfully bootstrapped on x86_64 and ppc64.
>>
>> Committed as rev 177916.
>>
>> 2011-08-19  Vladimir Makarov<vmakarov@redhat.com>
>>
>>           PR rtl-optimization/49936
>>           * ira.c (ira_init_register_move_cost): Ignore too small subclasses
>>           for calculation of max register move costs.
> Thanks for the patch.  The allocno class costs for MIPS look
> much better now.
>
> However, the patch seems to expose a latent problem with the use of
> ira_reg_class_max_nregs.  We set the number of allocno objects based
> on the ira_reg_class_max_nregs of the allocno class, but often
> expect that to be the same as the ira_reg_class_max_nregs of the
> pressure class.  I can't see anything in the calculation of the
> pressure classes to enforce that though.
>
> In current trunk, this shows up as a failure to build libgcc
> on mips64-linux-gnu.  We abort on:
>
>    pclass = ira_pressure_class_translate[ALLOCNO_CLASS (a)];
>    nregs = ira_reg_class_max_nregs[pclass][ALLOCNO_MODE (a)];
>    gcc_assert (nregs == n);
>
> in ira-lives.c:mark_pseudo_regno_subword_live for the attached
> testcase, compiled with -O2 -mabi=64.
>
> In this case it's a MIPS backend bug.  The single pressure class
> for MIPS is ALL_REGS, and CLASS_MAX_NREGS (ALL_REGS, TImode)
> is returning 4, based on the fact that ALL_REGS includes the
> floating-point condition codes.  (CCmode is hard-wired to 4 bytes,
> so for CCV2 and CCV4, the correct number of registers is the size
> of the mode divided by 4.)  Since floating-point condition codes
> can't store TImode, the backend should be ignoring them and
> returning 2 instead.  I'm testing a fix for that now.
Thanks, Richard.  It looks like my merging with Bernd's introduction of 
objects about year ago results in a few typos (they are present in 
mark_pseudo_subword_{live|dead} but absent in other places).  It is 
obvious that allocno class should be used instead of pressure class for 
estimation how many registers are used).  I have the patch to fix it 
too.  You could use it for your patch if you want.

I found also another typo in mark_pseudo_subword_live (strangely it is 
absent mark_pseudo_subword_dead).  The pressure should be increased by 1 
not by nregs.

On 08/20/2011 06:39 PM, Vladimir Makarov wrote:
> On 08/20/2011 06:13 AM, Richard Sandiford wrote:
>> Hi Vlad,
>>
>> Vladimir Makarov<vmakarov@redhat.com>  writes:
>>> The following patch makes gcc4.7 behaving as gcc4.6 for the case
>>> described on http://gcc.gnu.org/bugzilla/show_bug.cgi?id=49936.
>>>
>>> The patch was successfully bootstrapped on x86_64 and ppc64.
>>>
>>> Committed as rev 177916.
>>>
>>> 2011-08-19  Vladimir Makarov<vmakarov@redhat.com>
>>>
>>>           PR rtl-optimization/49936
>>>           * ira.c (ira_init_register_move_cost): Ignore too small 
>>> subclasses
>>>           for calculation of max register move costs.
>> Thanks for the patch.  The allocno class costs for MIPS look
>> much better now.
>>
>> However, the patch seems to expose a latent problem with the use of
>> ira_reg_class_max_nregs.  We set the number of allocno objects based
>> on the ira_reg_class_max_nregs of the allocno class, but often
>> expect that to be the same as the ira_reg_class_max_nregs of the
>> pressure class.  I can't see anything in the calculation of the
>> pressure classes to enforce that though.
>>
>> In current trunk, this shows up as a failure to build libgcc
>> on mips64-linux-gnu.  We abort on:
>>
>>    pclass = ira_pressure_class_translate[ALLOCNO_CLASS (a)];
>>    nregs = ira_reg_class_max_nregs[pclass][ALLOCNO_MODE (a)];
>>    gcc_assert (nregs == n);
>>
>> in ira-lives.c:mark_pseudo_regno_subword_live for the attached
>> testcase, compiled with -O2 -mabi=64.
>>
>> In this case it's a MIPS backend bug.  The single pressure class
>> for MIPS is ALL_REGS, and CLASS_MAX_NREGS (ALL_REGS, TImode)
>> is returning 4, based on the fact that ALL_REGS includes the
>> floating-point condition codes.  (CCmode is hard-wired to 4 bytes,
>> so for CCV2 and CCV4, the correct number of registers is the size
>> of the mode divided by 4.)  Since floating-point condition codes
>> can't store TImode, the backend should be ignoring them and
>> returning 2 instead.  I'm testing a fix for that now.
> Thanks, Richard.  It looks like my merging with Bernd's introduction 
> of objects about year ago results in a few typos (they are present in 
> mark_pseudo_subword_{live|dead} but absent in other places).  It is 
> obvious that allocno class should be used instead of pressure class 
> for estimation how many registers are used).  I have the patch to fix 
> it too.  You could use it for your patch if you want.
>
> I found also another typo in mark_pseudo_subword_live (strangely it is 
> absent mark_pseudo_subword_dead).  The pressure should be increased by 
> 1 not by nregs.
>
>
I've just committed the patch as rev. 177939.

I successfully bootstrapped it on x86-64 and ppc64.

2011-08-20  Vladimir Makarov <vmakarov@redhat.com>

         * ira-lives.c (mark_pseudo_regno_subword_live): Use allocno class
         for ira_reg_class_max_nregs.  Increase pressure by 1.
         (mark_pseudo_regno_subword_dead): Use allocno class
         for ira_reg_class_max_nregs.