[RFC/RFT PATCH, middle end]: Fix PR 34621, [4.3 Regression] gcc.c-torture/execute/va-arg-25.c:32: internal compiler error: in expand_call, at calls.c:2785

[Uros Bizjak <ubizjak at gmail dot com>]

Hello!

This bug is triggered on i686-apple-darwin9, where STACK_BOUNDARY is 
fixed to 128, while PARM_BOUNDARY is set to 32 (BITS_PER_WORD).

In gcc.c-torture/execute/va-arg-25.c, a sequence of int/SSE/int/SSE 
arguments is pushed to the stack (due to stack-passing ABI for this 
particular target). SSE arguments are constrained by their 16 byte 
alignment and stack alignment at the call point is forced to 16byte by 
STACK_BOUNDARY define.

However, alignment_pad of the argument (this defines the size of the 
stack slot of the argument, so _next_ argument is aligned correctly) is 
calculated only when (boundary > PARM_BOUNDARY && boundary > 
STACK_BOUNDARY). This implies that PARM_BOUNDARY is more or equal to 
STACK_BOUNDARY to trigger the condition. In darwin9 case, this is not 
true anymore, and alignment_pad of "int" argument remains zero. This 
effect can be observed by disabling a couple of checking asserts (see 
comment #9 in the PR), in order to inspect wrong call frame for 
-mno-accumulate-outgoing-args:

main:
       subl    $28, %esp
       movaps  .LC0, %xmm0
       movaps  %xmm0, (%esp)     <- aligned to 16
       pushl   $2                <- esp = esp - 4
       movaps  .LC1, %xmm0
       subl    $16, %esp         <- esp = esp - 16
       movaps  %xmm0, (%esp)     <- *crash*
       pushl   $1                <- esp = esp - 4
       call    foo               <- *not aligned to 16 bytes*
       ...


When the condition is changed, so it checks if "boundary" is more than 
either PARM_BOUNDARY or STACK_BOUNDARY, alignment pad is correctly 
calculated, and we compile to:

main:
       subl    $40, %esp
       movaps  .LC0, %xmm0
       movaps  %xmm0, 12(%esp)  <- aligned to 16
       pushl   $2               <- pad (12) + 4
       movaps  .LC1, %xmm0
       subl    $28, %esp
       movaps  %xmm0, 12(%esp)  <- aligned to 16
       pushl   $1               <- pad (12) + 4
       call    foo              <- aligned to 16
          ...

In -maccumulate-outgoing-args case, we don't use alignment pads, because 
gcc constructs call frame using offsets relative to base %esp:

main:
       subl    $76, %esp
       movdqa  .LC0, %xmm0
       movl    $2, 32(%esp)
       movdqa  %xmm0, 48(%esp)
       movl    $1, (%esp)
       movdqa  .LC1, %xmm0
       movdqa  %xmm0, 16(%esp)
       call    foo
       ...


And for x86_64, we use register passing ABI, so this bug is not triggered.

If we don't want STACK_BOUNDARY to represent *HARDWARE* limitation (as 
in minimum stack pointer increase by the stack "push" insn) and thus 
want to have STACK_BOUNDARY > PARM_BOUNDARY, we need following patch:

2008-02-14  Uros Bizjak  <ubizjak@gmail.com>

       PR middle-end/34621
       * function.c (pad_to_arg_alignment): Calculate alignment_pad if
       "boundary" is larger than PARM_BOUNDARY or larger than 
STACK_BOUNDARY.

Patch was bootstrapped and regression tested on i686-pc-linux-gnu and 
x86_64-pc-linux-gnu. Also, I have compiled and successfully executed 
va-arg-25.c with STACK_BOUNDARY set to 128. For added fun, it also works 
when integer va-arguments are changed to double, so I'm pretty confident 
in this patch.

While I believe that the patch is correct, I don't have 
i686-apple-darwin target (or PPC that could also be affected) here to 
fully test this change. Volunteers are welcome...

BTW: svn blame attributes Jeffrey Law as the author of this part of 
function.c, so I have added him to Cc.

Uros.

Index: function.c
===================================================================
--- function.c	(revision 132317)
+++ function.c	(working copy)
@@ -3450,7 +3450,7 @@ pad_to_arg_alignment (struct args_size *
     sp_offset = 0;
 #endif
 
-  if (boundary > PARM_BOUNDARY && boundary > STACK_BOUNDARY)
+  if (boundary > PARM_BOUNDARY || boundary > STACK_BOUNDARY)
     {
       save_var = offset_ptr->var;
       save_constant = offset_ptr->constant;
@@ -3476,7 +3476,7 @@ pad_to_arg_alignment (struct args_size *
 	  offset_ptr->var = size_binop (MINUS_EXPR, rounded, sp_offset_tree);
 	  /* ARGS_SIZE_TREE includes constant term.  */
 	  offset_ptr->constant = 0;
-	  if (boundary > PARM_BOUNDARY && boundary > STACK_BOUNDARY)
+	  if (boundary > PARM_BOUNDARY || boundary > STACK_BOUNDARY)
 	    alignment_pad->var = size_binop (MINUS_EXPR, offset_ptr->var,
 					     save_var);
 	}
@@ -3488,7 +3488,7 @@ pad_to_arg_alignment (struct args_size *
 #else
 	    CEIL_ROUND (offset_ptr->constant + sp_offset, boundary_in_bytes);
 #endif
-	    if (boundary > PARM_BOUNDARY && boundary > STACK_BOUNDARY)
+	    if (boundary > PARM_BOUNDARY || boundary > STACK_BOUNDARY)
 	      alignment_pad->constant = offset_ptr->constant - save_constant;
 	}
     }