> On Tue, Oct 4, 2011 at 9:02 PM, Maxim Kuvyrkov <ma...@codesourcery.com> wrote: > > On 5/10/2011, at 1:49 AM, Richard Guenther wrote: > > > >> On Tue, Oct 4, 2011 at 9:17 AM, Maxim Kuvyrkov <ma...@codesourcery.com> > >> wrote: > >>> Richard, > >>> > >>> The following patch fixes a CFG consistency problem. > >>> > >>> When early IPA optimizations (e.g., early SRA) create a version of a > >>> function that no longer throws, versioning machinery deletes EH landings > >>> pads in _callers_ of the function [*]. However, the EH cfg edges are not > >>> updated in the callers, which eventually leads to an ICE in > >>> verify_eh_edges. > >> > >> It needs to update EH edges in the callers via the common idiom > >> > >> if (maybe_clean_or_replace_eh_stmt (stmt, stmt) > >> && gimple_purge_dead_eh_edges (gimple_bb (stmt))) > >> ... arrange for cfg-cleanup to be run ... > >> > >> where the cfg-cleanup part probably explains why it doesn't. So one option > >> is to not delete the landing pads. > > > > There are several places in the compiler where code does not the above > > idiom and calls maybe_clean[_or_replace]_eh_stmt without following it up > > with cfg-cleanup. > > > >> Where does it do that? > > > > The cases that I investigated are: > > - cgraphunit.c: update_call_expr(). The right thing to do /seems/ to be to > > remove call to maybe_clean_eh_stmt_fn(). > > Ok, that simply replaces the fndecl of a call statement, so yes, removing > the maybe_clean_eh_stmt_fn call should fix this case. But then you might > run into IL verification issues that you have a stmt with EH info that cannot > throw ... > > Basically these kind of local IPA passes are bad :/
This happens at materialization. Basically we should 1) materialize, 2) redirect callers and render some blocks unreachable, 3) inline, 4) cleanup cfg and get rid of them before updating SSA form. So it seems safe to me. Honza > > > - ipa-prop.c: ipa_modify_call_arguments() -> gsi_replace(). The right > > thing to do /seems/ to be to add follow up call to > > gimple_purge_dead_eh_edges(). > > I'd say call gsi_replace with , false and copy EH info manually and > unconditionally here. > > > Both cases above are triggered from early SRA. > > But maybe also used by other real IPA passes? > > > The other places that appear not to update CFG after calling > > maybe_clean_or_replace_eh_stmt are: > > - tree-cfg.c: replace_uses_by() > > - tree-ssa-dce.c: eliminate_unnecessary_stmts(). > > > >> I suppose it > >> merely fails to properly transfer the EH info from the old to the new call > >> (thus, fails to forcefully do what maybe_duplicate_eh_stmt does). > > > > I don't follow you here. The EH CFG edges that get out-of-date are in > > _caller_ function. It is the _callee_ function that is duplicated. > > Yes, what I mean is if a stmt is replaced in the caller then it will not > copy EH info if the new call cannot throw. But as we are not updating > the CFG we have to preserve EH info nevertheless (and allow "dead" > EH info in the verifiers). > > The alternative is to, for example in update_call_expr (), remove dead > EH edges, switch cfun to the caller context, call cfg_cleanup, switch > back. Expensive (but maybe doesn't happen often enough to worry?). > > I wonder why we end up, at IPA SRA time, knowing the cloned function > won't throw anymore. I suppose we don't. Do we? > > Thanks, > Richard. > > > Thank you, > > > > -- > > Maxim Kuvyrkov > > CodeSourcery / Mentor Graphics > > > >
