This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PREFERRED_STACK_BOUNDARY/function calling code fix
- To: Jason Merrill <jason at cygnus dot com>
- Subject: Re: PREFERRED_STACK_BOUNDARY/function calling code fix
- From: Jan Hubicka <hubicka at atrey dot karlin dot mff dot cuni dot cz>
- Date: Fri, 3 Mar 2000 12:52:35 +0100
- Cc: grahams <grahams at rcp dot co dot uk>, egcs-patches at egcs dot cygnus dot com, rth at cygnus dot com
- References: <20000223165651.E32692@atrey.karlin.mff.cuni.cz> <38BAF586.FE2CABC8@rcp.co.uk> <20000301124026.A24452@atrey.karlin.mff.cuni.cz> <u97lflc4sk.fsf@yorick.cygnus.com>
> 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,