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
