Re: [PATCH] Call maybe_clean_or_redirect_eh_stmt in gimple_fold_call (PR tree-optimization/51481)

[Jakub Jelinek <jakub at redhat dot com>]

On Tue, Dec 13, 2011 at 11:52:38AM +0100, Richard Guenther wrote:
> On Mon, Dec 12, 2011 at 6:01 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > In gimple_fold_call (called from fold_stmt from replace_uses_by from cfg
> > cleanup) we weren't calling maybe_clean_or_replace_eh_stmt, so when
> > we've replaced a printf call (which can throw) with puts (which can throw
> > too), nothing would update EH stmts. ?It would be problematic calling
> > gimple_purge_dead_eh_edges, because callers might be surprised by that,
> > especially when this happens during cfg cleanup, so instead I just assert
> > it is not needed and don't try to fold if a throwing stmt would be replaced
> > by non-throwing. ?FAB pass can handle that instead. ?No folding
> > has been actually disabled because of that check during bootstrap/regtest,
> > so it is there just in case.

Note, I've already committed the patch yesterday.

> I think that all callers of fold_stmt are supposed to handle EH
> updating themselves, similar to how they are supposed to
> call update_stmt.  I see that replace_uses_by does call

Some do something, but others don't.  Do you think it is preferable when
the callers do that (all of them)?  Even if that is chosen, while some could
purge dead eh edges, other places can't do that IMHO, so either we need to
simply not do transformations that remove EH edges in fold_stmt, or have an
argument that is passed down whether it is ok to do so.

> maybe_clean_or_replace_eh_stmt - are you sure it is this path
> the issue triggers on?

Yes, I'm sure this was in replace_uses_by.  Apparently it calls
maybe_clean_or_replace_eh_stmt, but incorrectly (similarly forwprop in some
cases doesn't call it at all, in other places incorrectly):

  maybe_clean_or_replace_eh_stmt (stmt, stmt);

This does nothing.  It must be called with the old stmt and new stmt it was
replaced with, otherwise, when it is called just with new stmt as in this
place lookup_stmt_eh_lp will just return 0 (unless fold_stmt did call it already
properly).

	Jakub