On Tue, Oct 1, 2019 at 1:05 PM Eric Botcazou <ebotca...@adacore.com> wrote: > > [Thanks for the quick review and sorry for the longish delay] > > > +/* Return the index number of the landing pad for STMT, if any. */ > > + > > +static int > > +lp_nr_for_store (gimple *stmt) > > +{ > > + if (!cfun->can_throw_non_call_exceptions || !cfun->eh) > > + return 0; > > + > > + if (!stmt_could_throw_p (cfun, stmt)) > > + return 0; > > + > > + return lookup_stmt_eh_lp (stmt); > > +} > > > > Did you add the wrapper as compile-time optimization? That is, > > I don't see why simply calling lookup_stmt_eh_lp wouldn't work? > > Yes, I added it for C & C++, which both trivially fail the first test. More > generally, every additional processing is (directly or indirectly) guarded by > the conjunction cfun->can_throw_non_call_exceptions && cfun->eh throughout. > > > + /* If the function can throw and catch non-call exceptions, we'll be > > trying + to merge stores across different basic blocks so we need to > > first unsplit + the EH edges in order to streamline the CFG of the > > function. */ + if (cfun->can_throw_non_call_exceptions && cfun->eh) > > + { > > + free_dominance_info (CDI_DOMINATORS); > > + maybe_remove_unreachable_handlers (); > > + changed = unsplit_all_eh (); > > + if (changed) > > + delete_unreachable_blocks (); > > + } > > > > uh, can unsplitting really result in unreachable blocks or does it > > merely forget to delete forwarders it made unreachable? > > The latter. > > > Removing unreachable handlers is also to make things match better? > > Nope, only because calculate_dominance_info aborts otherwise below. > > > Just wondering how much of this work we could delay to the first > > store-merging opportunity with EH we find (but I don't care too much > > about -fnon-call-exceptions). > > This block of code is a manual, stripped down ehcleanup pass. > > > To isolate the details above maybe move this piece into a helper > > in tree-eh.c so you also can avoid exporting unsplit_all_eh? > > The main point is the unsplitting though so this would trade an explicit call > for a less implicit one. But what I could do is to rename unsplit_all_eh into > unsplit_all_eh_1 and hide the technicalities in a new unsplit_all_eh.
that works for me - the patch is OK with that change. Thanks, Richard. > -- > Eric Botcazou