patch to fix PR21617

The following patch solves PR 21617 which is described on 
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21617.

Just adding number of necessary hard registers solves the problem but 
creates 2% SPEC2000 perlbmk degradation on x86.  Fortunately, removing 
allocno class comparison removes the degradation.   The allocno class 
comparison was originally added for debugging purposes to put coloring 
of allocnos of the same class in one place.  It was ok when we had cover 
classes which did not intersect.  Now allocno classes intersect and this 
comparison should be not used (at least as highest priority heuristic).

Beside solving the problem, the patch improves a bit SPEC2000 
performance and code size on x86.

The patch was successfully bootstrapped on x86/x86-64.  I expect it 
should be pretty safe for other targets because it changes heuristics only.

Committed as rev. 182263.


2011-12-12  Vladimir Makarov <vmakarov@redhat.com>

         PR rtl-optimization/21617
         * ira-color.c (bucket_allocno_compare_func): Don't compare
         allocno classes.  Compare number of hard registers needed.

2011/12/13 Vladimir Makarov <vmakarov@redhat.com>:
> The following patch solves PR 21617 which is described on
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21617.
>
> Just adding number of necessary hard registers solves the problem but
> creates 2% SPEC2000 perlbmk degradation on x86.  Fortunately, removing
> allocno class comparison removes the degradation.   The allocno class
> comparison was originally added for debugging purposes to put coloring of
> allocnos of the same class in one place.  It was ok when we had cover
> classes which did not intersect.  Now allocno classes intersect and this
> comparison should be not used (at least as highest priority heuristic).
>
> Beside solving the problem, the patch improves a bit SPEC2000 performance
> and code size on x86.
>
> The patch was successfully bootstrapped on x86/x86-64.  I expect it should
> be pretty safe for other targets because it changes heuristics only.
>
> Committed as rev. 182263.
>
>
> 2011-12-12  Vladimir Makarov <vmakarov@redhat.com>
>
>        PR rtl-optimization/21617
>        * ira-color.c (bucket_allocno_compare_func): Don't compare
>        allocno classes.  Compare number of hard registers needed.
>

Hello, Vladimir,

I think there may be a typo in this patch. I saw few degradations in
EEMBC2.0 benchmarks caused by this patch. Following fix removes these
degradations:

-  if ((diff = (ira_reg_class_max_nregs[cl1][ALLOCNO_MODE (a1)]
-              - ira_reg_class_max_nregs[cl2][ALLOCNO_MODE (a2)])) != 0)
+  if ((diff = (ira_reg_class_max_nregs[cl2][ALLOCNO_MODE (a2)]
+              - ira_reg_class_max_nregs[cl1][ALLOCNO_MODE (a1)])) != 0)

I suspect typo because previous calculation of 'diff' value used f(a2)
- f(a1) formula and now it is f(a1) - f(a2). Could you please look at
it?

Thanks,
Ilya

On 12/22/2011 06:19 AM, Ilya Enkovich wrote:
> 2011/12/13 Vladimir Makarov<vmakarov@redhat.com>:
>> The following patch solves PR 21617 which is described on
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21617.
>>
>> Just adding number of necessary hard registers solves the problem but
>> creates 2% SPEC2000 perlbmk degradation on x86.  Fortunately, removing
>> allocno class comparison removes the degradation.   The allocno class
>> comparison was originally added for debugging purposes to put coloring of
>> allocnos of the same class in one place.  It was ok when we had cover
>> classes which did not intersect.  Now allocno classes intersect and this
>> comparison should be not used (at least as highest priority heuristic).
>>
>> Beside solving the problem, the patch improves a bit SPEC2000 performance
>> and code size on x86.
>>
>> The patch was successfully bootstrapped on x86/x86-64.  I expect it should
>> be pretty safe for other targets because it changes heuristics only.
>>
>> Committed as rev. 182263.
>>
>>
>> 2011-12-12  Vladimir Makarov<vmakarov@redhat.com>
>>
>>         PR rtl-optimization/21617
>>         * ira-color.c (bucket_allocno_compare_func): Don't compare
>>         allocno classes.  Compare number of hard registers needed.
>>
> Hello, Vladimir,
>
> I think there may be a typo in this patch. I saw few degradations in
> EEMBC2.0 benchmarks caused by this patch. Following fix removes these
> degradations:
>
> -  if ((diff = (ira_reg_class_max_nregs[cl1][ALLOCNO_MODE (a1)]
> -              - ira_reg_class_max_nregs[cl2][ALLOCNO_MODE (a2)])) != 0)
> +  if ((diff = (ira_reg_class_max_nregs[cl2][ALLOCNO_MODE (a2)]
> +              - ira_reg_class_max_nregs[cl1][ALLOCNO_MODE (a1)])) != 0)
>
> I suspect typo because previous calculation of 'diff' value used f(a2)
> - f(a1) formula and now it is f(a1) - f(a2). Could you please look at
> it?
>
I don't think it was a typo.  The patch puts multi-registers pseudos at 
the end of the coloring bucket, so they will push to the coloring stacke 
last and popped from the stack first and get better chance to be 
assigned to hard registers because smaller chance of small holes 
creation of free hard registers.

Generally speaking, RA is all about heuristics and it is always possible 
to find a test when one heuristic will work better than another one and 
vise versa.  So for me, there is a little sense to work on such PRs of 
course unless it results in a discovery of better heuristics which 
improves overall score on a credible test set like SPECCPU, for example.

I work on these PRs mostly to help next gcc release to come out.

Returning to your complaint, could you give me info how the overall 
score of EEMBC changed after committing the patch, not just saying that 
there are few degradations.