diff mbox

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

Message ID 5655CBAB.9000800@arm.com
State New
Headers show

Commit Message

Kyrylo Tkachov Nov. 25, 2015, 2:54 p.m. UTC 
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
>>
>

Comments

Bernd Schmidt Nov. 25, 2015, 3:06 p.m. UTC | #1 
On 11/25/2015 03:54 PM, Kyrill Tkachov wrote:
> 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?

Sure. Thanks! (Backport too.)


Bernd
diff mbox

Patch

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