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