This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] PR77359: Properly align local variables in functions calling alloca.
- From: Segher Boessenkool <segher at kernel dot crashing dot org>
- To: vogt at linux dot vnet dot ibm dot com, gcc-patches at gcc dot gnu dot org, Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>, Ulrich Weigand <Ulrich dot Weigand at de dot ibm dot com>, David Edelsohn <dje dot gcc at gmail dot com>
- Date: Thu, 10 Nov 2016 18:17:57 -0600
- Subject: Re: [PATCH] PR77359: Properly align local variables in functions calling alloca.
- Authentication-results: sourceware.org; auth=none
- References: <20161103104044.GA8122@linux.vnet.ibm.com> <20161110234702.GA21356@linux.vnet.ibm.com>
On Fri, Nov 11, 2016 at 12:47:02AM +0100, Dominik Vogt wrote:
> 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.)
You say it needs more testing -- what testing?
(And it needs to be posted to gcc-patches@ of course).
> > +#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)))
Maybe you can make the comment explain these last two lines as well... It
seems to me you want to align STARTING_FRAME_OFFSET if calls_alloca?
Also add a comment for the one in rs6000.h?
> > +/* 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)
You don't need parens around STACK_POINTER_OFFSET.
Looks fine to me except for those nits,
Segher