[PATCH] Fix PR44941

[PATCH] Fix PR44941

[Richard Guenther]

This fixes PR44941 by early handling arguments of size zero.  With
MEM_REF type-punned cases can now be registers which we do not handle
properly.  After fixing this you can now see that we can avoid generating
the stack slot for the zero-sized arg completely.

Bootstrapped and tested on x86_64-unknown-linux-gnu.  Comments?

Thanks,
Richard.

2010-07-19  Richard Guenther  <rguenther@suse.de>

	PR middle-end/44941
	* expr.c (emit_block_move_hints): Move zero size check first.
	Move asserts to more useful places.
	* calls.c (load_register_parameters): Check for zero size.

	* gcc.c-torture/compile/pr44941.c: New testcase.

Index: gcc/expr.c
===================================================================
*** gcc/expr.c	(revision 162298)
--- gcc/expr.c	(working copy)
*************** emit_block_move_hints (rtx x, rtx y, rtx
*** 1120,1125 ****
--- 1120,1130 ----
    rtx retval = 0;
    unsigned int align;
  
+   gcc_assert (size);
+   if (CONST_INT_P (size)
+       && INTVAL (size) == 0)
+     return 0;
+ 
    switch (method)
      {
      case BLOCK_OP_NORMAL:
*************** emit_block_move_hints (rtx x, rtx y, rtx
*** 1143,1155 ****
        gcc_unreachable ();
      }
  
    align = MIN (MEM_ALIGN (x), MEM_ALIGN (y));
    gcc_assert (align >= BITS_PER_UNIT);
  
-   gcc_assert (MEM_P (x));
-   gcc_assert (MEM_P (y));
-   gcc_assert (size);
- 
    /* Make sure we've got BLKmode addresses; store_one_arg can decide that
       block copy is more efficient for other large modes, e.g. DCmode.  */
    x = adjust_address (x, BLKmode, 0);
--- 1148,1157 ----
        gcc_unreachable ();
      }
  
+   gcc_assert (MEM_P (x) && MEM_P (y));
    align = MIN (MEM_ALIGN (x), MEM_ALIGN (y));
    gcc_assert (align >= BITS_PER_UNIT);
  
    /* Make sure we've got BLKmode addresses; store_one_arg can decide that
       block copy is more efficient for other large modes, e.g. DCmode.  */
    x = adjust_address (x, BLKmode, 0);
*************** emit_block_move_hints (rtx x, rtx y, rtx
*** 1159,1167 ****
       can be incorrect is coming from __builtin_memcpy.  */
    if (CONST_INT_P (size))
      {
-       if (INTVAL (size) == 0)
- 	return 0;
- 
        x = shallow_copy_rtx (x);
        y = shallow_copy_rtx (y);
        set_mem_size (x, size);
--- 1161,1166 ----
Index: gcc/testsuite/gcc.c-torture/compile/pr44941.c
===================================================================
*** gcc/testsuite/gcc.c-torture/compile/pr44941.c	(revision 0)
--- gcc/testsuite/gcc.c-torture/compile/pr44941.c	(revision 0)
***************
*** 0 ****
--- 1,8 ----
+ struct S { };
+ 
+ extern void bar(struct S);
+ 
+ void foo (int i)
+ {
+   bar (*(struct S *)&i);
+ }
Index: gcc/calls.c
===================================================================
*** gcc/calls.c	(revision 162298)
--- gcc/calls.c	(working copy)
*************** load_register_parameters (struct arg_dat
*** 1668,1674 ****
  	      emit_move_insn (gen_rtx_REG (word_mode, REGNO (reg) + j),
  			      args[i].aligned_regs[j]);
  
! 	  else if (partial == 0 || args[i].pass_on_stack)
  	    {
  	      rtx mem = validize_mem (args[i].value);
  
--- 1668,1675 ----
  	      emit_move_insn (gen_rtx_REG (word_mode, REGNO (reg) + j),
  			      args[i].aligned_regs[j]);
  
! 	  else if ((partial == 0 || args[i].pass_on_stack)
! 		   && size != 0)
  	    {
  	      rtx mem = validize_mem (args[i].value);
  

Re: [PATCH] Fix PR44941

[IainS]

Hi Richard,

On 19 Jul 2010, at 13:42, Richard Guenther wrote:

>
> This fixes PR44941 by early handling arguments of size zero.  With
> MEM_REF type-punned cases can now be registers which we do not handle
> properly.  After fixing this you can now see that we can avoid  
> generating
> the stack slot for the zero-sized arg completely.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.  Comments?


> 2010-07-19  Richard Guenther  <rguenther@suse.de>
>
> 	PR middle-end/44941
> 	* expr.c (emit_block_move_hints): Move zero size check first.
> 	Move asserts to more useful places.
> 	* calls.c (load_register_parameters): Check for zero size.
>
> 	* gcc.c-torture/compile/pr44941.c: New testcase.
>  	    {
>  	      rtx mem = validize_mem (args[i].value);

The change to calls.c caused some compat/* fails on powerpc*-darwin9.

As discussed on irc, (hopefully I understood what was wanted)
the attached patch restores powerpc...
...  whilst compile.exp=pr44941.c still passes on i686-darwin9.
bootstrapped on {i686,powerpc}-darwin9
OK for trunk?
Iain




Index: gcc/calls.c
===================================================================
--- gcc/calls.c	(revision 162339)
+++ gcc/calls.c	(working copy)
@@ -1668,15 +1668,16 @@ load_register_parameters (struct arg_data  
*args, i
  	      emit_move_insn (gen_rtx_REG (word_mode, REGNO (reg) + j),
  			      args[i].aligned_regs[j]);

-	  else if ((partial == 0 || args[i].pass_on_stack)
-		   && size != 0)
+	  else if (partial == 0 || args[i].pass_on_stack)
  	    {
  	      rtx mem = validize_mem (args[i].value);

-	      /* Check for overlap with already clobbered argument area.  */
+	      /* Check for overlap with already clobbered argument area,
+	         providing that this has finite size.  */
  	      if (is_sibcall
-		  && 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;

  	      /* Handle a BLKmode that needs shifting.  */

Re: [PATCH] Fix PR44941

[Richard Guenther]

On Wed, 21 Jul 2010, IainS wrote:

> Hi Richard,
> 
> On 19 Jul 2010, at 13:42, Richard Guenther wrote:
> 
> > 
> > This fixes PR44941 by early handling arguments of size zero.  With
> > MEM_REF type-punned cases can now be registers which we do not handle
> > properly.  After fixing this you can now see that we can avoid generating
> > the stack slot for the zero-sized arg completely.
> > 
> > Bootstrapped and tested on x86_64-unknown-linux-gnu.  Comments?
> 
> 
> > 2010-07-19  Richard Guenther  <rguenther@suse.de>
> > 
> > 	PR middle-end/44941
> > 	* expr.c (emit_block_move_hints): Move zero size check first.
> > 	Move asserts to more useful places.
> > 	* calls.c (load_register_parameters): Check for zero size.
> > 
> > 	* gcc.c-torture/compile/pr44941.c: New testcase.
> > 	    {
> > 	      rtx mem = validize_mem (args[i].value);
> 
> The change to calls.c caused some compat/* fails on powerpc*-darwin9.
> 
> As discussed on irc, (hopefully I understood what was wanted)
> the attached patch restores powerpc...
> ...  whilst compile.exp=pr44941.c still passes on i686-darwin9.
> bootstrapped on {i686,powerpc}-darwin9
> OK for trunk?

Ok with a proper changelog entry and ....

> Iain
> 
> 
> 
> 
> Index: gcc/calls.c
> ===================================================================
> --- gcc/calls.c	(revision 162339)
> +++ gcc/calls.c	(working copy)
> @@ -1668,15 +1668,16 @@ load_register_parameters (struct arg_data *args, i
> 	      emit_move_insn (gen_rtx_REG (word_mode, REGNO (reg) + j),
> 			      args[i].aligned_regs[j]);
> 
> -	  else if ((partial == 0 || args[i].pass_on_stack)
> -		   && size != 0)
> +	  else if (partial == 0 || args[i].pass_on_stack)
> 	    {
> 	      rtx mem = validize_mem (args[i].value);
> 
> -	      /* Check for overlap with already clobbered argument area.  */
> +	      /* Check for overlap with already clobbered argument area,
> +	         providing that this has finite size.  */
> 	      if (is_sibcall
> -		  && mem_overlaps_already_clobbered_arg_p (XEXP
> (args[i].value, 0),
> -							   size))
> +		  && (size == 0
> +		      || mem_overlaps_already_clobbered_arg_p (

the parantheses moved to the next line.

Thanks,
Richard.

> +					    XEXP (args[i].value, 0), size)))
> 		*sibcall_failure = 1;
> 
> 	      /* Handle a BLKmode that needs shifting.  */
> 

Re: [PATCH] Fix PR44941

[IainS]

On 21 Jul 2010, at 09:13, Richard Guenther wrote:
>> OK for trunk?
>
> Ok with a proper changelog entry and ....

done
>> +		      || mem_overlaps_already_clobbered_arg_p (
>
> the parantheses moved to the next line.

done

r162402
Iain