Re: [PATCH] Fix PR44563 more

[Jan Hubicka <hubicka@ucw.cz>]

> > 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.
> 
> This patch should get a good speedup on all compiles (we run
> CFG-cleanup a _lot_), by removing pointless IL walks and expensive
> gimple_call_flags calls on calls.

Yes, i definitely like it.  The expensiveness of cfg-cleanup always
quite bothered me.
On unrelated note, I think it is possible to do cfg-cleanup with
only fixed number of passes over CFG (my old RTL code did that but
it was changed since then adding crossjumping). Both RTL and tree
cfg cleanups have complicated history, there are probably quite
few ways to get them cheaper.

Thanks for working on it. As I noted in PR I plan to finally fix the
inliner non-linearity with the sreal metrics next stage1 too.

Honza