Ilya is on vacation so I'll make the answer.

Overall score became worse on 0.3%.

> -----Original Message-----
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-ow...@gcc.gnu.org] On 
> Behalf Of Vladimir Makarov
> Sent: Thursday, December 22, 2011 8:15 PM
> To: Ilya Enkovich
> Cc: gcc-patches
> Subject: Re: patch to fix PR21617
>
> On 12/22/2011 06:19 AM, Ilya Enkovich wrote:
>> 2011/12/13 Vladimir Makarov<vmaka...@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<vmaka...@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.

Reply via email to