diff mbox

Tighten check for whether a sibcall references local variables

Message ID 87h96yp1gf.fsf@e105548-lin.cambridge.arm.com
State New
Headers show

Commit Message

Richard Sandiford Nov. 23, 2016, 5:09 p.m. UTC 
Richard Biener <richard.guenther@gmail.com> writes:
> 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.

OK, how's this?  Tested on aarch64-linux-gnu and x86_64-linux-gnu.

Thanks,
Richard


gcc/testsuite/
	* gcc.dg/tree-ssa/tailcall-7-run.c: New test.
	* gcc.dg/tree-ssa/tailcall-8-run.c: Likewise.

Comments

Jeff Law Nov. 23, 2016, 6:20 p.m. UTC | #1 
On 11/23/2016 10:09 AM, Richard Sandiford wrote:
> Richard Biener <richard.guenther@gmail.com> writes:
>> 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.
>
> OK, how's this?  Tested on aarch64-linux-gnu and x86_64-linux-gnu.
>
> Thanks,
> Richard
>
>
> gcc/testsuite/
> 	* gcc.dg/tree-ssa/tailcall-7-run.c: New test.
> 	* gcc.dg/tree-ssa/tailcall-8-run.c: Likewise.
Works for me.
jeff
diff mbox

Patch

diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-7-run.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-7-run.c
new file mode 100644
index 0000000..d18ff9d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-7-run.c
@@ -0,0 +1,72 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+/* { dg-additional-sources "tailcall-7.c" } */
+
+struct s { int x; };
+extern struct s global;
+
+void g1 (void);
+void g2 (void);
+void g3 (struct s *);
+struct s g4 (struct s);
+struct s g5 (void);
+struct s g6 (void);
+struct s g7 (void);
+struct s g8 (struct s *);
+int g9 (struct s);
+int g10 (int);
+
+struct s last;
+struct s tmp;
+
+struct s
+f (int i)
+{
+  struct s ret;
+  ret.x = i + 100;
+  last = ret;
+  return ret;
+}
+
+void
+callit (void (*fn) (void))
+{
+  fn ();
+}
+
+int
+test (int last_val, int global_val, int tmp_val)
+{
+  return last.x == last_val && global.x == global_val && tmp.x == tmp_val;
+}
+
+int
+main (void)
+{
+  global.x = 200;
+  tmp.x = 300;
+  g1 ();
+  if (!test (101, 200, 300))
+    __builtin_abort ();
+  g2 ();
+  if (!test (102, 102, 300))
+    __builtin_abort ();
+  g3 (&tmp);
+  if (!test (103, 102, 103))
+    __builtin_abort ();
+  if (g4 (tmp).x != 104 || !test (104, 102, 103))
+    __builtin_abort ();
+  if (g5 ().x != 105 || !test (105, 102, 103))
+    __builtin_abort ();
+  if (g6 ().x != 106 || !test (106, 102, 103))
+    __builtin_abort ();
+  if (g7 ().x != 107 || !test (107, 107, 103))
+    __builtin_abort ();
+  if (g8 (&tmp).x != 108 || !test (108, 107, 108))
+    __builtin_abort ();
+  if (g9 (tmp) != 9 || !test (109, 107, 108))
+    __builtin_abort ();
+  if (g10 (10) != 10 || !test (110, 107, 108))
+    __builtin_abort ();
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/tailcall-8-run.c b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-8-run.c
new file mode 100644
index 0000000..f892909
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/tailcall-8-run.c
@@ -0,0 +1,86 @@ 
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+/* { dg-additional-sources "tailcall-8.c" } */
+
+struct s { int x; };
+
+int expected;
+struct s *last_ptr;
+struct s tmp;
+
+void
+start (int val, struct s *initial_last_ptr)
+{
+  expected = val;
+  tmp.x = val;
+  last_ptr = initial_last_ptr;
+}
+
+void
+f_direct (struct s param)
+{
+  if (param.x != expected)
+    __builtin_abort ();
+}
+
+void
+f_indirect (struct s *ptr)
+{
+  if (ptr->x != expected)
+    __builtin_abort ();
+  last_ptr = ptr;
+  ptr->x += 100;
+}
+
+void
+f_void (void)
+{
+  if (last_ptr->x != expected + 100)
+    __builtin_abort ();
+}
+
+
+void g1 (struct s);
+void g2 (struct s *);
+void g3 (struct s *);
+void g4 (struct s *);
+void g5 (struct s);
+void g6 (struct s);
+void g7 (struct s);
+void g8 (struct s *);
+void g9 (struct s *);
+
+int
+main (void)
+{
+  struct s g6_s = { 106 };
+
+  start (1, 0);
+  g1 (tmp);
+
+  start (2, 0);
+  g2 (&tmp);
+
+  start (3, 0);
+  g3 (&tmp);
+
+  start (4, 0);
+  g4 (&tmp);
+
+  start (5, 0);
+  g5 (tmp);
+
+  start (6, &g6_s);
+  g6 (tmp);
+
+  start (7, 0);
+  g7 (tmp);
+
+  start (8, 0);
+  g8 (&tmp);
+
+  start (9, 0);
+  g9 (&tmp);
+
+  return 0;
+}