On Sun, May 5, 2024 at 8:16 PM Andi Kleen <[email protected]> wrote:
>
> - Give error messages for all causes of non sibling call generation
> - Don't override choices of other non sibling call checks with
> must tail. This causes ICEs. The must tail attribute now only
> overrides flag_optimize_sibling_calls locally.
> - Error out when tree-tailcall failed to mark a must-tail call
> sibcall. In this case it doesn't know the true reason and only gives
> a vague message (this could be improved, but it's already useful without
> that) tree-tailcall usually fails without optimization, so must
> adjust the existing must-tail plugin test to specify -O2.
>
> PR83324
>
> gcc/ChangeLog:
>
> * calls.cc (expand_call): Fix mustcall implementation.
>
> gcc/testsuite/ChangeLog:
>
> * gcc.dg/plugin/must-tail-call-1.c: Adjust.
> ---
> gcc/calls.cc | 30 ++++++++++++-------
> .../gcc.dg/plugin/must-tail-call-1.c | 1 +
> 2 files changed, 21 insertions(+), 10 deletions(-)
>
> diff --git a/gcc/calls.cc b/gcc/calls.cc
> index 21d78f9779fe..a6b8ee44cc29 100644
> --- a/gcc/calls.cc
> +++ b/gcc/calls.cc
> @@ -2650,7 +2650,9 @@ expand_call (tree exp, rtx target, int ignore)
> /* The type of the function being called. */
> tree fntype;
> bool try_tail_call = CALL_EXPR_TAILCALL (exp);
> - bool must_tail_call = CALL_EXPR_MUST_TAIL_CALL (exp);
> + /* tree-tailcall decided not to do tail calls. Error for the musttail
> case. */
> + if (!try_tail_call)
> + maybe_complain_about_tail_call (exp, "other reasons");
> int pass;
>
> /* Register in which non-BLKmode value will be returned,
> @@ -3022,10 +3024,22 @@ expand_call (tree exp, rtx target, int ignore)
> pushed these optimizations into -O2. Don't try if we're already
> expanding a call, as that means we're an argument. Don't try if
> there's cleanups, as we know there's code to follow the call. */
> - if (currently_expanding_call++ != 0
> - || (!flag_optimize_sibling_calls && !CALL_FROM_THUNK_P (exp))
> - || args_size.var
> - || dbg_cnt (tail_call) == false)
> + if (currently_expanding_call++ != 0)
> + {
> + maybe_complain_about_tail_call (exp, "inside another call");
> + try_tail_call = 0;
> + }
> + if (!flag_optimize_sibling_calls
> + && !CALL_FROM_THUNK_P (exp)
> + && !CALL_EXPR_MUST_TAIL_CALL (exp))
> + try_tail_call = 0;
> + if (args_size.var)
If we are both inside another call and run into this we give two errors,
but I guess that's OK ...
> + {
> + /* ??? correct message? */
> + maybe_complain_about_tail_call (exp, "stack space needed");
args_size.var != NULL_TREE means the argument size is not constant.
I'm quite sure this is an overly conservative check.
> + try_tail_call = 0;
> + }
> + if (dbg_cnt (tail_call) == false)
> try_tail_call = 0;
>
> /* Workaround buggy C/C++ wrappers around Fortran routines with
> @@ -3046,15 +3060,11 @@ expand_call (tree exp, rtx target, int ignore)
> if (MEM_P (*iter))
> {
> try_tail_call = 0;
> + maybe_complain_about_tail_call (exp, "hidden string length
> argument");
"hidden string length argument passed on stack"
from what I read the code.
> break;
> }
> }
>
> - /* If the user has marked the function as requiring tail-call
> - optimization, attempt it. */
> - if (must_tail_call)
> - try_tail_call = 1;
> -
> /* Rest of purposes for tail call optimizations to fail. */
> if (try_tail_call)
> try_tail_call = can_implement_as_sibling_call_p (exp,
> diff --git a/gcc/testsuite/gcc.dg/plugin/must-tail-call-1.c
> b/gcc/testsuite/gcc.dg/plugin/must-tail-call-1.c
> index 3a6d4cceaba7..44af361e2925 100644
> --- a/gcc/testsuite/gcc.dg/plugin/must-tail-call-1.c
> +++ b/gcc/testsuite/gcc.dg/plugin/must-tail-call-1.c
> @@ -1,4 +1,5 @@
> /* { dg-do compile { target tail_call } } */
> +/* { dg-options "-O2" } */
So I think this is unfortunate - I think when there's a must-tail attribute
we should either run the tailcall pass to check the call even at -O0 or
trust the user with correctness (hoping no optimization interfered with
the ability to tail-call).
What were the ICEs you ran into?
I would guess it's for example problematic to duplicate must-tail calls?
Thanks,
Richard.
> /* { dg-options "-fdelayed-branch" { target sparc*-*-* } } */
>
> extern void abort (void);
> --
> 2.44.0
>
