> Am 11.04.2025 um 21:00 schrieb Jakub Jelinek <ja...@redhat.com>:
>
> Hi!
>
> The following patch makes some adjustments so that users can analyze what
> calls weren't tail called even without using musttail attribute (though I'm
> still not convinced it should be a warning, because we don't distinguish
> between calls in return call (...); statements vs. calls that just happened
> to end up in tail positions because something has been optimized away etc.
>
> E.g. for
> int foo (int, int, int, int, int, int, int);
> int bar (int);
> void qux (int *);
>
> int
> baz (int x)
> {
> if (x)
> return foo (1, 2, 3, 4, 5, 6, 7);
> else
> {
> int y;
> qux (&y);
> return bar (x);
> }
> }
> ./xgcc -B ./ -S -O2 -fdump-{tree-tailc,rtl-expand}-details pr119718.c ; grep
> -B1 '^\(;; \)\?Cannot tail-call:' pr119718.c.*
> pr119718.c.222t.tailc-_7 = bar (0);
> pr119718.c.222t.tailc:Cannot tail-call: call invocation refers to locals
> --
> pr119718.c.270r.expand-;; foo (1, 2, 3, 4, 5, 6, 7) [tail call]
> pr119718.c.270r.expand:;; Cannot tail-call: callee required more stack slots
> than the caller
>
> The changes are:
> 1) in tailc pass use wording more consistent with the musttail error wording
> 2) do it only in *-details dump
> 3) add similar diagnostics on the expand side, but this time only for the
> CALL_EXPR_TAILCALL calls, if something wasn't marked that way, it is up
> to tailc pass to emit message about it, if it was and it still can't be
> tail called, let it tell users about that; in this case I need
> to use the ;; prefix because it will appear in the middle of
> the IL dump and ;; is what is used for such purposes in other spots
> 4) I've tried to improve formatting of the maybe_error_musttail and
> maybe_complain_about_tail_call calls
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK, but it might be a good match for -foot-info reporting as well with
MSG_MISSED_OPTIMIZATION? I suppose for less ‚false positives‘ we’d have to
record whether a call was in tail call position originally (whether it could
have had a musttail attribute).
Richard
> 2025-04-11 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/119718
> * tree-tailcall.cc (maybe_error_musttail): Only dump into dump_file
> if dump_flags & TDF_DETAILS. Use "Cannot tail-call: " prefix instead
> of "Cannot convert: ".
> (find_tail_calls, tree_optimize_tail_calls_1): Formatting fixes
> for maybe_error_musttail calls.
> * calls.cc (maybe_complain_about_tail_call): Emit also a message
> into dump_file when dump_flags & TDF_DETAILS for CALL_EXPR_TAILCALL
> calls.
> (initialize_argument_information): Formatting fix for
> maybe_complain_about_tail_call calls.
> (can_implement_as_sibling_call_p, expand_call): Likewise.
>
> --- gcc/tree-tailcall.cc.jj 2025-04-08 14:09:32.189711950 +0200
> +++ gcc/tree-tailcall.cc 2025-04-11 09:38:22.483257379 +0200
> @@ -492,10 +492,10 @@ maybe_error_musttail (gcall *call, const
> gimple_call_set_must_tail (call, false); /* Avoid another error. */
> gimple_call_set_tail (call, false);
> }
> - if (dump_file)
> + if (dump_file && (dump_flags & TDF_DETAILS))
> {
> print_gimple_stmt (dump_file, call, 0, TDF_SLIM);
> - fprintf (dump_file, "Cannot convert: %s\n", err);
> + fprintf (dump_file, "Cannot tail-call: %s\n", err);
> }
> }
>
> @@ -596,9 +596,8 @@ find_tail_calls (basic_block bb, struct
> return;
> if (bad_stmt)
> {
> - maybe_error_musttail (call,
> - _("memory reference or volatile after "
> - "call"), diag_musttail);
> + maybe_error_musttail (call, _("memory reference or volatile "
> + "after call"), diag_musttail);
> return;
> }
> ass_var = gimple_call_lhs (call);
> @@ -711,9 +710,8 @@ find_tail_calls (basic_block bb, struct
>
> if (!e)
> {
> - maybe_error_musttail (call,
> - _("call may throw exception that does not "
> - "propagate"), diag_musttail);
> + maybe_error_musttail (call, _("call may throw exception that does not "
> + "propagate"), diag_musttail);
> return;
> }
>
> @@ -721,9 +719,9 @@ find_tail_calls (basic_block bb, struct
> || !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);
> + maybe_error_musttail (call, _("call may throw exception caught "
> + "locally or perform cleanups"),
> + diag_musttail);
> return;
> }
> }
> @@ -854,9 +852,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"),
> - diag_musttail);
> + maybe_error_musttail (call, _("call invocation refers to "
> + "locals"), diag_musttail);
> return;
> }
> else
> @@ -883,9 +880,8 @@ find_tail_calls (basic_block bb, struct
> continue;
> }
> BITMAP_FREE (local_live_vars);
> - maybe_error_musttail (call,
> - _("call invocation refers to locals"),
> - diag_musttail);
> + maybe_error_musttail (call, _("call invocation refers to "
> + "locals"), diag_musttail);
> return;
> }
> }
> @@ -1020,8 +1016,7 @@ find_tail_calls (basic_block bb, struct
> if (!VOID_TYPE_P (rettype)
> && !useless_type_conversion_p (rettype, calltype))
> {
> - maybe_error_musttail (call,
> - _("call and return value are different"),
> + maybe_error_musttail (call, _("call and return value are different"),
> diag_musttail);
> return;
> }
> @@ -1092,8 +1087,7 @@ find_tail_calls (basic_block bb, struct
> }
> if (!ok)
> {
> - maybe_error_musttail (call,
> - _("call and return value are different"),
> + maybe_error_musttail (call, _("call and return value are different"),
> diag_musttail);
> return;
> }
> @@ -1103,18 +1097,16 @@ find_tail_calls (basic_block bb, struct
> multiplicands. */
> if (!tail_recursion && (m || a))
> {
> - maybe_error_musttail (call,
> - _("operations after non tail recursive call"),
> - diag_musttail);
> + maybe_error_musttail (call, _("operations after non tail recursive "
> + "call"), diag_musttail);
> return;
> }
>
> /* For pointers only allow additions. */
> if (m && POINTER_TYPE_P (TREE_TYPE (DECL_RESULT (current_function_decl))))
> {
> - maybe_error_musttail (call,
> - _("tail recursion with pointers can only use "
> - "additions"), diag_musttail);
> + maybe_error_musttail (call, _("tail recursion with pointers can only "
> + "use additions"), diag_musttail);
> return;
> }
>
> @@ -1594,10 +1586,10 @@ tree_optimize_tail_calls_1 (bool opt_tai
> struct tailcall *a = *p;
> *p = (*p)->next;
> gcall *call = as_a <gcall *> (gsi_stmt (a->call_gsi));
> - maybe_error_musttail (call,
> - _("tail recursion with accumulation "
> - "mixed with musttail "
> - "non-recursive call"), diag_musttail);
> + maybe_error_musttail (call, _("tail recursion with "
> + "accumulation mixed with "
> + "musttail non-recursive call"),
> + diag_musttail);
> free (a);
> }
> else
> --- gcc/calls.cc.jj 2025-04-08 14:08:48.472320552 +0200
> +++ gcc/calls.cc 2025-04-11 10:02:22.211243184 +0200
> @@ -1273,11 +1273,19 @@ void
> maybe_complain_about_tail_call (tree call_expr, const char *reason)
> {
> gcc_assert (TREE_CODE (call_expr) == CALL_EXPR);
> - if (!CALL_EXPR_MUST_TAIL_CALL (call_expr))
> - return;
> -
> - error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", reason);
> - CALL_EXPR_MUST_TAIL_CALL (call_expr) = 0;
> + if (CALL_EXPR_MUST_TAIL_CALL (call_expr))
> + {
> + error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", reason);
> + CALL_EXPR_MUST_TAIL_CALL (call_expr) = 0;
> + }
> + if (CALL_EXPR_TAILCALL (call_expr)
> + && dump_file
> + && (dump_flags & TDF_DETAILS))
> + {
> + fprintf (dump_file, ";; ");
> + print_generic_expr (dump_file, call_expr, TDF_SLIM);
> + fprintf (dump_file, "\n;; Cannot tail-call: %s\n", reason);
> + }
> }
>
> /* Fill in ARGS_SIZE and ARGS array based on the parameters found in
> @@ -1447,10 +1455,10 @@ initialize_argument_information (int num
> if (!call_from_thunk_p && DECL_P (base) && !TREE_STATIC (base))
> {
> *may_tailcall = false;
> - maybe_complain_about_tail_call (exp,
> - _("a callee-copied argument is"
> - " stored in the current"
> - " function's frame"));
> + maybe_complain_about_tail_call (exp, _("a callee-copied "
> + "argument is stored "
> + "in the current "
> + "function's frame"));
> }
>
> args[i].tree_value = build_fold_addr_expr_loc (loc,
> @@ -2534,10 +2542,9 @@ can_implement_as_sibling_call_p (tree ex
> if (!targetm.have_sibcall_epilogue ()
> && !targetm.emit_epilogue_for_sibcall)
> {
> - maybe_complain_about_tail_call
> - (exp,
> - _("machine description does not have"
> - " a sibcall_epilogue instruction pattern"));
> + maybe_complain_about_tail_call (exp, _("machine description does not "
> + "have a sibcall_epilogue "
> + "instruction pattern"));
> return false;
> }
>
> @@ -2555,9 +2562,8 @@ can_implement_as_sibling_call_p (tree ex
> into a sibcall. */
> if (!targetm.function_ok_for_sibcall (fndecl, exp))
> {
> - maybe_complain_about_tail_call (exp,
> - _("target is not able to optimize the"
> - " call into a sibling call"));
> + maybe_complain_about_tail_call (exp, _("target is not able to optimize
> "
> + "the call into a sibling call"));
> return false;
> }
>
> @@ -2606,9 +2612,8 @@ can_implement_as_sibling_call_p (tree ex
> if (maybe_gt (args_size.constant,
> crtl->args.size - crtl->args.pretend_args_size))
> {
> - maybe_complain_about_tail_call (exp,
> - _("callee required more stack slots"
> - " than the caller"));
> + maybe_complain_about_tail_call (exp, _("callee required more stack "
> + "slots than the caller"));
> return false;
> }
>
> @@ -2621,9 +2626,8 @@ can_implement_as_sibling_call_p (tree ex
> (current_function_decl),
> crtl->args.size)))
> {
> - maybe_complain_about_tail_call (exp,
> - _("inconsistent number of"
> - " popped arguments"));
> + maybe_complain_about_tail_call (exp, _("inconsistent number of"
> + " popped arguments"));
> return false;
> }
>
> @@ -2685,7 +2689,7 @@ expand_call (tree exp, rtx target, int i
> so this shouldn't really happen unless the
> the musttail pass gave up walking before finding the call. */
> if (!try_tail_call)
> - maybe_complain_about_tail_call (exp, _("other reasons"));
> + maybe_complain_about_tail_call (exp, _("other reasons"));
> int pass;
>
> /* Register in which non-BLKmode value will be returned,
> @@ -3092,8 +3096,9 @@ expand_call (tree exp, rtx target, int i
> if (MEM_P (*iter))
> {
> try_tail_call = 0;
> - maybe_complain_about_tail_call (exp,
> - _("hidden string length argument passed on stack"));
> + maybe_complain_about_tail_call (exp, _("hidden string length "
> + "argument passed on "
> + "stack"));
> break;
> }
> }
> @@ -3140,10 +3145,9 @@ expand_call (tree exp, rtx target, int i
> || partial_subreg_p (caller_mode, callee_mode)))))
> {
> try_tail_call = 0;
> - maybe_complain_about_tail_call (exp,
> - _("caller and callee disagree in"
> - " promotion of function"
> - " return value"));
> + maybe_complain_about_tail_call (exp, _("caller and callee disagree "
> + "in promotion of function "
> + "return value"));
> }
> }
>
>
> Jakub
>
