Re: [PATCH 2/7]: Ping3: Merge from Stack Branch - collect alignment info

[Richard Sandiford <rdsandiford at googlemail dot com>]

"H.J. Lu" <hjl.tools@gmail.com> writes:
> On Wed, May 28, 2008 at 11:10 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
>
>>
>>>
>>> This is probably going over old ground, sorry, but would it make sense
>>> to have the default MAX_STACK_ALIGNMENT be PREFERRED_STACK_BOUNDARY
>>> (or whatever the correct value is when dynamic realignment isn't
>>> supported)?  Dynamic reallocation would then be supported iff
>>> MAX_STACK_ALIGNMENT > PREFERRED_STACK_BOUNDARY.
>>>
>>
>> I will give it a try.
>>
>> Thanks.
>>
>
> Hi Richard,
>
> I am checking this patch into stack branch.

Thanks.  This certainly looks better to me FWIW.

I'm still worried about the way assign_stack_local_1 ignores the
actual value of MAX_STACK_ALIGNMENT though.  I realise it's a
very big value for x86, but shouldn't it still be enforced?
To be clear, I'm talking about:

  if (MAX_STACK_ALIGNMENT > STACK_BOUNDARY)
    {
      if (crtl->stack_alignment_estimated < alignment_in_bits)
	{
          if (!crtl->stack_realign_processed)
            crtl->stack_alignment_estimated = alignment_in_bits;

which keeps the original value of alignment_in_bits even if it
exceeds MAX_STACK_ALIGNMENT.

Also, the "else" branch of this "if" statement still uses
PREFERRED_STACK_BOUNDARY instead of STACK_BONDARY.  I can never
get my head around the STACK_BOUNDARY/PREFERRED_STACK_BOUNDARY thing,
so I won't try to comment on whether

  #define MAX_STACK_ALIGNMENT STACK_BOUNDARY

is right.  But using STACK_BOUNDARY in the "if" condition and
PREFERRED_STACK_BOUNDARY in the "else" block does seem odd on
the face of it.  Perhaps a comment is needed?

Richard