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

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

[Jason R Thorpe]

[-- Attachment #1: Type: text/plain, Size: 2875 bytes --]

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.

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 :-)

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.

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.

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

[-- Attachment #2: arm-stack-patch --]
[-- Type: text/plain, Size: 7498 bytes --]

Index: config/arm/arm-protos.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm-protos.h,v
retrieving revision 1.32
diff -c -r1.32 arm-protos.h
*** config/arm/arm-protos.h	6 Sep 2002 14:54:48 -0000	1.32
--- config/arm/arm-protos.h	6 Sep 2002 21:37:27 -0000
***************
*** 31,36 ****
--- 31,37 ----
  extern int    arm_volatile_func		PARAMS ((void));
  extern const char * arm_output_epilogue	PARAMS ((int));
  extern void   arm_expand_prologue	PARAMS ((void));
+ extern HOST_WIDE_INT arm_get_frame_size	PARAMS ((void));
  /* Used in arm.md, but defined in output.c.  */
  extern void   assemble_align		PARAMS ((int)); 
  extern const char * arm_strip_name_encoding	PARAMS ((const char *));
Index: config/arm/arm.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.c,v
retrieving revision 1.227
diff -c -r1.227 arm.c
*** config/arm/arm.c	6 Sep 2002 14:54:48 -0000	1.227
--- config/arm/arm.c	6 Sep 2002 21:37:35 -0000
***************
*** 919,925 ****
        /* Of if the function calls __builtin_eh_return () */
        || ARM_FUNC_TYPE (func_type) == ARM_FT_EXCEPTION_HANDLER
        /* Or if there is no frame pointer and there is a stack adjustment.  */
!       || ((get_frame_size () + current_function_outgoing_args_size != 0)
  	  && !frame_pointer_needed))
      return 0;
  
--- 919,925 ----
        /* Of if the function calls __builtin_eh_return () */
        || ARM_FUNC_TYPE (func_type) == ARM_FT_EXCEPTION_HANDLER
        /* Or if there is no frame pointer and there is a stack adjustment.  */
!       || ((arm_get_frame_size () + current_function_outgoing_args_size != 0)
  	  && !frame_pointer_needed))
      return 0;
  
***************
*** 7545,7551 ****
       frame that is $fp + 4 for a non-variadic function.  */
    int floats_offset = 0;
    rtx operands[3];
!   int frame_size = get_frame_size ();
    FILE * f = asm_out_file;
    rtx eh_ofs = cfun->machine->eh_epilogue_sp_ofs;
  
--- 7545,7551 ----
       frame that is $fp + 4 for a non-variadic function.  */
    int floats_offset = 0;
    rtx operands[3];
!   int frame_size = arm_get_frame_size ();
    FILE * f = asm_out_file;
    rtx eh_ofs = cfun->machine->eh_epilogue_sp_ofs;
  
***************
*** 7818,7823 ****
--- 7818,7826 ----
      }
    else
      {
+       /* We need to take into accounbt any stack-frame rounding.  */
+       frame_size = arm_get_frame_size ();
+ 
        if (use_return_insn (FALSE)
  	  && return_used_this_function
  	  && (frame_size + current_function_outgoing_args_size) != 0
***************
*** 8060,8066 ****
       unsigned int from;
       unsigned int to;
  {
!   unsigned int local_vars    = (get_frame_size () + 3) & ~3;
    unsigned int outgoing_args = current_function_outgoing_args_size;
    unsigned int stack_frame;
    unsigned int call_saved_registers;
--- 8063,8069 ----
       unsigned int from;
       unsigned int to;
  {
!   unsigned int local_vars    = arm_get_frame_size ();
    unsigned int outgoing_args = current_function_outgoing_args_size;
    unsigned int stack_frame;
    unsigned int call_saved_registers;
***************
*** 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;
+ 
+   int base_size = ROUND_UP (get_frame_size ());
+   int entry_size = 0;
+   int live_regs_mask = 0;
+   int volatile_func = (optimize > 0
+ 		       && TREE_THIS_VOLATILE (current_function_decl));
+ 
+   if (! TARGET_ATPCS)
+     return base_size;
+ 
+   /* We know that SP will be word aligned on entry, and we must
+      preserve that condition at any subroutine call.  But those are
+      the only constraints.  */
+ 
+   /* Space for variadic functions.  */
+   if (current_function_pretend_args_size)
+     entry_size += current_function_pretend_args_size;
+ 
+   if (! volatile_func)
+     {
+       for (regno = 0; regno <= 10; regno++)
+ 	if (regs_ever_live[regno] && ! call_used_regs[regno])
+ 	  live_regs_mask |= 1 << regno;
+ 
+       if (flag_pic && regs_ever_live[PIC_OFFSET_TABLE_REGNUM])
+ 	live_regs_mask |= 1 << PIC_OFFSET_TABLE_REGNUM;
+ 
+       if (regs_ever_live[14])
+ 	live_regs_mask |= 1 << 14;
+     }
+ 
+   if (frame_pointer_needed)
+     live_regs_mask |= 0xd800;
+ 
+   /* If we have to push any registers, we must also push lr as well.  */
+   if (live_regs_mask)
+     live_regs_mask |= 1 << 14;
+ 
+   for (regno = 0; regno <= LAST_ARM_REGNUM; regno++)
+     if (live_regs_mask & (1 << regno))
+       entry_size += 4;
+ 
+   /* XXXJRT Should this also be conditional on TARGET_HARD_FLOAT?  What
+      happens when extended asm uses FPA regs even if TARGET_SOFT_FLOAT?  */
+   if (! volatile_func)
+     {
+       for (regno = 23; regno > LAST_ARM_REGNUM; regno--)
+ 	if (regs_ever_live[regno] && ! call_used_regs[regno])
+ 	  entry_size += 12;
+     }
+ 
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     base_size += 4;
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     abort ();
+ 
+   return base_size;
+ }
+ 
  /* Generate the prologue instructions for entry into an ARM function.  */
  
  void
***************
*** 8416,8422 ****
  	}
      }
  
!   amount = GEN_INT (-(get_frame_size ()
  		      + current_function_outgoing_args_size));
  
    if (amount != const0_rtx)
--- 8485,8491 ----
  	}
      }
  
!   amount = GEN_INT (-(arm_get_frame_size ()
  		      + current_function_outgoing_args_size));
  
    if (amount != const0_rtx)
***************
*** 10167,10173 ****
  void
  thumb_expand_prologue ()
  {
!   HOST_WIDE_INT amount = (get_frame_size ()
  			  + current_function_outgoing_args_size);
    unsigned long func_type;
  
--- 10236,10242 ----
  void
  thumb_expand_prologue ()
  {
!   HOST_WIDE_INT amount = (arm_get_frame_size ()
  			  + current_function_outgoing_args_size);
    unsigned long func_type;
  
***************
*** 10262,10268 ****
  void
  thumb_expand_epilogue ()
  {
!   HOST_WIDE_INT amount = (get_frame_size ()
  			  + current_function_outgoing_args_size);
    
    /* Naked functions don't have prologues.  */
--- 10331,10337 ----
  void
  thumb_expand_epilogue ()
  {
!   HOST_WIDE_INT amount = (arm_get_frame_size ()
  			  + current_function_outgoing_args_size);
    
    /* Naked functions don't have prologues.  */
Index: config/arm/arm.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.h,v
retrieving revision 1.159
diff -c -r1.159 arm.h
*** config/arm/arm.h	6 Sep 2002 14:54:48 -0000	1.159
--- config/arm/arm.h	6 Sep 2002 21:37:38 -0000
***************
*** 689,694 ****
--- 689,696 ----
  
  #define STACK_BOUNDARY  32
  
+ #define PREFERRED_STACK_BOUNDARY (TARGET_ATPCS ? 64 : 32)
+ 
  #define FUNCTION_BOUNDARY  32
  
  /* The lowest bit is used to indicate Thumb-mode functions, so the
***************
*** 1681,1687 ****
    if ((TO) == STACK_POINTER_REGNUM)					\
      {									\
        (OFFSET) += current_function_outgoing_args_size;			\
!       (OFFSET) += ROUND_UP (get_frame_size ());				\
       }									\
  }
  
--- 1683,1689 ----
    if ((TO) == STACK_POINTER_REGNUM)					\
      {									\
        (OFFSET) += current_function_outgoing_args_size;			\
!       (OFFSET) += arm_get_frame_size ();				\
       }									\
  }
  

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

[Jason R Thorpe]

On Fri, Sep 06, 2002 at 03:40:10PM -0700, Jason R Thorpe wrote:

 > Anyway, to test, I'm going to enable TARGET_ATPCS in the arm-elf config
 > and run the testsuite with the arm-elf sim.

Well, I ran this test, and there were no regressions.  So, I guess I am
asking for approval for the patch, now.

 > 	* 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.

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

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

[Richard Earnshaw]

> 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.

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

[Jason R Thorpe]

...longjmp'ing back to an old thread...

On Mon, Sep 09, 2002 at 01:11:52PM +0100, Richard Earnshaw wrote:

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

Sigh, where does the time go... anyway, my arm-elf sim test environment
now runs with and without -mthumb ... the patch shows no regressions.  A
run without TARGET_ATPCS passed, and a run with TARGET_ATPCS forced also
passed (run against a newlib, etc. which were compiled with the baseline
compiler).  A run with a newlib compiled with a forced-TARGET_ATPCS will
churn away overnight.

 > > + /* 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.

Ok, let me make sure I understand this correctly; we want to simply
use the arm_compute_save_reg_mask return value to compute the saved-regs
space rather than computing that mask ourselves, but no other changes?

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

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

[Jason R Thorpe]

[-- Attachment #1: Type: text/plain, Size: 761 bytes --]

On Mon, Sep 09, 2002 at 01:11:52PM +0100, Richard Earnshaw wrote:

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

Is this what you had in mind?  "make check" currently running...

	* 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.

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

[-- Attachment #2: arm-stack-patch --]
[-- Type: text/plain, Size: 6880 bytes --]

Index: config/arm/arm-protos.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm-protos.h,v
retrieving revision 1.35
diff -c -r1.35 arm-protos.h
*** config/arm/arm-protos.h	20 Oct 2002 22:37:08 -0000	1.35
--- config/arm/arm-protos.h	4 Nov 2002 16:24:34 -0000
***************
*** 31,36 ****
--- 31,37 ----
  extern int    arm_volatile_func		PARAMS ((void));
  extern const char * arm_output_epilogue	PARAMS ((int));
  extern void   arm_expand_prologue	PARAMS ((void));
+ extern HOST_WIDE_INT arm_get_frame_size	PARAMS ((void));
  /* Used in arm.md, but defined in output.c.  */
  extern void   assemble_align		PARAMS ((int)); 
  extern const char * arm_strip_name_encoding	PARAMS ((const char *));
Index: config/arm/arm.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.c,v
retrieving revision 1.240
diff -c -r1.240 arm.c
*** config/arm/arm.c	1 Nov 2002 14:41:57 -0000	1.240
--- config/arm/arm.c	4 Nov 2002 16:24:42 -0000
***************
*** 928,934 ****
        /* Of if the function calls __builtin_eh_return () */
        || ARM_FUNC_TYPE (func_type) == ARM_FT_EXCEPTION_HANDLER
        /* Or if there is no frame pointer and there is a stack adjustment.  */
!       || ((get_frame_size () + current_function_outgoing_args_size != 0)
  	  && !frame_pointer_needed))
      return 0;
  
--- 928,934 ----
        /* Of if the function calls __builtin_eh_return () */
        || ARM_FUNC_TYPE (func_type) == ARM_FT_EXCEPTION_HANDLER
        /* Or if there is no frame pointer and there is a stack adjustment.  */
!       || ((arm_get_frame_size () + current_function_outgoing_args_size != 0)
  	  && !frame_pointer_needed))
      return 0;
  
***************
*** 7564,7570 ****
       frame that is $fp + 4 for a non-variadic function.  */
    int floats_offset = 0;
    rtx operands[3];
!   int frame_size = get_frame_size ();
    FILE * f = asm_out_file;
    rtx eh_ofs = cfun->machine->eh_epilogue_sp_ofs;
  
--- 7564,7570 ----
       frame that is $fp + 4 for a non-variadic function.  */
    int floats_offset = 0;
    rtx operands[3];
!   int frame_size = arm_get_frame_size ();
    FILE * f = asm_out_file;
    rtx eh_ofs = cfun->machine->eh_epilogue_sp_ofs;
  
***************
*** 7846,7851 ****
--- 7846,7854 ----
      }
    else
      {
+       /* We need to take into account any stack-frame rounding.  */
+       frame_size = arm_get_frame_size ();
+ 
        if (use_return_insn (FALSE)
  	  && return_used_this_function
  	  && (frame_size + current_function_outgoing_args_size) != 0
***************
*** 8088,8094 ****
       unsigned int from;
       unsigned int to;
  {
!   unsigned int local_vars    = (get_frame_size () + 3) & ~3;
    unsigned int outgoing_args = current_function_outgoing_args_size;
    unsigned int stack_frame;
    unsigned int call_saved_registers;
--- 8091,8097 ----
       unsigned int from;
       unsigned int to;
  {
!   unsigned int local_vars    = arm_get_frame_size ();
    unsigned int outgoing_args = current_function_outgoing_args_size;
    unsigned int stack_frame;
    unsigned int call_saved_registers;
***************
*** 8209,8214 ****
--- 8212,8263 ----
      }
  }
  
+ /* 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;
+ 
+   int base_size = ROUND_UP (get_frame_size ());
+   int entry_size = 0;
+   int save_regs_mask;
+   unsigned long func_type = arm_current_func_type ();
+ 
+   if (! TARGET_ATPCS)
+     return base_size;
+ 
+   /* We know that SP will be word aligned on entry, and we must
+      preserve that condition at any subroutine call.  But those are
+      the only constraints.  */
+ 
+   /* Space for variadic functions.  */
+   if (current_function_pretend_args_size)
+     entry_size += current_function_pretend_args_size;
+ 
+   /* Space for saved registers.  */
+   save_regs_mask = arm_compute_save_reg_mask ();
+   for (regno = 0; regno <= LAST_ARM_REGNUM; regno++)
+     if (save_regs_mask & (1 << regno))
+       entry_size += 4;
+ 
+   /* Space for saved FPA registers.  */
+   if (! IS_VOLATILE (func_type))
+     {
+       for (regno = 23; regno > LAST_ARM_REGNUM; regno--)
+       if (regs_ever_live[regno] && ! call_used_regs[regno])
+ 	entry_size += 12;
+     }
+ 
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     base_size += 4;
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     abort ();
+ 
+   return base_size;
+ }
+ 
  /* Generate the prologue instructions for entry into an ARM function.  */
  
  void
***************
*** 8444,8450 ****
  	}
      }
  
!   amount = GEN_INT (-(get_frame_size ()
  		      + current_function_outgoing_args_size));
  
    if (amount != const0_rtx)
--- 8493,8499 ----
  	}
      }
  
!   amount = GEN_INT (-(arm_get_frame_size ()
  		      + current_function_outgoing_args_size));
  
    if (amount != const0_rtx)
***************
*** 10198,10204 ****
  void
  thumb_expand_prologue ()
  {
!   HOST_WIDE_INT amount = (get_frame_size ()
  			  + current_function_outgoing_args_size);
    unsigned long func_type;
  
--- 10247,10253 ----
  void
  thumb_expand_prologue ()
  {
!   HOST_WIDE_INT amount = (arm_get_frame_size ()
  			  + current_function_outgoing_args_size);
    unsigned long func_type;
  
***************
*** 10293,10299 ****
  void
  thumb_expand_epilogue ()
  {
!   HOST_WIDE_INT amount = (get_frame_size ()
  			  + current_function_outgoing_args_size);
    
    /* Naked functions don't have prologues.  */
--- 10342,10348 ----
  void
  thumb_expand_epilogue ()
  {
!   HOST_WIDE_INT amount = (arm_get_frame_size ()
  			  + current_function_outgoing_args_size);
    
    /* Naked functions don't have prologues.  */
Index: config/arm/arm.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.h,v
retrieving revision 1.166
diff -c -r1.166 arm.h
*** config/arm/arm.h	20 Oct 2002 22:37:09 -0000	1.166
--- config/arm/arm.h	4 Nov 2002 16:24:45 -0000
***************
*** 689,694 ****
--- 689,696 ----
  
  #define STACK_BOUNDARY  32
  
+ #define PREFERRED_STACK_BOUNDARY (TARGET_ATPCS ? 64 : 32)
+ 
  #define FUNCTION_BOUNDARY  32
  
  /* The lowest bit is used to indicate Thumb-mode functions, so the
***************
*** 1679,1685 ****
    if ((TO) == STACK_POINTER_REGNUM)					\
      {									\
        (OFFSET) += current_function_outgoing_args_size;			\
!       (OFFSET) += ROUND_UP (get_frame_size ());				\
       }									\
  }
  
--- 1681,1687 ----
    if ((TO) == STACK_POINTER_REGNUM)					\
      {									\
        (OFFSET) += current_function_outgoing_args_size;			\
!       (OFFSET) += arm_get_frame_size ();				\
       }									\
  }
  

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

[Richard Earnshaw]

> On Mon, Sep 09, 2002 at 01:11:52PM +0100, Richard Earnshaw wrote:
> 
>  > This function needs to be rewritten to make use of 
>  > arm_compute_save_reg_mask and arm_compute_save_reg0_reg12_mask.
> 
> Is this what you had in mind?  "make check" currently running...
> 
> 	* 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.

We're getting there.  However, as things stand this is incorrect for 
Thumb.  arm_compute_saved_reg_mask can't be used for a thumb function[*], 
so either we need to handle Thumb specifically in arm_get_frame_size or we 
need a thumb_get_frame_size function to handle that case.

R.

[*] The basic saved regs are computed correctly, but the behaviour differs 
for handling a frame pointer and other uses of fixed regs.

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

[Richard Earnshaw]

>  > > + /* 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.
> 
> Ok, let me make sure I understand this correctly; we want to simply
> use the arm_compute_save_reg_mask return value to compute the saved-regs
> space rather than computing that mask ourselves, but no other changes?

The aim is to avoid having multiple functions that have to scan the raw 
information about which registers need saving and then applying further 
rules about which other registers have to be saved.  All that leads to 
hard-to-maintain code, which of course leads to bugs...

arm_compute_saved_reg_mask will give you a bitmask of the integer 
registers that need saving.  You can then count the number of bits set in 
the mask to find the amount of storage required for those registers.

R.

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

[Jason R Thorpe]

[-- Attachment #1: Type: text/plain, Size: 1065 bytes --]

On Mon, Nov 04, 2002 at 05:04:03PM +0000, Richard Earnshaw wrote:

 > We're getting there.  However, as things stand this is incorrect for 
 > Thumb.  arm_compute_saved_reg_mask can't be used for a thumb function[*], 
 > so either we need to handle Thumb specifically in arm_get_frame_size or we 
 > need a thumb_get_frame_size function to handle that case.

Ok, I'm currently regtesting the following... please make sure I
understood the rules right in thumb_get_frame_size()...

	* config/arm/arm-protos.h (arm_get_frame_size)
	(thumb_get_frame_size): New prototypes.
	* config/arm/arm.c (arm_get_frame_size)
	(thumb_get_frame_size): New functions.
	(use_return_insn, arm_output_epilogue, arm_output_function_epilogue)
	(arm_compute_initial_elimination_offset, arm_expand_prologue): Use
	arm_get_frame_size.
	(thumb_expand_prologue, thumb_expand_epilogue): Use
	thumb_get_frame_size.
	* config/arm/arm.h (PREFERRED_STACK_BOUNDARY): Define.
	(THUMB_INITIAL_ELIMINATION_OFFSET): Use thumb_get_frame_size.

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

[-- Attachment #2: arm-stack-patch --]
[-- Type: text/plain, Size: 9092 bytes --]

Index: config/arm/arm-protos.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm-protos.h,v
retrieving revision 1.35
diff -c -r1.35 arm-protos.h
*** config/arm/arm-protos.h	20 Oct 2002 22:37:08 -0000	1.35
--- config/arm/arm-protos.h	4 Nov 2002 23:25:53 -0000
***************
*** 31,36 ****
--- 31,37 ----
  extern int    arm_volatile_func		PARAMS ((void));
  extern const char * arm_output_epilogue	PARAMS ((int));
  extern void   arm_expand_prologue	PARAMS ((void));
+ extern HOST_WIDE_INT arm_get_frame_size	PARAMS ((void));
  /* Used in arm.md, but defined in output.c.  */
  extern void   assemble_align		PARAMS ((int)); 
  extern const char * arm_strip_name_encoding	PARAMS ((const char *));
***************
*** 160,165 ****
--- 161,167 ----
  extern void   arm_init_expanders	PARAMS ((void));
  extern int    thumb_far_jump_used_p	PARAMS ((int));
  extern const char * thumb_unexpanded_epilogue	PARAMS ((void));
+ extern HOST_WIDE_INT thumb_get_frame_size PARAMS ((void));
  extern void   thumb_expand_prologue	PARAMS ((void));
  extern void   thumb_expand_epilogue	PARAMS ((void));
  #ifdef TREE_CODE
Index: config/arm/arm.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.c,v
retrieving revision 1.240
diff -c -r1.240 arm.c
*** config/arm/arm.c	1 Nov 2002 14:41:57 -0000	1.240
--- config/arm/arm.c	4 Nov 2002 23:26:01 -0000
***************
*** 928,934 ****
        /* Of if the function calls __builtin_eh_return () */
        || ARM_FUNC_TYPE (func_type) == ARM_FT_EXCEPTION_HANDLER
        /* Or if there is no frame pointer and there is a stack adjustment.  */
!       || ((get_frame_size () + current_function_outgoing_args_size != 0)
  	  && !frame_pointer_needed))
      return 0;
  
--- 928,934 ----
        /* Of if the function calls __builtin_eh_return () */
        || ARM_FUNC_TYPE (func_type) == ARM_FT_EXCEPTION_HANDLER
        /* Or if there is no frame pointer and there is a stack adjustment.  */
!       || ((arm_get_frame_size () + current_function_outgoing_args_size != 0)
  	  && !frame_pointer_needed))
      return 0;
  
***************
*** 7564,7570 ****
       frame that is $fp + 4 for a non-variadic function.  */
    int floats_offset = 0;
    rtx operands[3];
!   int frame_size = get_frame_size ();
    FILE * f = asm_out_file;
    rtx eh_ofs = cfun->machine->eh_epilogue_sp_ofs;
  
--- 7564,7570 ----
       frame that is $fp + 4 for a non-variadic function.  */
    int floats_offset = 0;
    rtx operands[3];
!   int frame_size = arm_get_frame_size ();
    FILE * f = asm_out_file;
    rtx eh_ofs = cfun->machine->eh_epilogue_sp_ofs;
  
***************
*** 7846,7851 ****
--- 7846,7854 ----
      }
    else
      {
+       /* We need to take into account any stack-frame rounding.  */
+       frame_size = arm_get_frame_size ();
+ 
        if (use_return_insn (FALSE)
  	  && return_used_this_function
  	  && (frame_size + current_function_outgoing_args_size) != 0
***************
*** 8088,8094 ****
       unsigned int from;
       unsigned int to;
  {
!   unsigned int local_vars    = (get_frame_size () + 3) & ~3;
    unsigned int outgoing_args = current_function_outgoing_args_size;
    unsigned int stack_frame;
    unsigned int call_saved_registers;
--- 8091,8097 ----
       unsigned int from;
       unsigned int to;
  {
!   unsigned int local_vars    = arm_get_frame_size ();
    unsigned int outgoing_args = current_function_outgoing_args_size;
    unsigned int stack_frame;
    unsigned int call_saved_registers;
***************
*** 8209,8214 ****
--- 8212,8266 ----
      }
  }
  
+ /* 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;
+ 
+   int base_size = ROUND_UP (get_frame_size ());
+   int entry_size = 0;
+   int save_regs_mask;
+   unsigned long func_type = arm_current_func_type ();
+ 
+   if (! TARGET_ARM)
+     abort();
+ 
+   if (! TARGET_ATPCS)
+     return base_size;
+ 
+   /* We know that SP will be word aligned on entry, and we must
+      preserve that condition at any subroutine call.  But those are
+      the only constraints.  */
+ 
+   /* Space for variadic functions.  */
+   if (current_function_pretend_args_size)
+     entry_size += current_function_pretend_args_size;
+ 
+   /* Space for saved registers.  */
+   save_regs_mask = arm_compute_save_reg_mask ();
+   for (regno = 0; regno <= LAST_ARM_REGNUM; regno++)
+     if (save_regs_mask & (1 << regno))
+       entry_size += 4;
+ 
+   /* Space for saved FPA registers.  */
+   if (! IS_VOLATILE (func_type))
+     {
+       for (regno = FIRST_ARM_FP_REGNUM; regno <= LAST_ARM_FP_REGNUM; regno++)
+       if (regs_ever_live[regno] && ! call_used_regs[regno])
+ 	entry_size += 12;
+     }
+ 
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     base_size += 4;
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     abort ();
+ 
+   return base_size;
+ }
+ 
  /* Generate the prologue instructions for entry into an ARM function.  */
  
  void
***************
*** 8444,8450 ****
  	}
      }
  
!   amount = GEN_INT (-(get_frame_size ()
  		      + current_function_outgoing_args_size));
  
    if (amount != const0_rtx)
--- 8496,8502 ----
  	}
      }
  
!   amount = GEN_INT (-(arm_get_frame_size ()
  		      + current_function_outgoing_args_size));
  
    if (amount != const0_rtx)
***************
*** 10193,10204 ****
    init_machine_status = arm_init_machine_status;
  }
  
  /* Generate the rest of a function's prologue.  */
  
  void
  thumb_expand_prologue ()
  {
!   HOST_WIDE_INT amount = (get_frame_size ()
  			  + current_function_outgoing_args_size);
    unsigned long func_type;
  
--- 10245,10314 ----
    init_machine_status = arm_init_machine_status;
  }
  
+ HOST_WIDE_INT
+ thumb_get_frame_size ()
+ {
+   int regno;
+ 
+   int base_size = ROUND_UP (get_frame_size ());
+   int count_regs = 0;
+   int entry_size = 0;
+ 
+   if (! TARGET_THUMB)
+     abort ();
+ 
+   if (! TARGET_ATPCS)
+     return base_size;
+ 
+   /* We know that SP will be word aligned on entry, and we must
+      preserve that condition at any subroutine call.  But those are
+      the only constraints.  */
+ 
+   /* Space for variadic functions.  */
+   if (current_function_pretend_args_size)
+     entry_size += current_function_pretend_args_size;
+ 
+   /* Space for pushed lo registers.  */
+   for (regno = 0; regno <= LAST_LO_REGNUM; regno++)
+     if (THUMB_REG_PUSHED_P (regno))
+       count_regs++;
+ 
+   /* Space for backtrace structure.  */
+   if (TARGET_BACKTRACE)
+     {
+       if (count_regs == 0 && regs_ever_live[LAST_ARG_REGNUM] != 0)
+ 	entry_size += 20;
+       else
+ 	entry_size += 16;
+     }
+ 
+   if (count_regs || !leaf_function_p () || thumb_far_jump_used_p (1))
+     count_regs++;	/* LR */
+ 
+   entry_size += count_regs * 4;
+   count_regs = 0;
+ 
+   /* Space for pushed hi regs.  */
+   for (regno = 8; regno < 13; regno++)
+     if (THUMB_REG_PUSHED_P (regno))
+       count_regs++;
+ 
+   entry_size += count_regs * 4;
+ 
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     base_size += 4;
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     abort ();
+ 
+   return base_size;
+ }
+ 
  /* Generate the rest of a function's prologue.  */
  
  void
  thumb_expand_prologue ()
  {
!   HOST_WIDE_INT amount = (thumb_get_frame_size ()
  			  + current_function_outgoing_args_size);
    unsigned long func_type;
  
***************
*** 10293,10299 ****
  void
  thumb_expand_epilogue ()
  {
!   HOST_WIDE_INT amount = (get_frame_size ()
  			  + current_function_outgoing_args_size);
    
    /* Naked functions don't have prologues.  */
--- 10403,10409 ----
  void
  thumb_expand_epilogue ()
  {
!   HOST_WIDE_INT amount = (thumb_get_frame_size ()
  			  + current_function_outgoing_args_size);
    
    /* Naked functions don't have prologues.  */
Index: config/arm/arm.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.h,v
retrieving revision 1.166
diff -c -r1.166 arm.h
*** config/arm/arm.h	20 Oct 2002 22:37:09 -0000	1.166
--- config/arm/arm.h	4 Nov 2002 23:26:03 -0000
***************
*** 689,694 ****
--- 689,696 ----
  
  #define STACK_BOUNDARY  32
  
+ #define PREFERRED_STACK_BOUNDARY (TARGET_ATPCS ? 64 : 32)
+ 
  #define FUNCTION_BOUNDARY  32
  
  /* The lowest bit is used to indicate Thumb-mode functions, so the
***************
*** 1679,1685 ****
    if ((TO) == STACK_POINTER_REGNUM)					\
      {									\
        (OFFSET) += current_function_outgoing_args_size;			\
!       (OFFSET) += ROUND_UP (get_frame_size ());				\
       }									\
  }
  
--- 1681,1687 ----
    if ((TO) == STACK_POINTER_REGNUM)					\
      {									\
        (OFFSET) += current_function_outgoing_args_size;			\
!       (OFFSET) += thumb_get_frame_size ();				\
       }									\
  }
  

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

[Richard Earnshaw]

[-- Attachment #1: Type: text/plain, Size: 1244 bytes --]

> On Mon, Nov 04, 2002 at 05:04:03PM +0000, Richard Earnshaw wrote:
> 
>  > We're getting there.  However, as things stand this is incorrect for 
>  > Thumb.  arm_compute_saved_reg_mask can't be used for a thumb function[*], 
>  > so either we need to handle Thumb specifically in arm_get_frame_size or we 
>  > need a thumb_get_frame_size function to handle that case.
> 
> Ok, I'm currently regtesting the following... please make sure I
> understood the rules right in thumb_get_frame_size()...

That looks ok to me.

Can you please roll the attached patch in and then use bit_count() to 
simplify the following part of arm_get_frame_size():

+   for (regno = 0; regno <= LAST_ARM_REGNUM; regno++)
+     if (save_regs_mask & (1 << regno))
+       entry_size += 4;

Once that's done I think we are there; though a good eyeballing of some 
generated thumb code wouldn't go amiss.

R.

2002-11-05  Richard Earnshaw  (rearnsha@arm.com)

	* arm.c (bit_count): Make argument unsigned long.  Return unsigned.
	Adjust code to use portable unsigned bit manipulation.
	(insn_flags, tune_flags): Change type to unsigned.
	(struct processors): Make flags unsigned long.
	(arm_override_options): Change type of count and current_bit_count
	to unsigned.


[-- Attachment #2: countbits.patch --]
[-- Type: text/x-patch , Size: 4524 bytes --]

Index: arm.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.c,v
retrieving revision 1.240
diff -p -r1.240 arm.c
*** arm.c	1 Nov 2002 14:41:57 -0000	1.240
--- arm.c	5 Nov 2002 14:45:02 -0000
*************** const struct attribute_spec arm_attribut
*** 65,71 ****
  /* Forward function declarations.  */
  static void      arm_add_gc_roots 		PARAMS ((void));
  static int       arm_gen_constant		PARAMS ((enum rtx_code, Mmode, Hint, rtx, rtx, int, int));
! static Ulong     bit_count 			PARAMS ((signed int));
  static int       const_ok_for_op 		PARAMS ((Hint, enum rtx_code));
  static int       eliminate_lr2ip		PARAMS ((rtx *));
  static rtx	 emit_multi_reg_push		PARAMS ((int));
--- 65,71 ----
  /* Forward function declarations.  */
  static void      arm_add_gc_roots 		PARAMS ((void));
  static int       arm_gen_constant		PARAMS ((enum rtx_code, Mmode, Hint, rtx, rtx, int, int));
! static unsigned  bit_count 			PARAMS ((Ulong));
  static int       const_ok_for_op 		PARAMS ((Hint, enum rtx_code));
  static int       eliminate_lr2ip		PARAMS ((rtx *));
  static rtx	 emit_multi_reg_push		PARAMS ((int));
*************** int    arm_structure_size_boundary = DEF
*** 246,258 ****
  
  /* The bits in this mask specify which
     instructions we are allowed to generate.  */
! static int insn_flags = 0;
  
  /* The bits in this mask specify which instruction scheduling options should
     be used.  Note - there is an overlap with the FL_FAST_MULT.  For some
     hardware we want to be able to generate the multiply instructions, but to
     tune as if they were not present in the architecture.  */
! static int tune_flags = 0;
  
  /* The following are used in the arm.md file as equivalents to bits
     in the above two flag variables.  */
--- 246,258 ----
  
  /* The bits in this mask specify which
     instructions we are allowed to generate.  */
! static unsigned long insn_flags = 0;
  
  /* The bits in this mask specify which instruction scheduling options should
     be used.  Note - there is an overlap with the FL_FAST_MULT.  For some
     hardware we want to be able to generate the multiply instructions, but to
     tune as if they were not present in the architecture.  */
! static unsigned long tune_flags = 0;
  
  /* The following are used in the arm.md file as equivalents to bits
     in the above two flag variables.  */
*************** static const char * const arm_condition_
*** 324,330 ****
  struct processors
  {
    const char *const name;
!   const unsigned int flags;
  };
  
  /* Not all of these give usefully different compilation alternatives,
--- 324,330 ----
  struct processors
  {
    const char *const name;
!   const unsigned long flags;
  };
  
  /* Not all of these give usefully different compilation alternatives,
*************** struct arm_cpu_select arm_select[] =
*** 412,428 ****
    { NULL,	"-mtune=",	all_cores }
  };
  
! /* Return the number of bits set in value' */
! static unsigned long
  bit_count (value)
!      signed int value;
  {
    unsigned long count = 0;
    
    while (value)
      {
!       value &= ~(value & -value);
!       ++count;
      }
  
    return count;
--- 412,428 ----
    { NULL,	"-mtune=",	all_cores }
  };
  
! /* Return the number of bits set in VALUE.  */
! static unsigned
  bit_count (value)
!      unsigned long value;
  {
    unsigned long count = 0;
    
    while (value)
      {
!       count++;
!       value &= value - 1;  /* Clear the least-significant set bit.  */
      }
  
    return count;
*************** arm_override_options ()
*** 549,555 ****
  
  	  if (sel->name == NULL)
  	    {
! 	      unsigned int        current_bit_count = 0;
  	      const struct processors * best_fit = NULL;
  	      
  	      /* Ideally we would like to issue an error message here
--- 549,555 ----
  
  	  if (sel->name == NULL)
  	    {
! 	      unsigned current_bit_count = 0;
  	      const struct processors * best_fit = NULL;
  	      
  	      /* Ideally we would like to issue an error message here
*************** arm_override_options ()
*** 568,574 ****
  	      for (sel = all_cores; sel->name != NULL; sel++)
  		if ((sel->flags & sought) == sought)
  		  {
! 		    unsigned int count;
  
  		    count = bit_count (sel->flags & insn_flags);
  
--- 568,574 ----
  	      for (sel = all_cores; sel->name != NULL; sel++)
  		if ((sel->flags & sought) == sought)
  		  {
! 		    unsigned count;
  
  		    count = bit_count (sel->flags & insn_flags);
  

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

[Jason R Thorpe]

On Tue, Nov 05, 2002 at 03:05:04PM +0000, Richard Earnshaw wrote:

 > That looks ok to me.

Hm, it appears to be "not quite right".  There are some regressions
for thumb with my latest patch.  I'm going to double-check the corner
cases in the prologue code...

 > Once that's done I think we are there; though a good eyeballing of some 
 > generated thumb code wouldn't go amiss.

OK, I've folded this in.  Lemme try and track down the source of these
regressions, and I'll post another patch...

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

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

[Jason R Thorpe]

[-- Attachment #1: Type: text/plain, Size: 1523 bytes --]

On Tue, Nov 05, 2002 at 08:12:34AM -0800, Jason R Thorpe wrote:

 > OK, I've folded this in.  Lemme try and track down the source of these
 > regressions, and I'll post another patch...

Ok, the source of the Thumb regressions has been found (thanks a million,
Richard :-), and an arm-elf sim testsuite run is in-progress.

OK for mainline if it passes?

2002-11-06  Jason Thorpe  <thorpej@wasabisystems.com> 

        * config/arm/arm-protos.h (arm_get_frame_size)
        (thumb_get_frame_size): New prototypes.
        * config/arm/arm.c (arm_get_frame_size)
        (thumb_get_frame_size): New functions.
        (use_return_insn, arm_output_epilogue, arm_output_function_epilogue)
        (arm_compute_initial_elimination_offset, arm_expand_prologue): Use
        arm_get_frame_size.
        (thumb_expand_prologue, thumb_expand_epilogue): Use
        thumb_get_frame_size.
        * config/arm/arm.h (PREFERRED_STACK_BOUNDARY): Define.
        (machine_function): Add frame_size member.
        (THUMB_INITIAL_ELIMINATION_OFFSET): Use thumb_get_frame_size.

2002-11-05  Richard Earnshaw  (rearnsha@arm.com)

        * arm.c (bit_count): Make argument unsigned long.  Return unsigned.
        Adjust code to use portable unsigned bit manipulation.
        (insn_flags, tune_flags): Change type to unsigned.
        (struct processors): Make flags unsigned long.
        (arm_override_options): Change type of count and current_bit_count
        to unsigned.

-- 
        -- Jason R. Thorpe <thorpej@wasabisystems.com>

[-- Attachment #2: arm-stack-patch --]
[-- Type: text/plain, Size: 14157 bytes --]

Index: config/arm/arm-protos.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm-protos.h,v
retrieving revision 1.35
diff -c -r1.35 arm-protos.h
*** config/arm/arm-protos.h	20 Oct 2002 22:37:08 -0000	1.35
--- config/arm/arm-protos.h	6 Nov 2002 18:16:09 -0000
***************
*** 31,36 ****
--- 31,37 ----
  extern int    arm_volatile_func		PARAMS ((void));
  extern const char * arm_output_epilogue	PARAMS ((int));
  extern void   arm_expand_prologue	PARAMS ((void));
+ extern HOST_WIDE_INT arm_get_frame_size	PARAMS ((void));
  /* Used in arm.md, but defined in output.c.  */
  extern void   assemble_align		PARAMS ((int)); 
  extern const char * arm_strip_name_encoding	PARAMS ((const char *));
***************
*** 160,165 ****
--- 161,167 ----
  extern void   arm_init_expanders	PARAMS ((void));
  extern int    thumb_far_jump_used_p	PARAMS ((int));
  extern const char * thumb_unexpanded_epilogue	PARAMS ((void));
+ extern HOST_WIDE_INT thumb_get_frame_size PARAMS ((void));
  extern void   thumb_expand_prologue	PARAMS ((void));
  extern void   thumb_expand_epilogue	PARAMS ((void));
  #ifdef TREE_CODE
Index: config/arm/arm.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.c,v
retrieving revision 1.240
diff -c -r1.240 arm.c
*** config/arm/arm.c	1 Nov 2002 14:41:57 -0000	1.240
--- config/arm/arm.c	6 Nov 2002 18:16:18 -0000
***************
*** 65,71 ****
  /* Forward function declarations.  */
  static void      arm_add_gc_roots 		PARAMS ((void));
  static int       arm_gen_constant		PARAMS ((enum rtx_code, Mmode, Hint, rtx, rtx, int, int));
! static Ulong     bit_count 			PARAMS ((signed int));
  static int       const_ok_for_op 		PARAMS ((Hint, enum rtx_code));
  static int       eliminate_lr2ip		PARAMS ((rtx *));
  static rtx	 emit_multi_reg_push		PARAMS ((int));
--- 65,71 ----
  /* Forward function declarations.  */
  static void      arm_add_gc_roots 		PARAMS ((void));
  static int       arm_gen_constant		PARAMS ((enum rtx_code, Mmode, Hint, rtx, rtx, int, int));
! static unsigned  bit_count 			PARAMS ((Ulong));
  static int       const_ok_for_op 		PARAMS ((Hint, enum rtx_code));
  static int       eliminate_lr2ip		PARAMS ((rtx *));
  static rtx	 emit_multi_reg_push		PARAMS ((int));
***************
*** 246,258 ****
  
  /* The bits in this mask specify which
     instructions we are allowed to generate.  */
! static int insn_flags = 0;
  
  /* The bits in this mask specify which instruction scheduling options should
     be used.  Note - there is an overlap with the FL_FAST_MULT.  For some
     hardware we want to be able to generate the multiply instructions, but to
     tune as if they were not present in the architecture.  */
! static int tune_flags = 0;
  
  /* The following are used in the arm.md file as equivalents to bits
     in the above two flag variables.  */
--- 246,258 ----
  
  /* The bits in this mask specify which
     instructions we are allowed to generate.  */
! static unsigned long insn_flags = 0;
  
  /* The bits in this mask specify which instruction scheduling options should
     be used.  Note - there is an overlap with the FL_FAST_MULT.  For some
     hardware we want to be able to generate the multiply instructions, but to
     tune as if they were not present in the architecture.  */
! static unsigned long tune_flags = 0;
  
  /* The following are used in the arm.md file as equivalents to bits
     in the above two flag variables.  */
***************
*** 324,330 ****
  struct processors
  {
    const char *const name;
!   const unsigned int flags;
  };
  
  /* Not all of these give usefully different compilation alternatives,
--- 324,330 ----
  struct processors
  {
    const char *const name;
!   const unsigned long flags;
  };
  
  /* Not all of these give usefully different compilation alternatives,
***************
*** 412,428 ****
    { NULL,	"-mtune=",	all_cores }
  };
  
! /* Return the number of bits set in value' */
! static unsigned long
  bit_count (value)
!      signed int value;
  {
    unsigned long count = 0;
    
    while (value)
      {
!       value &= ~(value & -value);
!       ++count;
      }
  
    return count;
--- 412,428 ----
    { NULL,	"-mtune=",	all_cores }
  };
  
! /* Return the number of bits set in VALUE.  */
! static unsigned
  bit_count (value)
!      unsigned long value;
  {
    unsigned long count = 0;
    
    while (value)
      {
!       count++;
!       value &= value - 1;  /* Clear the least-significant set bit.  */
      }
  
    return count;
***************
*** 549,555 ****
  
  	  if (sel->name == NULL)
  	    {
! 	      unsigned int        current_bit_count = 0;
  	      const struct processors * best_fit = NULL;
  	      
  	      /* Ideally we would like to issue an error message here
--- 549,555 ----
  
  	  if (sel->name == NULL)
  	    {
! 	      unsigned current_bit_count = 0;
  	      const struct processors * best_fit = NULL;
  	      
  	      /* Ideally we would like to issue an error message here
***************
*** 568,574 ****
  	      for (sel = all_cores; sel->name != NULL; sel++)
  		if ((sel->flags & sought) == sought)
  		  {
! 		    unsigned int count;
  
  		    count = bit_count (sel->flags & insn_flags);
  
--- 568,574 ----
  	      for (sel = all_cores; sel->name != NULL; sel++)
  		if ((sel->flags & sought) == sought)
  		  {
! 		    unsigned count;
  
  		    count = bit_count (sel->flags & insn_flags);
  
***************
*** 928,934 ****
        /* Of if the function calls __builtin_eh_return () */
        || ARM_FUNC_TYPE (func_type) == ARM_FT_EXCEPTION_HANDLER
        /* Or if there is no frame pointer and there is a stack adjustment.  */
!       || ((get_frame_size () + current_function_outgoing_args_size != 0)
  	  && !frame_pointer_needed))
      return 0;
  
--- 928,934 ----
        /* Of if the function calls __builtin_eh_return () */
        || ARM_FUNC_TYPE (func_type) == ARM_FT_EXCEPTION_HANDLER
        /* Or if there is no frame pointer and there is a stack adjustment.  */
!       || ((arm_get_frame_size () + current_function_outgoing_args_size != 0)
  	  && !frame_pointer_needed))
      return 0;
  
***************
*** 7564,7570 ****
       frame that is $fp + 4 for a non-variadic function.  */
    int floats_offset = 0;
    rtx operands[3];
!   int frame_size = get_frame_size ();
    FILE * f = asm_out_file;
    rtx eh_ofs = cfun->machine->eh_epilogue_sp_ofs;
  
--- 7564,7570 ----
       frame that is $fp + 4 for a non-variadic function.  */
    int floats_offset = 0;
    rtx operands[3];
!   int frame_size = arm_get_frame_size ();
    FILE * f = asm_out_file;
    rtx eh_ofs = cfun->machine->eh_epilogue_sp_ofs;
  
***************
*** 7846,7851 ****
--- 7846,7854 ----
      }
    else
      {
+       /* We need to take into account any stack-frame rounding.  */
+       frame_size = arm_get_frame_size ();
+ 
        if (use_return_insn (FALSE)
  	  && return_used_this_function
  	  && (frame_size + current_function_outgoing_args_size) != 0
***************
*** 8088,8094 ****
       unsigned int from;
       unsigned int to;
  {
!   unsigned int local_vars    = (get_frame_size () + 3) & ~3;
    unsigned int outgoing_args = current_function_outgoing_args_size;
    unsigned int stack_frame;
    unsigned int call_saved_registers;
--- 8091,8097 ----
       unsigned int from;
       unsigned int to;
  {
!   unsigned int local_vars    = arm_get_frame_size ();
    unsigned int outgoing_args = current_function_outgoing_args_size;
    unsigned int stack_frame;
    unsigned int call_saved_registers;
***************
*** 8209,8214 ****
--- 8212,8262 ----
      }
  }
  
+ /* 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;
+ 
+   int base_size = ROUND_UP (get_frame_size ());
+   int entry_size = 0;
+   unsigned long func_type = arm_current_func_type ();
+ 
+   if (! TARGET_ARM)
+     abort();
+ 
+   if (! TARGET_ATPCS)
+     return base_size;
+ 
+   /* We know that SP will be word aligned on entry, and we must
+      preserve that condition at any subroutine call.  But those are
+      the only constraints.  */
+ 
+   /* Space for variadic functions.  */
+   if (current_function_pretend_args_size)
+     entry_size += current_function_pretend_args_size;
+ 
+   /* Space for saved registers.  */
+   entry_size += bit_count (arm_compute_save_reg_mask ()) * 4;
+ 
+   /* Space for saved FPA registers.  */
+   if (! IS_VOLATILE (func_type))
+     {
+       for (regno = FIRST_ARM_FP_REGNUM; regno <= LAST_ARM_FP_REGNUM; regno++)
+       if (regs_ever_live[regno] && ! call_used_regs[regno])
+ 	entry_size += 12;
+     }
+ 
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     base_size += 4;
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     abort ();
+ 
+   return base_size;
+ }
+ 
  /* Generate the prologue instructions for entry into an ARM function.  */
  
  void
***************
*** 8444,8450 ****
  	}
      }
  
!   amount = GEN_INT (-(get_frame_size ()
  		      + current_function_outgoing_args_size));
  
    if (amount != const0_rtx)
--- 8492,8498 ----
  	}
      }
  
!   amount = GEN_INT (-(arm_get_frame_size ()
  		      + current_function_outgoing_args_size));
  
    if (amount != const0_rtx)
***************
*** 10193,10204 ****
    init_machine_status = arm_init_machine_status;
  }
  
  /* Generate the rest of a function's prologue.  */
  
  void
  thumb_expand_prologue ()
  {
!   HOST_WIDE_INT amount = (get_frame_size ()
  			  + current_function_outgoing_args_size);
    unsigned long func_type;
  
--- 10241,10327 ----
    init_machine_status = arm_init_machine_status;
  }
  
+ HOST_WIDE_INT
+ thumb_get_frame_size ()
+ {
+   int regno;
+ 
+   int base_size = ROUND_UP (get_frame_size ());
+   int count_regs = 0;
+   int entry_size = 0;
+ 
+   if (! TARGET_THUMB)
+     abort ();
+ 
+   if (! TARGET_ATPCS)
+     return base_size;
+ 
+   /* We need to know if we are a leaf function.  Unfortunately, it
+      is possible to be called after start_sequence has been called,
+      which causes get_insns to return the insns for the sequence,
+      not the function, which will cause leaf_function_p to return
+      the incorrect result.
+ 
+      To work around this, we cache the computed frame size.  This
+      works because we will only be calling RTL expanders that need
+      to know about leaf functions once reload has completed, and the
+      frame size cannot be changed after that time, so we can safely
+      use the cached value.  */
+ 
+   if (reload_completed)
+     return cfun->machine->frame_size;
+ 
+   /* We know that SP will be word aligned on entry, and we must
+      preserve that condition at any subroutine call.  But those are
+      the only constraints.  */
+ 
+   /* Space for variadic functions.  */
+   if (current_function_pretend_args_size)
+     entry_size += current_function_pretend_args_size;
+ 
+   /* Space for pushed lo registers.  */
+   for (regno = 0; regno <= LAST_LO_REGNUM; regno++)
+     if (THUMB_REG_PUSHED_P (regno))
+       count_regs++;
+ 
+   /* Space for backtrace structure.  */
+   if (TARGET_BACKTRACE)
+     {
+       if (count_regs == 0 && regs_ever_live[LAST_ARG_REGNUM] != 0)
+ 	entry_size += 20;
+       else
+ 	entry_size += 16;
+     }
+ 
+   if (count_regs || !leaf_function_p () || thumb_far_jump_used_p (1))
+     count_regs++;	/* LR */
+ 
+   entry_size += count_regs * 4;
+   count_regs = 0;
+ 
+   /* Space for pushed hi regs.  */
+   for (regno = 8; regno < 13; regno++)
+     if (THUMB_REG_PUSHED_P (regno))
+       count_regs++;
+ 
+   entry_size += count_regs * 4;
+ 
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     base_size += 4;
+   if ((entry_size + base_size + current_function_outgoing_args_size) & 7)
+     abort ();
+ 
+   cfun->machine->frame_size = base_size;
+ 
+   return base_size;
+ }
+ 
  /* Generate the rest of a function's prologue.  */
  
  void
  thumb_expand_prologue ()
  {
!   HOST_WIDE_INT amount = (thumb_get_frame_size ()
  			  + current_function_outgoing_args_size);
    unsigned long func_type;
  
***************
*** 10293,10299 ****
  void
  thumb_expand_epilogue ()
  {
!   HOST_WIDE_INT amount = (get_frame_size ()
  			  + current_function_outgoing_args_size);
    
    /* Naked functions don't have prologues.  */
--- 10416,10422 ----
  void
  thumb_expand_epilogue ()
  {
!   HOST_WIDE_INT amount = (thumb_get_frame_size ()
  			  + current_function_outgoing_args_size);
    
    /* Naked functions don't have prologues.  */
Index: config/arm/arm.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/arm/arm.h,v
retrieving revision 1.166
diff -c -r1.166 arm.h
*** config/arm/arm.h	20 Oct 2002 22:37:09 -0000	1.166
--- config/arm/arm.h	6 Nov 2002 18:16:20 -0000
***************
*** 689,694 ****
--- 689,696 ----
  
  #define STACK_BOUNDARY  32
  
+ #define PREFERRED_STACK_BOUNDARY (TARGET_ATPCS ? 64 : 32)
+ 
  #define FUNCTION_BOUNDARY  32
  
  /* The lowest bit is used to indicate Thumb-mode functions, so the
***************
*** 1436,1441 ****
--- 1438,1445 ----
    int arg_pointer_live;
    /* Records if the save of LR has been eliminated.  */
    int lr_save_eliminated;
+   /* The size of the stack frame.  Only valid after reload.  */
+   int frame_size;
    /* Records the type of the current function.  */
    unsigned long func_type;
    /* Record if the function has a variable argument list.  */
***************
*** 1679,1685 ****
    if ((TO) == STACK_POINTER_REGNUM)					\
      {									\
        (OFFSET) += current_function_outgoing_args_size;			\
!       (OFFSET) += ROUND_UP (get_frame_size ());				\
       }									\
  }
  
--- 1683,1689 ----
    if ((TO) == STACK_POINTER_REGNUM)					\
      {									\
        (OFFSET) += current_function_outgoing_args_size;			\
!       (OFFSET) += thumb_get_frame_size ();				\
       }									\
  }
  

Re: [PATCH/RFA] ARM -- Implement ATPCS stack alignment rules

[Richard Earnshaw]

> On Tue, Nov 05, 2002 at 08:12:34AM -0800, Jason R Thorpe wrote:
> 
>  > OK, I've folded this in.  Lemme try and track down the source of these
>  > regressions, and I'll post another patch...
> 
> Ok, the source of the Thumb regressions has been found (thanks a million,
> Richard :-), and an arm-elf sim testsuite run is in-progress.
> 
> OK for mainline if it passes?
> 
> 2002-11-06  Jason Thorpe  <thorpej@wasabisystems.com> 
> 
>         * config/arm/arm-protos.h (arm_get_frame_size)
>         (thumb_get_frame_size): New prototypes.
>         * config/arm/arm.c (arm_get_frame_size)
>         (thumb_get_frame_size): New functions.
>         (use_return_insn, arm_output_epilogue, arm_output_function_epilogue)
>         (arm_compute_initial_elimination_offset, arm_expand_prologue): Use
>         arm_get_frame_size.
>         (thumb_expand_prologue, thumb_expand_epilogue): Use
>         thumb_get_frame_size.
>         * config/arm/arm.h (PREFERRED_STACK_BOUNDARY): Define.
>         (machine_function): Add frame_size member.
>         (THUMB_INITIAL_ELIMINATION_OFFSET): Use thumb_get_frame_size.
> 
> 2002-11-05  Richard Earnshaw  (rearnsha@arm.com)
> 
>         * arm.c (bit_count): Make argument unsigned long.  Return unsigned.
>         Adjust code to use portable unsigned bit manipulation.
>         (insn_flags, tune_flags): Change type to unsigned.
>         (struct processors): Make flags unsigned long.
>         (arm_override_options): Change type of count and current_bit_count
>         to unsigned.

Normally it would be too late for a patch of this complexity to be placed 
on the trunk, but since this doesn't change the behaviour for existing 
configurations and since we need it before we can add a netbsd/elf 
configuration, then yes, this is ok for the trunk.

R.