On Wed, Dec 12, 2012 at 11:29:28AM +0100, Steven Bosscher wrote: > >> (fixup_reorder_chain): Set cfg_layout_function_header to NULL to > >> avoid dangling pointers into GC-allocated insns. Clear BB_HEADER, > >> BB_FOOTER, and cfg_layout_function_footer for the same reason. > >> Do not link new barriers for cfgrtl mode to cfglayout's BB_FOOTER. > >> * combine.c (combine_instructions): Fix end-of-block check to not > >> expect BARRIERs, which may not exist in cfglayout mode. > > > > For the rest, I wonder if this really is stage3 material. Do you have a > > testcase that ICEs or is miscompiled without the changes? > > 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. > 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. Jakub