On Mon, Jan 26, 2015 at 12:47 PM, Jakub Jelinek <ja...@redhat.com> wrote: > On Mon, Jan 26, 2015 at 12:35:30PM +0100, Steven Bosscher wrote: >> On Mon, Jan 26, 2015 at 10:11 AM, Jakub Jelinek wrote: >> > On Mon, Jan 26, 2015 at 10:06:05AM +0100, Eric Botcazou wrote: >> >> > While the cleanup_barriers runs after cleaning up BLOCK_FOR_INSNs, >> >> > some targets like i?86/x86_64 choose to populate it again during machine >> >> > reorg and some target don't free it at the end of machine reorg. >> >> > This patch updates cleanup_barrier pass, so that it adjusts basic block >> >> > boundaries and BLOCK_FOR_INSNs in that case, so that we don't crash >> >> > during >> >> > final pass. >> >> >> >> This isn't a recent regression so what about fixing it more "properly"? >> >> For >> >> example, by calling free_bb_for_insn at the end of the machinre reorg >> >> passes >> >> which called compute_bb_for_insn at the beginning? Or do the affected >> >> ports >> >> need the BB info all the way down to final? >> > >> > Yes, they do, that is why it crashed during final. >> >> And they should not do so. It cannot be relied upon, in general. Even >> now, recomputing BLOCK_FOR_INSN only works because machine reorg is >> the first pass after free_cfg (so nothing has changed yet to the insns >> stream). >> >> Some ports (including x86_64/i386, ia64, and most non-sched_dbr ports) >> could simply do away with free_cfg and cleanup_barriers. But some >> ports put things into the insns stream after free_cfg that >> verify_flow_info chokes on, e.g. the fake insns for const pools for >> ARM (that causes bugs elsewhere also, see e.g. >> https://gcc.gnu.org/ml/gcc-patches/2011-07/msg02101.html). The current >> situation is confusing and bound to give bugs sooner or later... >> >> I had patches to have an "early" and "late" free_cfg pass -- I think I >> even posted them and I still believe that's the right short-term >> solution -- to make sure nobody can put something between free_cfg and >> a target with a machine_reorg that needs the CFG, or at least >> BLOCK_FOR_INSN. > > Sure, we have targets which need cfg late, and targets which don't, and > targets which do need it only diring machine reorg and not afterwards. > > pass_free_cfg doesn't do just free_bb_for_insn though, it also adds > a note for hot cold partitioning, and for DBR targets other stuff, though > DBR targets are likely helpless with keeping CFG till the end. > > What my patch does is still compatible with removing that > free_bb_for_insn call from pass_free_cfg::execute if some flag is set > in targetm, it teaches the pass to handle cfg if it is around. > > I agree that freeing the cfg and immediately computing it again doesn't make > sense, but I just don't see this patch being incompatible with that.
I wonder if handing over pass pipeline control to targets at machine_reorg time (including free_cfg) makes sense... (either giving control back for stuff like final() or make it call into the middle-end again). Richard. > Jakub
