diff mbox series

[v8,01/12] Improve must tail in RTL backend

Message ID 20240622185557.1589179-2-ak@linux.intel.com
State New
Headers show
Series [v8,01/12] Improve must tail in RTL backend | expand

Commit Message

Andi Kleen June 22, 2024, 6:54 p.m. UTC
- Give error messages for all causes of non sibling call generation
- When giving error messages clear the musttail flag to avoid ICEs
- 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.

	PR83324

gcc/ChangeLog:

	* calls.cc (maybe_complain_about_tail_call): Clear must tail
        flag on error.
	(expand_call): Give error messages for all musttail failures.
---
 gcc/calls.cc | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

Comments

Richard Biener July 5, 2024, 10:39 a.m. UTC | #1
On Sat, Jun 22, 2024 at 8:57 PM Andi Kleen <ak@linux.intel.com> wrote:
>
> - Give error messages for all causes of non sibling call generation
> - When giving error messages clear the musttail flag to avoid ICEs
> - 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.

We seem to have concluded that we need the tailcall pass run and
both markings after all.

Thus OK.

Richard.

>         PR83324
>
> gcc/ChangeLog:
>
>         * calls.cc (maybe_complain_about_tail_call): Clear must tail
>         flag on error.
>         (expand_call): Give error messages for all musttail failures.
> ---
>  gcc/calls.cc | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/calls.cc b/gcc/calls.cc
> index 21d78f9779fe..883eb9971257 100644
> --- a/gcc/calls.cc
> +++ b/gcc/calls.cc
> @@ -1249,6 +1249,7 @@ maybe_complain_about_tail_call (tree call_expr, const char *reason)
>      return;
>
>    error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", reason);
> +  CALL_EXPR_MUST_TAIL_CALL (call_expr) = 0;
>  }
>
>  /* Fill in ARGS_SIZE and ARGS array based on the parameters found in
> @@ -2650,7 +2651,13 @@ 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,
> +     unfortunately we don't know the reason so it's fairly vague.
> +     When tree-tailcall reported an error it already cleared the flag,
> +     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");
>    int pass;
>
>    /* Register in which non-BLKmode value will be returned,
> @@ -3022,10 +3029,21 @@ 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)
> +    {
> +      maybe_complain_about_tail_call (exp, "variable size arguments");
> +      try_tail_call = 0;
> +    }
> +  if (dbg_cnt (tail_call) == false)
>      try_tail_call = 0;
>
>    /* Workaround buggy C/C++ wrappers around Fortran routines with
> @@ -3046,13 +3064,15 @@ 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 passed on stack");
>                 break;
>               }
>         }
>
>    /* If the user has marked the function as requiring tail-call
>       optimization, attempt it.  */
> -  if (must_tail_call)
> +  if (CALL_EXPR_MUST_TAIL_CALL (exp))
>      try_tail_call = 1;
>
>    /*  Rest of purposes for tail call optimizations to fail.  */
> --
> 2.45.2
>
diff mbox series

Patch

diff --git a/gcc/calls.cc b/gcc/calls.cc
index 21d78f9779fe..883eb9971257 100644
--- a/gcc/calls.cc
+++ b/gcc/calls.cc
@@ -1249,6 +1249,7 @@  maybe_complain_about_tail_call (tree call_expr, const char *reason)
     return;
 
   error_at (EXPR_LOCATION (call_expr), "cannot tail-call: %s", reason);
+  CALL_EXPR_MUST_TAIL_CALL (call_expr) = 0;
 }
 
 /* Fill in ARGS_SIZE and ARGS array based on the parameters found in
@@ -2650,7 +2651,13 @@  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,
+     unfortunately we don't know the reason so it's fairly vague.
+     When tree-tailcall reported an error it already cleared the flag,
+     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");
   int pass;
 
   /* Register in which non-BLKmode value will be returned,
@@ -3022,10 +3029,21 @@  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)
+    {
+      maybe_complain_about_tail_call (exp, "variable size arguments");
+      try_tail_call = 0;
+    }
+  if (dbg_cnt (tail_call) == false)
     try_tail_call = 0;
 
   /* Workaround buggy C/C++ wrappers around Fortran routines with
@@ -3046,13 +3064,15 @@  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 passed on stack");
 		break;
 	      }
 	}
 
   /* If the user has marked the function as requiring tail-call
      optimization, attempt it.  */
-  if (must_tail_call)
+  if (CALL_EXPR_MUST_TAIL_CALL (exp))
     try_tail_call = 1;
 
   /*  Rest of purposes for tail call optimizations to fail.  */