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

["Richard Guenther" <richard dot guenther at gmail dot com>]

On Sun, May 25, 2008 at 5:02 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> Hi Richard,
>
> Thanks for reviewing the patch.
>
> On Sun, May 25, 2008 at 04:37:12PM +0200, Richard Guenther wrote:
>> On Mon, May 19, 2008 at 11:33 AM, Ye, Joey <joey.ye@intel.com> wrote:
>> > Jan,
>> >
>> > This update addresses your comments about caller alignment in
>> > http://gcc.gnu.org/ml/gcc-patches/2008-04/msg02194.html
>> >
>> > OK for mainline?
>>
>> +#ifndef MAX_VECTORIZE_STACK_ALIGNMENT
>> +#define MAX_VECTORIZE_STACK_ALIGNMENT 0
>> +#endif
>> +
>> +#ifndef LOCAL_ALIGNMENT
>> +#define LOCAL_ALIGNMENT(TYPE, MODE, ALIGNMENT) ALIGNMENT
>> +#endif
>> +
>
> LOCAL_ALIGNMENT update comes from the fix for
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=36253
>
> We noticed the caller save stack alignment bug while working on
> stack alignment. I submitted a patch:
>
> http://gcc.gnu.org/ml/gcc-patches/2008-05/msg01029.html
>
> If you can review it, the stack alignment patch can be simplified.
>
>>
>> These both need documenting.  I'm lost in the patch otherwise -- can you point
>> me to a mail with an overview of the whole picture, especially what this
>> MAX_VECTORIZE_STACK_ALIGNMENT is all about?
>>
>
> We don't want to penalize targets which don't implement stack
> alignment.  We need a compile time constant to check if we need
> to do any stack alignment work in middle-end.  On the other hand
> vectorizer needs to know if a backend can align the stack:
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32893
>
> So on stack branch, vect_can_force_dr_alignment_p has
>
>  if (TREE_STATIC (decl))
>    return (alignment <= MAX_OFILE_ALIGNMENT);
>  else if (MAX_VECTORIZE_STACK_ALIGNMENT)
>    return (alignment <= MAX_VECTORIZE_STACK_ALIGNMENT);
>  else
>    return (alignment <= STACK_BOUNDARY);
>
> and we define MAX_VECTORIZE_STACK_ALIGNMENT for x86 as
>
> #define MAX_VECTORIZE_STACK_ALIGNMENT BIGGEST_ALIGNMENT
>
> On x86, we can align stack greater than BIGGEST_ALIGNMENT.  Maybe we
> should define and document a different macro instead, like
>
> #define MAX_STACK_ALIGNMENT MAX_OFILE_ALIGNMENT
>
> and use
>
>  if (TREE_STATIC (decl))
>    return (alignment <= MAX_OFILE_ALIGNMENT);
>  else if (MAX_STACK_REALIGNMENT)
>    return (alignment <= MAX_STACK_ALIGNMENT);
>  else
>    return (alignment <= STACK_BOUNDARY);

Yes, that would make it less confusing IMHO.

Thanks,
Richard.