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.