Message ID | 87polnq46z.fsf@e105548-lin.cambridge.arm.com |
---|---|
State | New |
Headers | show |
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; >
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;