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

[Richard Guenther <richard dot guenther at gmail dot com>]

On Tue, Dec 13, 2011 at 1:00 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> 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.

Yes, I've seen that.  I think it's ugly and inconsistent - we can
certaily fold non-call stmts from throwing to non-throwing.
Think of -fnon-call-exceptions and x + 1.3 and folding after
propagating 1.0 to x.

It is not fold_stmts business to care about EH info or edges
(it works on statements in isolation).

>> 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.

I think all callers can (should be able to) do that.  In the light of
-fnon-call-exceptions we probably miss quite a few - but we should
fix them.

[Unfortunately some of our propagation helpers do not, or do not have
a good way of returning whether EH/CFG cleanup is necessary,
notably gsi_remove, which removes the stmt from the EH tables
but does not purge edges.]

Richard.