This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch] cfglayout fixes
On Wed, Dec 12, 2012 at 11:48 AM, Jakub Jelinek wrote:
>> The fixup_reorder_chain patch fixes and ICE with hot-cold partitioning
>> and GCAC checking. A BARRIER is removed somewhere along the line, but
>> the BB_FOOTER pointer still points to it. I'm not sure how things go
>> bad from there, but we end up with BB_FOOTER pointing to a collected
>> barrier.
>
> Then a testcase should be added together with the patch and the change
> shouldn't be part of further unrelated changes.
You're kidding, right? A test case for a GCAC failure with profiling?
I wouldn't even know how to write a test case for it :-)
I'll resubmit for GCC 4.9. Instead of a test case, I have a verifier
queued that asserts no BARRIERs appear in the insns stream when in
cfglayout mode. (This verifier currently breaks hot-cold partitioning
all over the place due to a bug in try_redirect_by_replacing_jump.)
>> The combine fix is a fix for a real bug. Your patch makes combine look
>> across basic block boundaries and look for BARRIERs which can never be
>> there. Technically this is a regression.
>
> I don't see how. If there is no barrier, but the prev note or insn doesn't
> belong to the current bb, then BLOCK_FOR_INSN (last_combined_insn) != this_basic_block
> will be true, and if there is no insn at all, last_combined_insn will be
> NULL. I view your combine.c change purely as a cleanup, thus IMHO stage 1
> material.
Whatever, also fine. I assume you'll take care of this, given that you
introduced this bug?
Ciao!
Steven