Re: [PATCH] ia64 -fstack-protector support

[Jakub Jelinek <jakub at redhat dot com>]

On Mon, Jul 04, 2005 at 02:53:37PM -0700, Richard Henderson wrote:
> On Mon, Jul 04, 2005 at 02:52:28PM -0400, Jakub Jelinek wrote:
> > -#define FRAME_GROWS_DOWNWARD 0
> > +#define FRAME_GROWS_DOWNWARD (flag_stack_protect != 0)
> 
> As much as possible, this should not change based on stack-protect.
> We want as much testing as possible with frame layout.

At least for leaf functions, I don't think we can beat the
FRAME_GROWS_DOWNWARD == 0 setup.  There sfp is eliminated to 0,
so if say just one int or long var has its address taken,
then with FRAME_GROWS_DOWNWARD we can just store it to [r12] and
don't do any additions/subtractions.  As STACK_BOUNDARY is 16 on ia64,
we can't achieve that with FRAME_GROWS_DOWNWARD != 0.

> I know what you're trying to do is avoid extra padding here:
> 
> > +  if (FRAME_GROWS_DOWNWARD)
> > +    size = IA64_STACK_ALIGN (size);

This is actually maybe unnecessarily, servers just a double check
and documentation that it really must be aligned.  I have found
that all the sizes at least on the testcases I have tried were
already multiples of 16, because the alignment e.g. reload enforces.

> but that simply means you chose the wrong point at which to base
> the soft frame pointer.  Seems like the most appropriate place is
> the incoming stack pointer + 16, aka the CFA.  This does mean that
> the initial stack offset is variable (based on spill_size etc), but
> that's ok.

But will look if ia64 ABI mandates anything about the spill location
and if not, will change the patch to move the spills down below the vars
area.

> > +  ia64_compute_frame_size (size = get_frame_size ());
> 
> Please avoid embedding assignments in other expressions when possible.

Ok.

	Jakub