This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH][RFC] Sanity checking for -freorder-blocks-and-partition failures
- From: Steven Bosscher <stevenb dot gcc at gmail dot com>
- To: Teresa Johnson <tejohnson at google dot com>
- Cc: Christophe Lyon <christophe dot lyon at st dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Matthew Gretton-Dann <matthew dot gretton-dann at linaro dot org>, Christophe Lyon <christophe dot lyon at linaro dot org>
- Date: Fri, 9 Nov 2012 21:12:06 +0100
- Subject: Re: [PATCH][RFC] Sanity checking for -freorder-blocks-and-partition failures
- References: <CABu31nNHQhd-3_yJ-=yOzZ7v7ky1htPdVJFSoZbHQSpEzTmTQQ@mail.gmail.com> <CAAe5K+UXNQzQT0Dy_W7nrrYmt2_V_T9_UChuBET5OMySPUjrVA@mail.gmail.com> <509182F7.email@example.com> <CAAe5K+Uqhr_TuW+jTWXeRh-mWv_WNU3Ka5KYhsebHFqPfirstname.lastname@example.org> <CAAe5K+U5bDumqet8qGjsLg7_TQj=xd1qnt7FNatokQXPZUK7XA@mail.gmail.com>
It seems to me that it's better if you commit it along with your set
of fixes. My patch doesn't fix any bugs, it just exposes them :-)
On Fri, Nov 9, 2012 at 9:09 PM, Teresa Johnson <email@example.com> wrote:
> Hi Steven,
> I've spent this week trying to clean up all the issues exposed by this new verification patch. Some of the issues I described in the email thread on my related patch (http://gcc.gnu.org/ml/gcc-patches/2012-11/msg00287.html) and earlier in this thread. It also exposed more issues of the type described in the last message regarding my patch (the link I included here), where transformations were being applied but the partitions not being correctly fixed up. Things look clean now across SPEC2006 int C benchmarks at peak, gcc regression tests and our internal benchmarks. I need to update from head, retest and clean things up though before sending the new patch. But do you want to go ahead and commit this patch? I guess it should be fine to commit asynchronously with mine since -freorder-blocks-and-partition is off by default and not working anyway. I assume it can still go in since it was proposed already and is related to some outstanding bugs?
> On Wed, Oct 31, 2012 at 1:06 PM, Teresa Johnson <firstname.lastname@example.org> wrote:
>> On Wed, Oct 31, 2012 at 12:58 PM, Christophe Lyon
>> <email@example.com> wrote:
>> > On 30.10.2012 17:59, Teresa Johnson wrote:
>> >> On Tue, Oct 30, 2012 at 9:26 AM, Steven Bosscher <firstname.lastname@example.org>
>> >> wrote:
>> >>> Hello,
>> >>> Hot/cold partitioning is apparently a hot topic all of a sudden, which
>> >>> is a good thing of course, because it's in need of some TLC.
>> >>> The attached patch adds another check the RTL cfg checking
>> >>> (verify_flow_info) for the partitioning: A hot block can never be
>> >>> dominated by a cold block (because the dominated block must also be
>> >>> cold). This trips in PR55121.
>> >>> I haven't tested this with any profiling tests, but it's bound to
>> >>> break things. From my POV, whatever gets broken by this patch was
>> >>> already broken to begin with :-) If you're in CC, it's because I
>> >>> hope you can help test this patch.
>> >> I will try testing your patch on top of mine with our fdo benchmarks.
>> >> For the others on the cc list, you may need to include my patch as
>> >> well for testing. Without it, -freorder-blocks-and-partition was DOA
>> >> for me. For my patch, see
>> >> http://gcc.gnu.org/ml/gcc-patches/2012-10/msg02692.html
>> >> Teresa
>> > I have tried Steven's patch an indeed it reported numerous errors while
>> > compiling spec2k.
>> > I tried Teresa's patch too, but it changed nothing in my tests. The patches
>> > already posted by Matt are still necessary and Teresa's patch does not
>> > improve my tests.
>> With checking enabled I am seeing additional failures that my fixes
>> are not addressing. Looking into those now.
>> Can someone point me to Matt's patches? Is that this one:
>> or are there others?
>> > I am out of office at the moment, so it's a little bit inconvenient to
>> > investigate deeper the reasons for all the errors reported by Steven's
>> > patch. Anyway it looks like it's really needed :)
>> > I also noticed that some compilations failed with an ICE in calc_dfs_tree at
>> > dominance.c:395.
>> > Christophe.
>> Teresa Johnson | Software Engineer | email@example.com | 408-460-2413
> Teresa Johnson | Software Engineer | firstname.lastname@example.org | 408-460-2413