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