> Am 02.04.2025 um 18:10 schrieb Jakub Jelinek <ja...@redhat.com>:
>
> Hi!
>
> The following testcases FAIL, because EH cleanup is performed only before
> IPA and then right before musttail pass.
> At -O2 etc. (except for -O0/-Og) we handle musttail calls in the tailc
> pass though, and we can fail at that point because the calls might appear
> to throw internal exceptions which just don't do anything interesting
> (perhaps have debug statements or clobber statements in them) before they
> continue with resume of the exception (i.e. throw it externally).
>
> As Richi said in the PR (and I agree) that moving passes is risky at this
> point, the following patch instead teaches the tail{r,c} and musttail
> passes to deal with such extra EDGE_EH edges.
>
> It is fairly simple thing, if we see an EDGE_EH edge from the call we
> just look up where it lands and if there are no
> non-debug/non-clobber/non-label statements before resx which throws
> externally, such edge can be ignored for tail call optimization or
> tail recursion. At other spots I just need to avoid using
> single_succ/single_succ_edge because the bb might have another edge -
> EDGE_EH.
>
> To make this less risky, this is done solely for the musttail calls for now.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok
Richard
>
> 2025-04-02 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/119491
> * tree-tailcall.cc (single_non_eh_succ_edge): New function.
> (independent_of_stmt_p): Use single_non_eh_succ_edge (bb)->dest
> instead of single_succ (bb).
> (empty_eh_cleanup): New function.
> (find_tail_calls): Diagnose throwing of exceptions which do not
> propagate only if there are no EDGE_EH successor edges. If there are
> and the call is musttail, use empty_eh_cleanup to find if the cleanup
> is not empty. If not or the call is not musttail, use different
> diagnostics. Set is_noreturn even if there are successor edges. Use
> single_non_eh_succ_edge (abb) instead of single_succ_edge (abb). Punt
> on internal noreturn calls.
> (decrease_profile): Don't assert 0 or 1 successor edges.
> (eliminate_tail_call): Use
> single_non_eh_succ_edge (gsi_bb (t->call_gsi)) instead of
> single_succ_edge (gsi_bb (t->call_gsi)).
> (tree_optimize_tail_calls_1): Also look into basic blocks with
> single succ edge which is EDGE_EH for noreturn musttail calls.
>
> * g++.dg/opt/musttail3.C: New test.
> * g++.dg/opt/musttail4.C: New test.
> * g++.dg/opt/musttail5.C: New test.
>
> --- gcc/tree-tailcall.cc.jj 2025-04-01 16:47:30.373502796 +0200
> +++ gcc/tree-tailcall.cc 2025-04-02 09:02:35.572760732 +0200
> @@ -219,6 +219,23 @@ suitable_for_tail_call_opt_p (gcall *cal
> return true;
> }
>
> +/* Return single successor edge ignoring EDGE_EH edges. */
> +
> +static edge
> +single_non_eh_succ_edge (basic_block bb)
> +{
> + edge e, ret = NULL;
> + edge_iterator ei;
> + FOR_EACH_EDGE (e, ei, bb->succs)
> + if ((e->flags & EDGE_EH) == 0)
> + {
> + gcc_assert (ret == NULL);
> + ret = e;
> + }
> + gcc_assert (ret);
> + return ret;
> +}
> +
> /* Checks whether the expression EXPR in stmt AT is independent of the
> statement pointed to by GSI (in a sense that we already know EXPR's value
> at GSI). We use the fact that we are only called from the chain of
> @@ -245,7 +262,7 @@ independent_of_stmt_p (tree expr, gimple
> /* Mark the blocks in the chain leading to the end. */
> at_bb = gimple_bb (at);
> call_bb = gimple_bb (gsi_stmt (gsi));
> - for (bb = call_bb; bb != at_bb; bb = single_succ (bb))
> + for (bb = call_bb; bb != at_bb; bb = single_non_eh_succ_edge (bb)->dest)
> bb->aux = &bb->aux;
> bb->aux = &bb->aux;
>
> @@ -289,7 +306,7 @@ independent_of_stmt_p (tree expr, gimple
> }
>
> /* Unmark the blocks. */
> - for (bb = call_bb; bb != at_bb; bb = single_succ (bb))
> + for (bb = call_bb; bb != at_bb; bb = single_non_eh_succ_edge (bb)->dest)
> bb->aux = NULL;
> bb->aux = NULL;
>
> @@ -462,6 +479,33 @@ maybe_error_musttail (gcall *call, const
> }
> }
>
> +/* Return true if there is no real work performed in the exception
> + path starting at BB and it will in the end result in external exception.
> + Search at most CNT basic blocks (so that we don't need to do trivial
> + loop discovery). */
> +static bool
> +empty_eh_cleanup (basic_block bb, int cnt)
> +{
> + if (EDGE_COUNT (bb->succs) > 1)
> + return false;
> +
> + for (gimple_stmt_iterator gsi = gsi_after_labels (bb); !gsi_end_p (gsi);
> + gsi_next (&gsi))
> + {
> + gimple *g = gsi_stmt (gsi);
> + if (is_gimple_debug (g) || gimple_clobber_p (g))
> + continue;
> + if (is_gimple_resx (g) && stmt_can_throw_external (cfun, g))
> + return true;
> + return false;
> + }
> + if (!single_succ_p (bb))
> + return false;
> + if (cnt == 1)
> + return false;
> + return empty_eh_cleanup (single_succ (bb), cnt - 1);
> +}
> +
> /* Argument for compute_live_vars/live_vars_at_stmt and what compute_live_vars
> returns. Computed lazily, but just once for the function. */
> static live_vars_map *live_vars;
> @@ -612,14 +656,36 @@ find_tail_calls (basic_block bb, struct
> if ((stmt_could_throw_p (cfun, stmt)
> && !stmt_can_throw_external (cfun, stmt)) || EDGE_COUNT (bb->succs) >
> 1)
> {
> - if (stmt == last_stmt)
> - maybe_error_musttail (call,
> - _("call may throw exception that does not "
> - "propagate"), diag_musttail);
> - else
> - maybe_error_musttail (call, _("code between call and return"),
> - diag_musttail);
> - return;
> + if (stmt != last_stmt)
> + {
> + maybe_error_musttail (call, _("code between call and return"),
> + diag_musttail);
> + return;
> + }
> +
> + edge e;
> + edge_iterator ei;
> + FOR_EACH_EDGE (e, ei, bb->succs)
> + if (e->flags & EDGE_EH)
> + break;
> +
> + if (!e)
> + {
> + maybe_error_musttail (call,
> + _("call may throw exception that does not "
> + "propagate"), diag_musttail);
> + return;
> + }
> +
> + if (!gimple_call_must_tail_p (call)
> + || !empty_eh_cleanup (e->dest, 20)
> + || EDGE_COUNT (bb->succs) > 2)
> + {
> + maybe_error_musttail (call,
> + _("call may throw exception caught locally "
> + "or perform cleanups"), diag_musttail);
> + return;
> + }
> }
>
> /* If the function returns a value, then at present, the tail call
> @@ -763,8 +829,7 @@ find_tail_calls (basic_block bb, struct
> a = NULL_TREE;
> auto_bitmap to_move_defs;
> auto_vec<gimple *> to_move_stmts;
> - bool is_noreturn
> - = EDGE_COUNT (bb->succs) == 0 && gimple_call_noreturn_p (call);
> + bool is_noreturn = gimple_call_noreturn_p (call);
>
> abb = bb;
> agsi = gsi;
> @@ -776,8 +841,9 @@ find_tail_calls (basic_block bb, struct
>
> while (gsi_end_p (agsi))
> {
> - ass_var = propagate_through_phis (ass_var, single_succ_edge (abb));
> - abb = single_succ (abb);
> + edge e = single_non_eh_succ_edge (abb);
> + ass_var = propagate_through_phis (ass_var, e);
> + abb = e->dest;
> agsi = gsi_start_bb (abb);
> }
>
> @@ -851,6 +917,11 @@ find_tail_calls (basic_block bb, struct
> /* See if this is a tail call we can handle. */
> if (is_noreturn)
> {
> + if (gimple_call_internal_p (call))
> + {
> + maybe_error_musttail (call, _("internal call"), diag_musttail);
> + return;
> + }
> tree rettype = TREE_TYPE (TREE_TYPE (current_function_decl));
> tree calltype = TREE_TYPE (gimple_call_fntype (call));
> if (!VOID_TYPE_P (rettype)
> @@ -1112,11 +1183,6 @@ static void
> decrease_profile (basic_block bb, profile_count count)
> {
> bb->count = bb->count - count;
> - if (!single_succ_p (bb))
> - {
> - gcc_assert (!EDGE_COUNT (bb->succs));
> - return;
> - }
> }
>
> /* Eliminates tail call described by T. TMP_VARS is a list of
> @@ -1181,7 +1247,7 @@ eliminate_tail_call (struct tailcall *t,
> else
> {
> /* Number of executions of function has reduced by the tailcall. */
> - e = single_succ_edge (gsi_bb (t->call_gsi));
> + e = single_non_eh_succ_edge (gsi_bb (t->call_gsi));
>
> profile_count count = e->count ();
>
> @@ -1196,8 +1262,7 @@ eliminate_tail_call (struct tailcall *t,
> decrease_profile (e->dest, count);
>
> /* Replace the call by a jump to the start of function. */
> - e = redirect_edge_and_branch (single_succ_edge (gsi_bb (t->call_gsi)),
> - first);
> + e = redirect_edge_and_branch (e, first);
> }
> gcc_assert (e);
> PENDING_STMT (e) = NULL;
> @@ -1362,7 +1427,9 @@ tree_optimize_tail_calls_1 (bool opt_tai
> {
> basic_block bb;
> FOR_EACH_BB_FN (bb, cfun)
> - if (EDGE_COUNT (bb->succs) == 0)
> + if (EDGE_COUNT (bb->succs) == 0
> + || (single_succ_p (bb)
> + && (single_succ_edge (bb)->flags & EDGE_EH)))
> if (gimple *c = last_nondebug_stmt (bb))
> if (is_gimple_call (c)
> && gimple_call_must_tail_p (as_a <gcall *> (c))
> --- gcc/testsuite/g++.dg/opt/musttail3.C.jj 2025-04-01 18:47:10.474945080
> +0200
> +++ gcc/testsuite/g++.dg/opt/musttail3.C 2025-04-01 18:49:27.063068029
> +0200
> @@ -0,0 +1,41 @@
> +// PR tree-optimization/119491
> +// { dg-do compile { target { external_musttail && c++11 } } }
> +// { dg-options "-O2" }
> +
> +struct A {
> + struct B {};
> + A () {}
> +};
> +void qux ();
> +unsigned char v;
> +A w;
> +void foo (A);
> +
> +template <typename T>
> +[[gnu::always_inline]] static inline void
> +bar (int &)
> +{
> +}
> +
> +[[gnu::always_inline]] static inline void
> +baz (int *)
> +{
> + int r = 0;
> + bar<int> (r);
> +}
> +
> +[[gnu::always_inline]] inline void
> +corge (A)
> +{
> + if (v)
> + qux ();
> + [[gnu::musttail]] return foo (w);
> +}
> +
> +void
> +freddy (A)
> +{
> + int t;
> + baz (&t);
> + [[gnu::musttail]] return corge (A{});
> +}
> --- gcc/testsuite/g++.dg/opt/musttail4.C.jj 2025-04-01 19:10:56.389350911
> +0200
> +++ gcc/testsuite/g++.dg/opt/musttail4.C 2025-04-01 19:28:18.285020409
> +0200
> @@ -0,0 +1,35 @@
> +// { dg-do compile { target { external_musttail && c++11 } } }
> +// { dg-options "-O2 -fexceptions" }
> +
> +struct S { ~S (); };
> +volatile int v;
> +struct T { ~T () { v = v + 1; } };
> +struct U { ~U () {} };
> +int foo ();
> +
> +int
> +bar () noexcept
> +{
> + [[gnu::musttail]] return foo (); // { dg-error "cannot tail-call: call
> may throw exception that does not propagate" }
> +}
> +
> +int
> +baz ()
> +{
> + S s;
> + [[gnu::musttail]] return foo (); // { dg-error "cannot tail-call: other
> reasons" }
> +}
> +
> +int
> +qux ()
> +{
> + T t;
> + [[gnu::musttail]] return foo (); // { dg-error "cannot tail-call: other
> reasons" }
> +}
> +
> +int
> +corge ()
> +{
> + U u;
> + [[gnu::musttail]] return foo ();
> +}
> --- gcc/testsuite/g++.dg/opt/musttail5.C.jj 2025-04-01 19:14:50.981127712
> +0200
> +++ gcc/testsuite/g++.dg/opt/musttail5.C 2025-04-01 19:11:25.249954382
> +0200
> @@ -0,0 +1,41 @@
> +// PR tree-optimization/119491
> +// { dg-do compile { target { external_musttail && c++11 } } }
> +// { dg-options "-O2" }
> +
> +struct A {
> + struct B {};
> + A () {}
> +};
> +void qux ();
> +unsigned char v;
> +A w;
> +[[noreturn]] void foo (A);
> +
> +template <typename T>
> +[[gnu::always_inline]] static inline void
> +bar (int &)
> +{
> +}
> +
> +[[gnu::always_inline]] static inline void
> +baz (int *)
> +{
> + int r = 0;
> + bar<int> (r);
> +}
> +
> +[[gnu::always_inline]] inline void
> +corge (A)
> +{
> + if (v)
> + qux ();
> + [[gnu::musttail]] return foo (w);
> +}
> +
> +void
> +freddy (A)
> +{
> + int t;
> + baz (&t);
> + [[gnu::musttail]] return corge (A{});
> +}
>
> Jakub
>
