[PATCH/RFC] ARM -- Implement ATPCS stack alignment rules

Richard Earnshaw rearnsha@cambridge.arm.com
Mon Sep 9 05:14:00 GMT 2002


> This is another patch that originates from Richard Earnshaw, and
> that has been in-use in NetBSD's 2.95.3-based compiler for some time.
> 
> It implements the ATPCS stack alignment rules, which is to say, the stack
> must always by aligned to an 8-byte boundary on entry to a function call
> (and thus non-leaf functions must have stack frames which are multiples
> of 8 bytes).
> 
> I'm not asking for approval of the patch yet, because I have more testing
> to do (I've tested it with arm-elf sim, with no regressions, to make sure
> the non-ATPCS case is not broken, but have not yet actually tested the
> ATPCS case, and the function prologue/epilogue code in 3.3 is quite a bit
> different than in 2.95.3), and there are some things I'm unsure about, and
> would like to get some feedback on.

You also need to test that -mthumb does not regress either, since you are 
making changes to the thumb code as well.

> 
> First of all, note that since arm_get_frame_size() always returns a rounded
> size, some uses of the ROUND_UP() macro were eliminated.
> 
> Also, the new arm_get_frame_size() function always rounds to at least a
> 4-byte boundary.  The diff will show that there are some parts of the code
> which were using get_frame_size() in an unprotected fashion (that is, not
> 4-byte-rounded).  These places are:
> 
> 	- use_return_insn: This one should be no problem, because the
> 	  test was really just for "was there a stack frame".
> 
> 	- arm_output_epilogue: This one actually used the raw frame
> 	  size from get_frame_size to generate an add insn to pop the
> 	  stack.  As I understand it, get_frame_size() isn't guaranteed
> 	  to return a value that is rounded to STACK_BOUNDARY, so this
> 	  one definitely needs to be sanity-checked.  Does the old code
> 	  actually have a bug?
> 
> 	- arm_expand_prologue: This also used the output of get_frame_size()
> 	  to generate a value directly used in a stack-adjusting insn.
> 
> 	- thumb_expand_prologue: This takes the raw size from
> 	  get_frame_size and then applies ROUND_UP() to it later.  I guess
> 	  I can now eliminate the ROUND_UP() of the value.  A sanity-check
> 	  here is also appreciated.
> 
> 	- thumb_expand_epilogue: This is the same situation is
> 	  thumb_expand_prologue.  Same question applies :-)

I think the code that allocates frame slots will guarantee that they are 
always rounded correctly, so the code is probably just paranoia.

> 
> Also, there's an "XXXJRT" in arm_get_frame_size() that should be
> checked -- basically, I'm wondering if we can skip checking for FPA
> regs if TARGET_SOFT_FLOAT.

All floating point registers will be marked call_used for soft-float (they 
will also be marked fixed, but that's not relevant here), so the loop will 
do nothing.  It is probably cleaner to leave it as it is, since there are 
already too many tests cluttering up some of this code.

> 
> Anyway, to test, I'm going to enable TARGET_ATPCS in the arm-elf config
> and run the testsuite with the arm-elf sim.
> 
> 	* config/arm/arm-protos.h (arm_get_frame_size): New prototype.
> 	* config/arm/arm.c (arm_get_frame_size): New function.
> 	(use_return_insn, arm_output_epilogue, arm_output_function_epilogue)
> 	(arm_compute_initial_elimination_offset, arm_expand_prologue)
> 	(thumb_expand_prologue, thumb_expand_epilogue): Use arm_get_frame_size.
> 	* config/arm/arm.h (PREFERRED_STACK_BOUNDARY): Define.
> 	(THUMB_INITIAL_ELIMINATION_OFFSET): Use arm_get_frame_size.
> 

There's still some work to do on this, since the compiler has been changed 
quite a bit since 2.95 in this respect.

> ***************
> *** 7818,7823 ****
> --- 7818,7826 ----
>       }
>     else
>       {
> +       /* We need to take into accounbt any stack-frame rounding.  */

Typo.

> ***************
> *** 8181,8186 ****
> --- 8184,8255 ----
>       }
>   }
>   
> + /* Calculate the size of the stack frame, taking into account any
> +    padding that is required to ensure stack-alignment.  */
> + 
> + HOST_WIDE_INT
> + arm_get_frame_size ()
> + {
> +   int regno;

This function needs to be rewritten to make use of 
arm_compute_save_reg_mask and arm_compute_save_reg0_reg12_mask.

R.



More information about the Gcc-patches mailing list