Re: Tighten check for whether a sibcall references local variables

[Richard Biener <richard dot guenther at gmail dot com>]

On Tue, Nov 22, 2016 at 10:00 AM, Richard Sandiford
<richard.sandiford@arm.com> wrote:
> This loop:
>
>   /* Make sure the tail invocation of this function does not refer
>      to local variables.  */
>   FOR_EACH_LOCAL_DECL (cfun, idx, var)
>     {
>       if (TREE_CODE (var) != PARM_DECL
>           && auto_var_in_fn_p (var, cfun->decl)
>           && (ref_maybe_used_by_stmt_p (call, var)
>               || call_may_clobber_ref_p (call, var)))
>         return;
>     }
>
> triggered even for local variables that are passed by value.
> This meant that we didn't allow local aggregates to be passed
> to a sibling call but did (for example) allow global aggregates
> to be passed.
>
> I think the loop is really checking for indirect references,
> so should be able to skip any variables that never have their
> address taken.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Ok.  We've had various correctness issues in this part in the past so
I'd prefer if you can rewrite your dg-do compile tests to dg-do run ones
that verify the code works correctly.  I suggest to use a dg-additional-sources
with a separate TU for the execution driver.

Thanks,
Richard.

> Thanks,
> Richard
>
>
> gcc/
>         * tree-tailcall.c (find_tail_calls): Allow calls to reference
>         local variables if all references are known to be direct.
>
> gcc/testsuite/
>         * gcc.dg/tree-ssa/tailcall-8.c: New test.
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-8.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-8.c
> new file mode 100644
> index 0000000..ffeabe5
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-8.c
> @@ -0,0 +1,80 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-tailc-details" } */
> +
> +struct s { int x; };
> +void f_direct (struct s);
> +void f_indirect (struct s *);
> +void f_void (void);
> +
> +/* Tail call.  */
> +void
> +g1 (struct s param)
> +{
> +  f_direct (param);
> +}
> +
> +/* Tail call.  */
> +void
> +g2 (struct s *param_ptr)
> +{
> +  f_direct (*param_ptr);
> +}
> +
> +/* Tail call.  */
> +void
> +g3 (struct s *param_ptr)
> +{
> +  f_indirect (param_ptr);
> +}
> +
> +/* Tail call.  */
> +void
> +g4 (struct s *param_ptr)
> +{
> +  f_indirect (param_ptr);
> +  f_void ();
> +}
> +
> +/* Tail call.  */
> +void
> +g5 (struct s param)
> +{
> +  struct s local = param;
> +  f_direct (local);
> +}
> +
> +/* Tail call.  */
> +void
> +g6 (struct s param)
> +{
> +  struct s local = param;
> +  f_direct (local);
> +  f_void ();
> +}
> +
> +/* Not a tail call.  */
> +void
> +g7 (struct s param)
> +{
> +  struct s local = param;
> +  f_indirect (&local);
> +}
> +
> +/* Not a tail call.  */
> +void
> +g8 (struct s *param_ptr)
> +{
> +  struct s local = *param_ptr;
> +  f_indirect (&local);
> +}
> +
> +/* Not a tail call.  */
> +void
> +g9 (struct s *param_ptr)
> +{
> +  struct s local = *param_ptr;
> +  f_indirect (&local);
> +  f_void ();
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Found tail call" 6 "tailc" } } */
> diff --git a/gcc/tree-tailcall.c b/gcc/tree-tailcall.c
> index f97541d..66a0a4c 100644
> --- a/gcc/tree-tailcall.c
> +++ b/gcc/tree-tailcall.c
> @@ -504,12 +504,14 @@ find_tail_calls (basic_block bb, struct tailcall **ret)
>         tail_recursion = true;
>      }
>
> -  /* Make sure the tail invocation of this function does not refer
> -     to local variables.  */
> +  /* Make sure the tail invocation of this function does not indirectly
> +     refer to local variables.  (Passing variables directly by value
> +     is OK.)  */
>    FOR_EACH_LOCAL_DECL (cfun, idx, var)
>      {
>        if (TREE_CODE (var) != PARM_DECL
>           && auto_var_in_fn_p (var, cfun->decl)
> +         && may_be_aliased (var)
>           && (ref_maybe_used_by_stmt_p (call, var)
>               || call_may_clobber_ref_p (call, var)))
>         return;
>