On Mon, Nov 21, 2016 at 11:34 AM, Richard Sandiford
<richard.sandif...@arm.com> wrote:
> We treated this g as a sibling call to f:
>
> int f (int);
> int g (void) { return f (1); }
>
> but not this one:
>
> struct s { int i; };
> struct s f (int);
> struct s g (void) { return f (1); }
>
> We treated them both as sibcalls on x86 before the first patch for PR36326,
> so I suppose this is a regression of sorts from 4.3.
>
> The patch allows function returns to be local aggregate variables as well
> as gimple registers.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
Ok.
Richard.
> Thanks,
> Richard
>
>
> gcc/
> * tree-tailcall.c (process_assignment): Simplify the check for
> a valid copy, allowing the source to be a local variable as
> well as an SSA name.
> (find_tail_calls): Allow copies between local variables to follow
> the call. Allow the result to be stored in any local variable,
> even if it's an aggregate.
> (eliminate_tail_call): Check whether the result is an SSA name
> before updating its SSA_NAME_DEF_STMT.
>
> gcc/testsuite/
> * gcc.dg/tree-ssa/tailcall-7.c: New test.
>
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-7.c
> b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-7.c
> new file mode 100644
> index 0000000..eabf1a8
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-7.c
> @@ -0,0 +1,89 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-tailc-details" } */
> +
> +struct s { int x; };
> +struct s f (int);
> +struct s global;
> +void callit (void (*) (void));
> +
> +/* Tail call. */
> +void
> +g1 (void)
> +{
> + f (1);
> +}
> +
> +/* Not a tail call. */
> +void
> +g2 (void)
> +{
> + global = f (2);
> +}
> +
> +/* Not a tail call. */
> +void
> +g3 (struct s *ptr)
> +{
> + *ptr = f (3);
> +}
> +
> +/* Tail call. */
> +struct s
> +g4 (struct s param)
> +{
> + param = f (4);
> + return param;
> +}
> +
> +/* Tail call. */
> +struct s
> +g5 (void)
> +{
> + struct s local = f (5);
> + return local;
> +}
> +
> +/* Tail call. */
> +struct s
> +g6 (void)
> +{
> + return f (6);
> +}
> +
> +/* Not a tail call. */
> +struct s
> +g7 (void)
> +{
> + struct s local = f (7);
> + global = local;
> + return local;
> +}
> +
> +/* Not a tail call. */
> +struct s
> +g8 (struct s *ptr)
> +{
> + struct s local = f (8);
> + *ptr = local;
> + return local;
> +}
> +
> +/* Not a tail call. */
> +int
> +g9 (struct s param)
> +{
> + void inner (void) { param = f (9); }
> + callit (inner);
> + return 9;
> +}
> +
> +/* Tail call. */
> +int
> +g10 (int param)
> +{
> + void inner (void) { f (param); }
> + callit (inner);
> + return 10;
> +}
> +
> +/* { dg-final { scan-tree-dump-times "Found tail call" 5 "tailc" } } */
> diff --git a/gcc/tree-tailcall.c b/gcc/tree-tailcall.c
> index 0436f0f..f97541d 100644
> --- a/gcc/tree-tailcall.c
> +++ b/gcc/tree-tailcall.c
> @@ -269,7 +269,7 @@ process_assignment (gassign *stmt, gimple_stmt_iterator
> call, tree *m,
> conversions that can never produce extra code between the function
> call and the function return. */
> if ((rhs_class == GIMPLE_SINGLE_RHS || gimple_assign_cast_p (stmt))
> - && (TREE_CODE (src_var) == SSA_NAME))
> + && src_var == *ass_var)
> {
> /* Reject a tailcall if the type conversion might need
> additional code. */
> @@ -287,9 +287,6 @@ process_assignment (gassign *stmt, gimple_stmt_iterator
> call, tree *m,
> return false;
> }
>
> - if (src_var != *ass_var)
> - return false;
> -
> *ass_var = dest;
> return true;
> }
> @@ -428,6 +425,13 @@ find_tail_calls (basic_block bb, struct tailcall **ret)
> break;
> }
>
> + /* Allow simple copies between local variables, even if they're
> + aggregates. */
> + if (is_gimple_assign (stmt)
> + && auto_var_in_fn_p (gimple_assign_lhs (stmt), cfun->decl)
> + && auto_var_in_fn_p (gimple_assign_rhs1 (stmt), cfun->decl))
> + continue;
> +
> /* If the statement references memory or volatile operands, fail. */
> if (gimple_references_memory_p (stmt)
> || gimple_has_volatile_ops (stmt))
> @@ -444,18 +448,20 @@ find_tail_calls (basic_block bb, struct tailcall **ret)
> return;
> }
>
> - /* If the LHS of our call is not just a simple register, we can't
> - transform this into a tail or sibling call. This situation happens,
> - in (e.g.) "*p = foo()" where foo returns a struct. In this case
> - we won't have a temporary here, but we need to carry out the side
> - effect anyway, so tailcall is impossible.
> + /* If the LHS of our call is not just a simple register or local
> + variable, we can't transform this into a tail or sibling call.
> + This situation happens, in (e.g.) "*p = foo()" where foo returns a
> + struct. In this case we won't have a temporary here, but we need
> + to carry out the side effect anyway, so tailcall is impossible.
>
> ??? In some situations (when the struct is returned in memory via
> invisible argument) we could deal with this, e.g. by passing 'p'
> itself as that argument to foo, but it's too early to do this here,
> and expand_call() will not handle it anyway. If it ever can, then
> we need to revisit this here, to allow that situation. */
> - if (ass_var && !is_gimple_reg (ass_var))
> + if (ass_var
> + && !is_gimple_reg (ass_var)
> + && !auto_var_in_fn_p (ass_var, cfun->decl))
> return;
>
> /* We found the call, check whether it is suitable. */
> @@ -888,7 +894,7 @@ eliminate_tail_call (struct tailcall *t)
>
> call = gsi_stmt (t->call_gsi);
> rslt = gimple_call_lhs (call);
> - if (rslt != NULL_TREE)
> + if (rslt != NULL_TREE && TREE_CODE (rslt) == SSA_NAME)
> {
> /* Result of the call will no longer be defined. So adjust the
> SSA_NAME_DEF_STMT accordingly. */
>
