Re: [PATCH] Fix PR44563 more

[Richard Biener <rguenther at suse dot de>]

On Fri, 13 Mar 2015, Jan Hubicka wrote:

> > > CFG cleanup currently searches for calls that became noreturn and
> > > fixes them up (splitting block and removing the fallthru).  Previously
> > > that was technically necessary as propagation may have turned an
> > > indirect call into a direct noreturn call and the CFG verifier would
> > > have barfed.  Today we guard that with GF_CALL_CTRL_ALTERING and
> > > thus we "remember" the previous call analysis.
> 
> Yep, I remember introducing this in back in tree-SSA branch days as kind
> of aftertought.
> > > 
> > > The following patch removes the CFG cleanup code (which is expensive
> > > because gimple_call_flags () is quite expensive, not to talk about
> > > walking all stmts).  This leaves the fixup_cfg passes to perform the
> > > very same optimization (relevant propagators can also be teached
> > > to call fixup_noreturn_call, but I don't think that's very important).
> > > 
> > > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > > 
> > > I'm somewhat undecided whether this is ok at this stage and if we
> > > _do_ want to make propagators fix those (previously indirect) calls up
> > > earlier at the same time.
> > > 
> > > Honza - I think we performed this in CFG cleanup for the sake of CFG 
> > > checking, not for the sake of prompt optimization, no?
> 
> It is first time I hear this.   We have verify_flow_info.
> I think most of CFG cleanups was scheudled because we need update-ssa and
> that would bomb on unreachable basic blocks.
> > > 
> > > This would make PR44563 a pure IPA pass issue.
> > 
> > Soo - testing revealed a single case where we mess up things (and
> > the verifier noticing only because of a LHS on a noreturn call...).
> > 
> > The following patch makes all propagators handle the noreturn transition
> > (the paths in all but PRE are not exercised by bootstrap or testsuite :/).
> > 
> > This patch makes CFG cleanup independent on BB size (during analysis,
> > merge_blocks and delete_basic_block are still O(n)) - which is
> > a very much desired property.
> > 
> > It also changes fixup_cfg to produce a dump only when run as
> > separate pass (otherwise the .optimized dump changes and I get
> > tons of scan related fails) - that also reduces noise in the
> > very many places we dump functions (they are dumped anyway for
> > all cases).
> > 
> > Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> > 
> > I wonder if you can throw this on firefox/chromium - the critical
> > paths are devirtualization introducing __builtin_unreachable.
> 
> I built chromium and firefox without problems with your patch.

Thanks - I went ahead and committed the patch.

Richard.