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
