[PATCH] Update BBs in cleanup_barriers pass (PR rtl-optimization/61058)

Jakub Jelinek jakub@redhat.com
Mon Jan 26 13:19:00 GMT 2015


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.

	Jakub



More information about the Gcc-patches mailing list