On Tue, 25 Mar 2025, Jakub Jelinek wrote:
> Hi!
>
> The following testcases FAIL because musttail failures are diagnosed
> not just in the tailc or musttail passes, but also during the tailr1
> and tailr2.
> tailr1 pass is before IPA and in the testcases eh cleanup has not
> cleaned up the IL sufficiently yet to make the musttail calls pass,
> even tailr2 could be too early.
>
> The following patch does that only during the tailc pass, and if that
> pass is not actually executed, during musttail pass.
> To do it only in the tailc pass, I chose to pass a new bool flag, because
> while we have the opt_tailcalls argument, it is actually passed by reference
> to find_tail_calls and sometimes cleared during that.
> musttail calls when the new DIAG_MUSTTAIL flag is not set are handled like
> any other calls, we simply silently punt on those if they can't be turned
> into tail calls.
>
> Furthermore, I had to tweak the musttail pass gate. Previously it was
> !flag_optimize_sibling_calls && f->has_musttail. The problem is that
> gate of tailr and tailc passes is
> flag_optimize_sibling_calls != 0 && dbg_cnt (tail_call)
> and furthermore, tailc pass is only in the normal optimization queue,
> so only if not -O0 or -Og. So when one would use tail_call dbg_cnt
> with some limit, or when e.g. using -foptimize-sibling-calls with -O0 or
> -Og, nothing would actually diagnose invalid musttail calls or set tail call
> flags on those if they are ok. I could insert a new PROP_ flag on whether
> musttail has been handled by tailc pass, but given that we have the
> cfun->has_musttail flag already and nothing after tailc/musttail passes uses
> it, I think it is easier to just clear the flag when musttail failures are
> diagnosed and correct ones have [[tail call]] flag added. Expansion will
> then only look at the [[tail call]] flag, it could even at the [[must tail
> call]] flag, but I don't see a point to check cfun->has_musttail.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2025-03-25 Jakub Jelinek <ja...@redhat.com>
>
> PR ipa/119376
> * tree-tailcall.cc (suitable_for_tail_opt_p): Add DIAG_MUSTTAIL
> argument, propagate it down to maybe_error_musttail.
> (suitable_for_tail_call_opt_p): Likewise.
> (maybe_error_musttail): Add DIAG_MUSTTAIL argument. Don't emit error
> for gimple_call_must_tail_p calls if it is false.
> (find_tail_calls): Add DIAG_MUSTTAIL argument, propagate it down to
> maybe_error_musttail, suitable_for_tail_opt_p,
> suitable_for_tail_call_opt_p and find_tail_calls calls.
> (tree_optimize_tail_calls_1): Add DIAG_MUSTTAIL argument, propagate
> it down to find_tail_calls and if set, clear cfun->has_musttail flag
> at the end. Rename OPT_MUSTCALL argument to OPT_MUSTTAIL.
> (execute_tail_calls): Pass true to DIAG_MUSTTAIL
> tree_optimize_tail_calls_1 argument.
> (pass_tail_recursion::execute): Pass false to DIAG_MUSTTAIL
> tree_optimize_tail_calls_1 argument.
> (pass_musttail::gate): Don't test flag_optimize_sibling_calls.
> (pass_musttail::execute): Pass true to DIAG_MUSTTAIL
> tree_optimize_tail_calls_1 argument.
>
> * g++.dg/torture/musttail1.C: New test.
> * g++.dg/opt/musttail2.C: New test.
>
> --- gcc/tree-tailcall.cc.jj 2025-01-16 09:27:53.645909094 +0100
> +++ gcc/tree-tailcall.cc 2025-03-24 12:51:56.271628242 +0100
> @@ -139,18 +139,18 @@ static tree m_acc, a_acc;
>
> static bitmap tailr_arg_needs_copy;
>
> -static void maybe_error_musttail (gcall *call, const char *err);
> +static void maybe_error_musttail (gcall *call, const char *err, bool);
>
> /* Returns false when the function is not suitable for tail call optimization
> from some reason (e.g. if it takes variable number of arguments). CALL
> is call to report for. */
>
> static bool
> -suitable_for_tail_opt_p (gcall *call)
> +suitable_for_tail_opt_p (gcall *call, bool diag_musttail)
> {
> if (cfun->stdarg)
> {
> - maybe_error_musttail (call, _("caller uses stdargs"));
> + maybe_error_musttail (call, _("caller uses stdargs"), diag_musttail);
> return false;
> }
>
> @@ -163,7 +163,7 @@ suitable_for_tail_opt_p (gcall *call)
> tail call discovery happen. CALL is call to report error for. */
>
> static bool
> -suitable_for_tail_call_opt_p (gcall *call)
> +suitable_for_tail_call_opt_p (gcall *call, bool diag_musttail)
> {
> tree param;
>
> @@ -171,7 +171,7 @@ suitable_for_tail_call_opt_p (gcall *cal
> sibling call optimizations, but not tail recursion. */
> if (cfun->calls_alloca)
> {
> - maybe_error_musttail (call, _("caller uses alloca"));
> + maybe_error_musttail (call, _("caller uses alloca"), diag_musttail);
> return false;
> }
>
> @@ -181,7 +181,8 @@ suitable_for_tail_call_opt_p (gcall *cal
> if (targetm_common.except_unwind_info (&global_options) == UI_SJLJ
> && current_function_has_exception_handlers ())
> {
> - maybe_error_musttail (call, _("caller uses sjlj exceptions"));
> + maybe_error_musttail (call, _("caller uses sjlj exceptions"),
> + diag_musttail);
> return false;
> }
>
> @@ -190,7 +191,7 @@ suitable_for_tail_call_opt_p (gcall *cal
> properly in the CFG so that this needn't be special cased. */
> if (cfun->calls_setjmp)
> {
> - maybe_error_musttail (call, _("caller uses setjmp"));
> + maybe_error_musttail (call, _("caller uses setjmp"), diag_musttail);
> return false;
> }
>
> @@ -198,7 +199,8 @@ suitable_for_tail_call_opt_p (gcall *cal
> that call __builtin_eh_return. */
> if (cfun->calls_eh_return)
> {
> - maybe_error_musttail (call, _("caller uses __builtin_eh_return"));
> + maybe_error_musttail (call, _("caller uses __builtin_eh_return"),
> + diag_musttail);
> return false;
> }
>
> @@ -209,7 +211,8 @@ suitable_for_tail_call_opt_p (gcall *cal
> param = DECL_CHAIN (param))
> if (TREE_ADDRESSABLE (param))
> {
> - maybe_error_musttail (call, _("address of caller arguments taken"));
> + maybe_error_musttail (call, _("address of caller arguments taken"),
> + diag_musttail);
> return false;
> }
>
> @@ -436,9 +439,9 @@ propagate_through_phis (tree var, edge e
> errors. */
>
> static void
> -maybe_error_musttail (gcall *call, const char *err)
> +maybe_error_musttail (gcall *call, const char *err, bool diag_musttail)
> {
> - if (gimple_call_must_tail_p (call))
> + if (gimple_call_must_tail_p (call) && diag_musttail)
> {
> error_at (call->location, "cannot tail-call: %s", err);
> /* Avoid another error. ??? If there are multiple reasons why tail
> @@ -461,12 +464,13 @@ static live_vars_map *live_vars;
> static vec<bitmap_head> live_vars_vec;
>
> /* Finds tailcalls falling into basic block BB. The list of found tailcalls
> is
> - added to the start of RET. When ONLY_MUSTTAIL is set only handle musttail.
> - Update OPT_TAILCALLS as output parameter. */
> + added to the start of RET. When ONLY_MUSTTAIL is set only handle
> musttail.
> + Update OPT_TAILCALLS as output parameter. If DIAG_MUSTTAIL, diagnose
> + failures for musttail calls. */
>
> static void
> find_tail_calls (basic_block bb, struct tailcall **ret, bool only_musttail,
> - bool &opt_tailcalls)
> + bool &opt_tailcalls, bool diag_musttail)
> {
> tree ass_var = NULL_TREE, ret_var, func, param;
> gimple *stmt;
> @@ -524,7 +528,7 @@ find_tail_calls (basic_block bb, struct
> {
> maybe_error_musttail (call,
> _("memory reference or volatile after "
> - "call"));
> + "call"), diag_musttail);
> return;
> }
> ass_var = gimple_call_lhs (call);
> @@ -561,15 +565,16 @@ find_tail_calls (basic_block bb, struct
> edge_iterator ei;
> /* Recurse to the predecessors. */
> FOR_EACH_EDGE (e, ei, bb->preds)
> - find_tail_calls (e->src, ret, only_musttail, opt_tailcalls);
> + find_tail_calls (e->src, ret, only_musttail, opt_tailcalls,
> + diag_musttail);
>
> return;
> }
>
> - if (!suitable_for_tail_opt_p (call))
> + if (!suitable_for_tail_opt_p (call, diag_musttail))
> return;
>
> - if (!suitable_for_tail_call_opt_p (call))
> + if (!suitable_for_tail_call_opt_p (call, diag_musttail))
> opt_tailcalls = false;
>
> /* If the LHS of our call is not just a simple register or local
> @@ -587,13 +592,13 @@ find_tail_calls (basic_block bb, struct
> && !is_gimple_reg (ass_var)
> && !auto_var_in_fn_p (ass_var, cfun->decl))
> {
> - maybe_error_musttail (call, _("return value in memory"));
> + maybe_error_musttail (call, _("return value in memory"),
> diag_musttail);
> return;
> }
>
> if (cfun->calls_setjmp)
> {
> - maybe_error_musttail (call, _("caller uses setjmp"));
> + maybe_error_musttail (call, _("caller uses setjmp"), diag_musttail);
> return;
> }
>
> @@ -605,9 +610,10 @@ find_tail_calls (basic_block bb, struct
> if (stmt == last_stmt)
> maybe_error_musttail (call,
> _("call may throw exception that does not "
> - "propagate"));
> + "propagate"), diag_musttail);
> else
> - maybe_error_musttail (call, _("code between call and return"));
> + maybe_error_musttail (call, _("code between call and return"),
> + diag_musttail);
> return;
> }
>
> @@ -639,7 +645,8 @@ find_tail_calls (basic_block bb, struct
> && may_be_aliased (result_decl)
> && ref_maybe_used_by_stmt_p (call, result_decl, false))
> {
> - maybe_error_musttail (call, _("return value used after call"));
> + maybe_error_musttail (call, _("return value used after call"),
> + diag_musttail);
> return;
> }
>
> @@ -723,7 +730,8 @@ find_tail_calls (basic_block bb, struct
> if (local_live_vars)
> BITMAP_FREE (local_live_vars);
> maybe_error_musttail (call,
> - _("call invocation refers to locals"));
> + _("call invocation refers to locals"),
> + diag_musttail);
> return;
> }
> else
> @@ -733,7 +741,8 @@ find_tail_calls (basic_block bb, struct
> {
> BITMAP_FREE (local_live_vars);
> maybe_error_musttail (call,
> - _("call invocation refers to locals"));
> + _("call invocation refers to locals"),
> + diag_musttail);
> return;
> }
> }
> @@ -780,7 +789,8 @@ find_tail_calls (basic_block bb, struct
>
> if (gimple_code (stmt) != GIMPLE_ASSIGN)
> {
> - maybe_error_musttail (call, _("unhandled code after call"));
> + maybe_error_musttail (call, _("unhandled code after call"),
> + diag_musttail);
> return;
> }
>
> @@ -789,7 +799,8 @@ find_tail_calls (basic_block bb, struct
> &tmp_m, &tmp_a, &ass_var, to_move_defs);
> if (ret == FAIL || (ret == TRY_MOVE && !tail_recursion))
> {
> - maybe_error_musttail (call, _("return value changed after call"));
> + maybe_error_musttail (call, _("return value changed after call"),
> + diag_musttail);
> return;
> }
> else if (ret == TRY_MOVE)
> @@ -864,7 +875,8 @@ find_tail_calls (basic_block bb, struct
> if (!ok)
> {
> maybe_error_musttail (call,
> - _("call and return value are different"));
> + _("call and return value are different"),
> + diag_musttail);
> return;
> }
> }
> @@ -874,7 +886,8 @@ find_tail_calls (basic_block bb, struct
> if (!tail_recursion && (m || a))
> {
> maybe_error_musttail (call,
> - _("operations after non tail recursive call"));
> + _("operations after non tail recursive call"),
> + diag_musttail);
> return;
> }
>
> @@ -883,7 +896,7 @@ find_tail_calls (basic_block bb, struct
> {
> maybe_error_musttail (call,
> _("tail recursion with pointers can only use "
> - "additions"));
> + "additions"), diag_musttail);
> return;
> }
>
> @@ -1258,11 +1271,13 @@ create_tailcall_accumulator (const char
> }
>
> /* Optimizes tail calls in the function, turning the tail recursion
> - into iteration. When ONLY_MUSTCALL is true only optimize mustcall
> - marked calls. */
> + into iteration. When ONLY_MUSTTAIL is true only optimize musttail
> + marked calls. When DIAG_MUSTTAIL, diagnose if musttail calls can't
> + be tail call optimized. */
>
> static unsigned int
> -tree_optimize_tail_calls_1 (bool opt_tailcalls, bool only_mustcall)
> +tree_optimize_tail_calls_1 (bool opt_tailcalls, bool only_musttail,
> + bool diag_musttail)
> {
> edge e;
> bool phis_constructed = false;
> @@ -1277,7 +1292,8 @@ tree_optimize_tail_calls_1 (bool opt_tai
> /* Only traverse the normal exits, i.e. those that end with return
> statement. */
> if (safe_is_a <greturn *> (*gsi_last_bb (e->src)))
> - find_tail_calls (e->src, &tailcalls, only_mustcall, opt_tailcalls);
> + find_tail_calls (e->src, &tailcalls, only_musttail, opt_tailcalls,
> + diag_musttail);
> }
>
> if (live_vars)
> @@ -1374,6 +1390,9 @@ tree_optimize_tail_calls_1 (bool opt_tai
> if (tailr_arg_needs_copy)
> BITMAP_FREE (tailr_arg_needs_copy);
>
> + if (diag_musttail)
> + cfun->has_musttail = false;
> +
> if (changed)
> return TODO_cleanup_cfg | TODO_update_ssa_only_virtuals;
> return 0;
> @@ -1388,7 +1407,7 @@ gate_tail_calls (void)
> static unsigned int
> execute_tail_calls (void)
> {
> - return tree_optimize_tail_calls_1 (true, false);
> + return tree_optimize_tail_calls_1 (true, false, true);
> }
>
> namespace {
> @@ -1421,7 +1440,7 @@ public:
> bool gate (function *) final override { return gate_tail_calls (); }
> unsigned int execute (function *) final override
> {
> - return tree_optimize_tail_calls_1 (false, false);
> + return tree_optimize_tail_calls_1 (false, false, false);
> }
>
> }; // class pass_tail_recursion
> @@ -1497,15 +1516,15 @@ public:
>
> /* opt_pass methods: */
> /* This pass is only used when the other tail call pass
> - doesn't run to make [[musttail]] still work. But only
> + doesn't run to make [[musttail]] still work. But only
> run it when there is actually a musttail in the function. */
> bool gate (function *f) final override
> {
> - return !flag_optimize_sibling_calls && f->has_musttail;
> + return f->has_musttail;
> }
> unsigned int execute (function *) final override
> {
> - return tree_optimize_tail_calls_1 (true, true);
> + return tree_optimize_tail_calls_1 (true, true, true);
> }
>
> }; // class pass_musttail
> --- gcc/testsuite/g++.dg/torture/musttail1.C.jj 2025-03-24
> 12:55:09.092982548 +0100
> +++ gcc/testsuite/g++.dg/torture/musttail1.C 2025-03-24 12:56:24.154952628
> +0100
> @@ -0,0 +1,15 @@
> +// PR ipa/119376
> +// { dg-do compile { target musttail } }
> +// { dg-additional-options "-ffat-lto-objects -fdump-tree-optimized" }
> +/* { dg-final { scan-tree-dump-times " \[^\n\r]* = foo \\\(\[^\n\r]*\\\);
> \\\[tail call\\\] \\\[must tail call\\\]" 1 "optimized" } } */
> +
> +struct S { int s; };
> +int foo (int);
> +
> +int
> +bar (int a)
> +{
> + S b = {a};
> + b.s++;
> + [[gnu::musttail]] return foo (a);
> +}
> --- gcc/testsuite/g++.dg/opt/musttail2.C.jj 2025-03-24 13:27:44.329204196
> +0100
> +++ gcc/testsuite/g++.dg/opt/musttail2.C 2025-03-24 13:28:08.975867389
> +0100
> @@ -0,0 +1,14 @@
> +// PR ipa/119376
> +// { dg-do compile { target musttail } }
> +// { dg-options "-O2 -fno-early-inlining -fdump-tree-optimized" }
> +// { dg-final { scan-tree-dump-times " \[^\n\r]* = foo \\\(\[^\n\r]*\\\);
> \\\[tail call\\\] \\\[must tail call\\\]" 1 "optimized" } }
> +
> +struct S { S () {} };
> +char *foo (S);
> +
> +char *
> +bar (S)
> +{
> + S t;
> + [[clang::musttail]] return foo (t);
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
