diff mbox

RFA: PATCH to load_register_parameters for empty structs and sibcalls

Message ID 56E9A9BE.8090009@redhat.com
State New
Headers show

Commit Message

Jason Merrill March 16, 2016, 6:45 p.m. UTC 
Discussion of empty class parameter passing ABI led me to notice that 
r162402 broke sibcalls with arguments of size 0 in some cases.  Before 
that commit, the code read

> else if ((partial == 0 || args[i].pass_on_stack)
>          && size != 0)
> {
>   rtx mem = validize_mem (args[i].value);
>
>   /* Check for overlap with already clobbered argument area. */
>   if (is_sibcall
>       && mem_overlaps_already_clobbered_arg_p (XEXP (args[i].value, 0), size))
>      *sibcall_failure = 1;

and after,

>         else if (partial == 0 || args[i].pass_on_stack)
>           {
>             rtx mem = validize_mem (args[i].value);
>
>             /* Check for overlap with already clobbered argument area,
>                providing that this has non-zero size.  */
>             if (is_sibcall
>                 && (size == 0
>                     || mem_overlaps_already_clobbered_arg_p
>                                          (XEXP (args[i].value, 0), size)))
>               *sibcall_failure = 1;

So now we set *sibcall_failure if size==0, whereas before we didn't 
enter the outer block.  The comment also contradicts the code.

OK for trunk?

Comments

Bernd Schmidt March 16, 2016, 7:04 p.m. UTC | #1 
On 03/16/2016 07:45 PM, Jason Merrill wrote:
> Discussion of empty class parameter passing ABI led me to notice that
> r162402 broke sibcalls with arguments of size 0 in some cases.  Before
> that commit, the code read
>
>> else if ((partial == 0 || args[i].pass_on_stack)
>>          && size != 0)
>> {
[...]
>>   if (is_sibcall
>>       && mem_overlaps_already_clobbered_arg_p (XEXP (args[i].value,
>> 0), size))
>>      *sibcall_failure = 1;
>
> and after,

>>             if (is_sibcall
>>                 && (size == 0
>>                     || mem_overlaps_already_clobbered_arg_p
>>                                          (XEXP (args[i].value, 0),
>> size)))

> So now we set *sibcall_failure if size==0, whereas before we didn't
> enter the outer block.  The comment also contradicts the code.

The patch looks ok. I was trying to research the earlier change, but I 
can't find a message in the archives. Cc'ing Iain in case he has input.


Bernd
diff mbox

Patch

commit 2fa659c5b7779ba190cd5e9d15a8b7bc3dc5df35
Author: Jason Merrill <jason@redhat.com>
Date:   Wed Mar 16 12:57:37 2016 -0400

    	* calls.c (load_register_parameters): Fix zero size sibcall logic.

diff --git a/gcc/calls.c b/gcc/calls.c
index 7b28f43..6415e08 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -2083,9 +2083,9 @@  load_register_parameters (struct arg_data *args, int num_actuals,
 	      /* Check for overlap with already clobbered argument area,
 	         providing that this has non-zero size.  */
 	      if (is_sibcall
-		  && (size == 0
-		      || mem_overlaps_already_clobbered_arg_p 
-					   (XEXP (args[i].value, 0), size)))
+		  && size != 0
+		  && (mem_overlaps_already_clobbered_arg_p
+		      (XEXP (args[i].value, 0), size)))
 		*sibcall_failure = 1;
 
 	      if (size % UNITS_PER_WORD == 0
diff --git a/gcc/testsuite/gcc.dg/sibcall-11.c b/gcc/testsuite/gcc.dg/sibcall-11.c
new file mode 100644
index 0000000..ae58770
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/sibcall-11.c
@@ -0,0 +1,7 @@ 
+// Test for sibcall optimization with empty struct.
+// { dg-options "-O2" }
+// { dg-final { scan-assembler "jmp" { target i?86-*-* x86_64-*-* } } }
+
+struct A { };
+void f(struct A);
+void g(struct A a) { f(a); }