Re: [PATCH] PR77359: Properly align local variables in functions calling alloca.

[Dominik Vogt <vogt at linux dot vnet dot ibm dot com>]

On Thu, Nov 03, 2016 at 11:40:44AM +0100, Dominik Vogt wrote:
> The attached patch fixes the stack layout problems on AIX and
> Power as described here:
> 
>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77359
> 
> The patch has been bootstrapped on AIX (32 Bit) and bootstrappend
> and regression tested on Power (biarch).  It needs more testing
> that I cannot do with the hardware available to me.
> 
> If the patch is good, this one can be re-applied:
> 
>   https://gcc.gnu.org/ml/gcc-patches/2016-07/msg01730.html
>   https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01616.html

So, is this patch in order to be committed?  (Assuming that a
followup patch will clean up the rs6000.h+aix.h quirks.)

> gcc/ChangeLog
> 
> 	* config/rs6000/rs6000.c (rs6000_stack_info): Properly align local
> 	variables in functions calling alloca.
> 	* config/rs6000/rs6000.h (STARTING_FRAME_OFFSET, STACK_DYNAMIC_OFFSET):
> 	Likewise.
> 	* config/rs6000/aix.h (STARTING_FRAME_OFFSET, STACK_DYNAMIC_OFFSET):
> 	Copy AIX specific versions of the rs6000.h macros to aix.h.

> >From bd36042fd82e29204d2f10c180b9e7c27281eef2 Mon Sep 17 00:00:00 2001
> From: Dominik Vogt <vogt@linux.vnet.ibm.com>
> Date: Fri, 28 Oct 2016 12:59:55 +0100
> Subject: [PATCH] PR77359: Properly align local variables in functions
>  calling alloca.
> 
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77359 for a discussion of the
> problem and the fix.
> ---
>  gcc/config/rs6000/aix.h    | 27 +++++++++++++++++++++++++++
>  gcc/config/rs6000/rs6000.c |  9 +++++++--
>  gcc/config/rs6000/rs6000.h | 14 ++++++++------
>  3 files changed, 42 insertions(+), 8 deletions(-)
> 
> diff --git a/gcc/config/rs6000/aix.h b/gcc/config/rs6000/aix.h
> index b254236..7773517 100644
> --- a/gcc/config/rs6000/aix.h
> +++ b/gcc/config/rs6000/aix.h
> @@ -40,6 +40,33 @@
>  #undef  STACK_BOUNDARY
>  #define STACK_BOUNDARY 128
>  
> +/* Offset within stack frame to start allocating local variables at.
> +   If FRAME_GROWS_DOWNWARD, this is the offset to the END of the
> +   first local allocated.  Otherwise, it is the offset to the BEGINNING
> +   of the first local allocated.
> +
> +   On the RS/6000, the frame pointer is the same as the stack pointer,
> +   except for dynamic allocations.  So we start after the fixed area and
> +   outgoing parameter area.  */
> +
> +#undef STARTING_FRAME_OFFSET
> +#define STARTING_FRAME_OFFSET						\
> +  (FRAME_GROWS_DOWNWARD							\
> +   ? 0									\
> +   : (cfun->calls_alloca						\
> +      ? RS6000_ALIGN (crtl->outgoing_args_size + RS6000_SAVE_AREA, 16)	\
> +      : (RS6000_ALIGN (crtl->outgoing_args_size, 16) + RS6000_SAVE_AREA)))
> +
> +/* Offset from the stack pointer register to an item dynamically
> +   allocated on the stack, e.g., by `alloca'.
> +
> +   The default value for this macro is `STACK_POINTER_OFFSET' plus the
> +   length of the outgoing arguments.  The default is correct for most
> +   machines.  See `function.c' for details.  */
> +#undef STACK_DYNAMIC_OFFSET
> +#define STACK_DYNAMIC_OFFSET(FUNDECL)					\
> +   RS6000_ALIGN (crtl->outgoing_args_size + (STACK_POINTER_OFFSET), 16)
> +
>  #undef  TARGET_IEEEQUAD
>  #define TARGET_IEEEQUAD 0
>  
> diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
> index f9e4739..02ed9c1 100644
> --- a/gcc/config/rs6000/rs6000.c
> +++ b/gcc/config/rs6000/rs6000.c
> @@ -26004,8 +26004,13 @@ rs6000_stack_info (void)
>    info->reg_size     = reg_size;
>    info->fixed_size   = RS6000_SAVE_AREA;
>    info->vars_size    = RS6000_ALIGN (get_frame_size (), 8);
> -  info->parm_size    = RS6000_ALIGN (crtl->outgoing_args_size,
> -					 TARGET_ALTIVEC ? 16 : 8);
> +  if (cfun->calls_alloca)
> +    info->parm_size  =
> +      RS6000_ALIGN (crtl->outgoing_args_size + info->fixed_size,
> +		    STACK_BOUNDARY / BITS_PER_UNIT) - info->fixed_size;
> +  else
> +    info->parm_size  = RS6000_ALIGN (crtl->outgoing_args_size,
> +				     TARGET_ALTIVEC ? 16 : 8);
>    if (FRAME_GROWS_DOWNWARD)
>      info->vars_size
>        += RS6000_ALIGN (info->fixed_size + info->vars_size + info->parm_size,
> diff --git a/gcc/config/rs6000/rs6000.h b/gcc/config/rs6000/rs6000.h
> index 4b83abd..c11dc1b 100644
> --- a/gcc/config/rs6000/rs6000.h
> +++ b/gcc/config/rs6000/rs6000.h
> @@ -1728,9 +1728,12 @@ extern enum reg_class rs6000_constraints[RS6000_CONSTRAINT_MAX];
>  #define STARTING_FRAME_OFFSET						\
>    (FRAME_GROWS_DOWNWARD							\
>     ? 0									\
> -   : (RS6000_ALIGN (crtl->outgoing_args_size,				\
> -		    (TARGET_ALTIVEC || TARGET_VSX) ? 16 : 8)		\
> -      + RS6000_SAVE_AREA))
> +   : (cfun->calls_alloca						\
> +      ? (RS6000_ALIGN (crtl->outgoing_args_size + RS6000_SAVE_AREA,	\
> +		       (TARGET_ALTIVEC || TARGET_VSX) ? 16 : 8 ))	\
> +      : (RS6000_ALIGN (crtl->outgoing_args_size,			\
> +		       (TARGET_ALTIVEC || TARGET_VSX) ? 16 : 8)		\
> +	 + RS6000_SAVE_AREA)))
>  
>  /* Offset from the stack pointer register to an item dynamically
>     allocated on the stack, e.g., by `alloca'.
> @@ -1739,9 +1742,8 @@ extern enum reg_class rs6000_constraints[RS6000_CONSTRAINT_MAX];
>     length of the outgoing arguments.  The default is correct for most
>     machines.  See `function.c' for details.  */
>  #define STACK_DYNAMIC_OFFSET(FUNDECL)					\
> -  (RS6000_ALIGN (crtl->outgoing_args_size,				\
> -		 (TARGET_ALTIVEC || TARGET_VSX) ? 16 : 8)		\
> -   + (STACK_POINTER_OFFSET))
> +  RS6000_ALIGN (crtl->outgoing_args_size + (STACK_POINTER_OFFSET),	\
> +		(TARGET_ALTIVEC || TARGET_VSX) ? 16 : 8)
>  
>  /* If we generate an insn to push BYTES bytes,
>     this says how many the stack pointer really advances by.
> -- 
> 2.3.0
> 



Ciao

Dominik ^_^  ^_^

-- 

Dominik Vogt
IBM Germany