Re: [PATCH][calls.c] PR rtl-optimization/67226: Take into account pretend_args_size when checking stack offsets for sibcall optimisation

[Kyrill Tkachov <kyrylo dot tkachov at arm dot com>]

On 25/11/15 14:14, Kyrill Tkachov wrote:
> On 25/11/15 14:12, Bernd Schmidt wrote:
> > On 11/25/2015 03:10 PM, Kyrill Tkachov wrote:
> >
> > > What should we do when we don't have STACK_GROWS_DOWNWARD? Do we need to
> > > write:
> > > if (STACK_GROWS_DOWNWARD)
> > >    i -= crtl->args.pretend_args_size;
> > > else
> > >    i += crtl->args.pretend_args_size;
> >
> > I think so. I mean, this should mirror this code here, right?
> >
> >       /* The argument block when performing a sibling call is the
> >      incoming argument block.  */
> >       if (pass == 0)
> >     {
> >       argblock = crtl->args.internal_arg_pointer;
> >       if (STACK_GROWS_DOWNWARD)
> >         argblock
> >           = plus_constant (Pmode, argblock, crtl->args.pretend_args_size);
> >       else
> >         argblock
> >           = plus_constant (Pmode, argblock, -crtl->args.pretend_args_size);
>
> Yes, you're right.
> I'll send out an updated version of the patch shortly.

And here it is.
This fixes the bug on arm and tests on arm look ok.
I've kicked off bootstraps and tests on arm, aarch64 and x86_64.
Ok for trunk if they come back clean?
The first version of the patch that I posted should be equivalent to this one on those targets
(as you noted).

I'll prepare a backport for GCC 5 and 4.9 as it occurs there as well.
We'll have to use the #ifdef check for STACK_GROWS_DOWNWARD on those branches...

Thanks,
Kyrill

2015-11-25  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
            Bernd Schmidt  <bschmidt@redhat.com>

    PR rtl-optimization/67226
    * calls.c (store_one_arg): Take into account
    crtl->args.pretend_args_size when checking for overlap between
    arg->value and argblock + arg->locate.offset during sibcall
    optimization.

2015-11-25  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

    PR rtl-optimization/67226
    * gcc.c-torture/execute/pr67226.c: New test.

> Kyrill
>
> > Bernd

commit cec1dd7490b3a5931aefac5aeafdb1380af9437f
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Mon Nov 23 13:19:57 2015 +0000

    PR rtl-optimization/67226: Take into account pretend_args_size when checking stack offsets for sibcall optimisation

diff --git a/gcc/calls.c b/gcc/calls.c
index b56556a..6cbe019 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -4939,6 +4939,13 @@ store_one_arg (struct arg_data *arg, rtx argblock, int flags,
 	      if (XEXP (x, 0) != crtl->args.internal_arg_pointer)
 		i = INTVAL (XEXP (XEXP (x, 0), 1));
 
+	      /* arg.locate doesn't contain the pretend_args_size offset,
+		 it's part of argblock.  Ensure we don't count it in I.  */
+	      if (STACK_GROWS_DOWNWARD)
+		i -= crtl->args.pretend_args_size;
+	      else
+		i += crtl->args.pretend_args_size;
+
 	      /* expand_call should ensure this.  */
 	      gcc_assert (!arg->locate.offset.var
 			  && arg->locate.size.var == 0
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr67226.c b/gcc/testsuite/gcc.c-torture/execute/pr67226.c
new file mode 100644
index 0000000..c533496
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr67226.c
@@ -0,0 +1,42 @@
+struct assembly_operand
+{
+  int type, value, symtype, symflags, marker;
+};
+
+struct assembly_operand to_input, from_input;
+
+void __attribute__ ((__noinline__, __noclone__))
+assemblez_1 (int internal_number, struct assembly_operand o1)
+{
+  if (o1.type != from_input.type)
+    __builtin_abort ();
+}
+
+void __attribute__ ((__noinline__, __noclone__))
+t0 (struct assembly_operand to, struct assembly_operand from)
+{
+  if (to.value == 0)
+    assemblez_1 (32, from);
+  else
+    __builtin_abort ();
+}
+
+int
+main (void)
+{
+  to_input.value = 0;
+  to_input.type = 1;
+  to_input.symtype = 2;
+  to_input.symflags = 3;
+  to_input.marker = 4;
+
+  from_input.value = 5;
+  from_input.type = 6;
+  from_input.symtype = 7;
+  from_input.symflags = 8;
+  from_input.marker = 9;
+
+  t0 (to_input, from_input);
+
+  return 0;
+}