RE: [PATCH] Fix PR 16660, attribute((aligned)) doesn't work for variables on the stack for greater than required alignment

["Meissner, Michael" <michael dot meissner at amd dot com>]

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org
[mailto:gcc-patches-owner@gcc.gnu.org]
> On Behalf Of Andrew_Pinski@PlayStation.Sony.com
> Sent: Monday, January 29, 2007 9:31 PM
> To: gcc-patches@gcc.gnu.org
> Cc: Trevor_Smigiel@PlayStation.Sony.com;
> Russell_Olsen@PlayStation.Sony.com
> Subject: [PATCH] Fix PR 16660, attribute((aligned)) doesn't work for
> variables on the stack for greater than required alignment
> 
> Hi,
>   This patch fixes PR 16660 where we ignore the user supplied
allignment
> on stack variables if it is greater than the required alignment on the
> specific target.  This fixes the issue by oversizing the stack
variable
> and then manually aligning the variable.
> 
> I had to remove some asserts from the x86 back-end as they are no
longer
> valid from the point of view of the middle-end could have a case where
> preferred_alignment is greater than the target's preferred alignment
and
> the stack alignment that is needed can be greater than the target's
> preferred alignment.
> 
> 
> OK? Bootstrapped and tested on i686-linux-gnu and powerpc-linux-gnu
with
> no regressions.
> 
> Thanks,
> Andrew Pinski
> 
> :ADDPATCH middle-end:
> 
> ChangeLog:
> 
>         * expr.h (allocate_dynamic_stack_space_1): New function
prototype.
>         * functionc.c (assign_temp): Take into account the alignment
>         of the temp if it is greater than the target's preferred
> alignment.
>         * cfgexpand.c: Include optabs.h.
>         (get_decl_align_unit): Update comment and don't lower the
> alignment
>         if it is greater than the target's preferred alignment.
>         (alloc_stack_frame_space): Take an unsigned HOST_WIDE_INT for
> align.
>         Take into account the variable's alignment if it is greater
than
>         the target's preferred alignment.
>         (expand_one_stack_var_at): Likewise.
>         (defer_stack_allocation): Defer variables who's alignment is
> greater
>         than the target's preferred alignment.
>         * explow.c (allocate_dynamic_stack_space_1): Split out from
>         allocate_dynamic_stack_space and take into acount the required
>         alignment.
>         (allocate_dynamic_stack_space): Call
> allocate_dynamic_stack_space_1.
>         * Makefile.in (cfgexpand.o): Update dependecies.
>         * stmt.c (expand_decl):  Take into account the alignment
>         of the variable if it is greater than the target's preferred
>         alignment.
>         * config/i386/i386.c (ix86_compute_frame_layout): Don't assert
>         that preferred alignment is greater than the normal preferred
>         alignment.  Don't assert that the stack alignment needed is
> greater
>         than the normal preferred alignment.
> 

I'm starting to look at patches after a long absence.  I looked at this
patch, and it looks ok.  I would suggest changing the comment in
function.c where you talk about comparing against BIGGEST_ALIGNMENT but
the code actually tests against PREFERRED_STACK_BOUNDARY.

--
Michael Meissner
AMD, MS 83-29
90 Central Street
Boxborough, MA 01719