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

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]