Re: Tighten check for whether a sibcall references local variables

[Richard Sandiford <richard dot sandiford at arm dot com>]

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.

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;
+}