This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: PREFERRED_STACK_BOUNDARY/function calling code fix


> Note that even with this patch, stack alignment is broken in some cases.
> This was not caused by your patch, but I noticed it while tracking down the
> recent breakage.  For the testcase
You are right, I have prepared a patch for this, but I see you did almost
the same trick.
Note that you need to handle this case even in libcall versions of emit_call
function. I will send a patch for this shortly.

Thanks,
Honza
> 
> void *f (void *p, int i) { return p; }
> int g (int i, int j, int k) { return i; }
> 
> int main ()
> {
>   f (f (0, 1), g (2, 3, 4));
> }
> 
> We get
> 
>         subl    $8, %esp		<--padding for outer call to f
>         subl    $4, %esp		<--padding for call to g
>         pushl   $4
>         pushl   $3
>         pushl   $2
>         call    g
>         pushl   %eax
>         subl    $8, %esp		<--padding for inner call to f
>         pushl   $1
>         pushl   $0
>         call    f
>         addl    $16, %esp
>         pushl   %eax
>         call    f
> 
> Note that the stack alignment is messed up for both of the inner calls,
> because we didn't record anywhere that we had started to push args onto the
> stack for the outer call.  Fixing that (with my patch below), we get
> 
>         subl    $8, %esp
>         subl    $12, %esp
>         pushl   $4
>         pushl   $3
>         pushl   $2
>         call    g
>         addl    $24, %esp
>         pushl   %eax
>         subl    $12, %esp
>         pushl   $1
>         pushl   $0
>         call    f
>         addl    $20, %esp
>         pushl   %eax
>         call    f
> 
> Which has the correct alignment.
> 
> Anyway, the patch:
> 
> 2000-03-02  Jason Merrill  <jason@casey.cygnus.com>
> 
> 	* function.h (struct expr_status): Add x_arg_space_so_far.
> 	(arg_space_so_far): New macro.
> 	* expr.c (init_expr): Initialize it.
> 	* calls.c (emit_call_1): Reset it.
> 	(compute_argument_block_size, expand_call): Use it.
> 	(expand_call, store_one_arg): Increment it.
> 
> Index: expr.c
> ===================================================================
> RCS file: /cvs/gcc/egcs/gcc/expr.c,v
> retrieving revision 1.203
> diff -c -p -r1.203 expr.c
> *** expr.c	2000/02/29 02:34:46	1.203
> --- expr.c	2000/03/02 23:42:26
> *************** init_expr ()
> *** 288,293 ****
> --- 288,294 ----
>   
>     pending_chain = 0;
>     pending_stack_adjust = 0;
> +   arg_space_so_far = 0;
>     inhibit_defer_pop = 0;
>     saveregs_value = 0;
>     apply_args_value = 0;
> Index: function.h
> ===================================================================
> RCS file: /cvs/gcc/egcs/gcc/function.h,v
> retrieving revision 1.46
> diff -c -p -r1.46 function.h
> *** function.h	2000/02/28 09:51:41	1.46
> --- function.h	2000/03/02 23:42:26
> *************** struct expr_status
> *** 130,135 ****
> --- 130,139 ----
>        These are the arguments to function calls that have already returned.  */
>     int x_pending_stack_adjust;
>   
> +   /* Number of units that we should eventually pop off the stack.
> +      These are the arguments to function calls that have not happened yet.  */
> +   int x_arg_space_so_far;
> + 
>     /* Under some ABIs, it is the caller's responsibility to pop arguments
>        pushed for function calls.  A naive implementation would simply pop
>        the arguments immediately after each call.  However, if several
> *************** struct expr_status
> *** 163,168 ****
> --- 167,173 ----
>   };
>   
>   #define pending_stack_adjust (cfun->expr->x_pending_stack_adjust)
> + #define arg_space_so_far (cfun->expr->x_arg_space_so_far)
>   #define inhibit_defer_pop (cfun->expr->x_inhibit_defer_pop)
>   #define saveregs_value (cfun->expr->x_saveregs_value)
>   #define apply_args_value (cfun->expr->x_apply_args_value)
> Index: calls.c
> ===================================================================
> RCS file: /cvs/gcc/egcs/gcc/calls.c,v
> retrieving revision 1.87
> diff -c -p -r1.87 calls.c
> *** calls.c	2000/03/02 11:49:51	1.87
> --- calls.c	2000/03/02 23:42:30
> *************** prepare_call_address (funexp, fndecl, ca
> *** 352,360 ****
>      says that the pointer to this aggregate is to be popped by the callee.
>   
>      STACK_SIZE is the number of bytes of arguments on the stack,
> !    rounded up to PREFERRED_STACK_BOUNDARY; zero if the size is variable.
> !    This is both to put into the call insn and
> !    to generate explicit popping code if necessary.
>   
>      STRUCT_VALUE_SIZE is the number of bytes wanted in a structure value.
>      It is zero if this call doesn't want a structure value.
> --- 352,361 ----
>      says that the pointer to this aggregate is to be popped by the callee.
>   
>      STACK_SIZE is the number of bytes of arguments on the stack,
> !    ROUNDED_STACK_SIZE is that number rounded up to
> !    PREFERRED_STACK_BOUNDARY; zero if the size is variable.  This is
> !    both to put into the call insn and to generate explicit popping
> !    code if necessary.
>   
>      STRUCT_VALUE_SIZE is the number of bytes wanted in a structure value.
>      It is zero if this call doesn't want a structure value.
> *************** emit_call_1 (funexp, fndecl, funtype, st
> *** 502,507 ****
> --- 503,512 ----
>        If returning from the subroutine does pop the args, indicate that the
>        stack pointer will be changed.  */
>   
> +   /* The space for the args is no longer waiting for the call; either it
> +      was popped by the call, or it'll be popped below.  */
> +   arg_space_so_far -= rounded_stack_size;
> + 
>     if (n_popped > 0)
>       {
>         if (!already_popped)
> *************** compute_argument_block_size (reg_parm_st
> *** 1219,1228 ****
> --- 1224,1235 ----
>   #ifdef PREFERRED_STACK_BOUNDARY
>         preferred_stack_boundary /= BITS_PER_UNIT;
>         args_size->constant = (((args_size->constant
> + 			       + arg_space_so_far
>   			       + pending_stack_adjust
>   			       + preferred_stack_boundary - 1)
>   			      / preferred_stack_boundary
>   			      * preferred_stack_boundary)
> + 			     - arg_space_so_far
>   			     - pending_stack_adjust);
>   #endif
>   
> *************** expand_call (exp, target, ignore)
> *** 2285,2290 ****
> --- 2292,2298 ----
>   	{
>   	  args_size.constant = (unadjusted_args_size
>   			        + ((pending_stack_adjust + args_size.constant
> + 				    + arg_space_so_far
>   				    - unadjusted_args_size)
>   			           % (preferred_stack_boundary / BITS_PER_UNIT)));
>   	  pending_stack_adjust -= args_size.constant - unadjusted_args_size;
> *************** expand_call (exp, target, ignore)
> *** 2292,2297 ****
> --- 2300,2310 ----
>   	}
>         else if (argblock == 0)
>   	anti_adjust_stack (GEN_INT (args_size.constant - unadjusted_args_size));
> +       arg_space_so_far += args_size.constant - unadjusted_args_size;
> + 
> +       /* Now that the stack is properly aligned, pops can't safely
> + 	 be deferred during the evaluation of the arguments.  */
> +       NO_DEFER_POP;
>       }
>   #endif
>   #endif
> *************** store_one_arg (arg, argblock, may_be_all
> *** 4061,4066 ****
> --- 4074,4080 ----
>   		      ARGS_SIZE_RTX (arg->offset), reg_parm_stack_space,
>   		      ARGS_SIZE_RTX (arg->alignment_pad));
>   
> +       arg_space_so_far += used;
>       }
>     else
>       {
> *************** store_one_arg (arg, argblock, may_be_all
> *** 4088,4093 ****
> --- 4102,4108 ----
>   	  excess = (arg->size.constant - int_size_in_bytes (TREE_TYPE (pval))
>   		    + partial * UNITS_PER_WORD);
>   	  size_rtx = expr_size (pval);
> + 	  arg_space_so_far += excess + INTVAL (size_rtx);
>   	}
>   
>         emit_push_insn (arg->value, arg->mode, TREE_TYPE (pval), size_rtx,

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]