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

["Andrew Pinski" <pinskia at gmail dot com>]

Ping?

On 8/28/07, Andrew Pinski <pinskia@gmail.com> wrote:
> Ping?
>
> On 8/4/07, Andrew Pinski <pinskia@gmail.com> wrote:
> > Ping?
> >
> > On 7/2/07, Andrew Pinski <pinskia@gmail.com> wrote:
> > > Ping?
> > >
> > > On 5/18/07, Andrew_Pinski@playstation.sony.com
> > > <Andrew_Pinski@playstation.sony.com> wrote:
> > > > Trevor Smigiel/R&D/SCEA@Playstation wrote on 05/17/2007 04:49:53 PM:
> > > >
> > > > > Andrew,
> > > > >
> > > > > It seems you missed merging a change that I made in our local tree.
> > > > >
> > > > > In cfgexpand.c, you removed this code:
> > > > >
> > > > > -  /* Set alignment we actually gave this decl.  */
> > > > > -  offset -= frame_phase;
> > > > > -  align = offset & -offset;
> > > > > -  align *= BITS_PER_UNIT;
> > > > > -  if (align > STACK_BOUNDARY || align == 0)
> > > > > -    align = STACK_BOUNDARY;
> > > > > -  DECL_ALIGN (decl) = align;
> > > > > -  DECL_USER_ALIGN (decl) = 0;
> > > > >
> > > > > In our local tree that code is conditioned on
> > > > >   if (!DECL_USER_ALIGN (decl))
> > > > > (Which could be done more precisely.  Consider the case where a user
> > > > > specified alignment is smaller than the decl's actual alignment on the
> > > > > stack.)
> > > > >
> > > > > Without it less efficient code can be generated because higher
> > > > > alignments are not propagated.  At least, that's the behaviour on 4.1.1.
> > > >
> > > > Here is the new patch which fixes that problem and we get the get the
> > > > correct alignment on the variables now that we were getting a smaller
> > > > alignment on with my older version of the patch.  It adds a testcase which
> > > > verifies this is the case too (I only check on ia32/x86_64, powerpc* and
> > > > spu as those are the only targets which I know the normal alignment of
> > > > stack is and the only targets I could test the testcase on).
> > > >
> > > > OK? Bootstrapped and tested on i686-linux-gnu with no regressions.
> > > >
> > > > Thanks,
> > > > Andrew Pinski
> > > >
> > > > 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.  Only reset the alignment of
> > > > the decl to given
> > > >         alignment if the alignment is less 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.  Use AND opcode instead of shifting left and shifting
> > > >         back right.
> > > >         (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.
> > > >
> > > >         * gcc.c-torture/execute/pr16660-1.c: New testcase.
> > > >         * gcc.c-torture/execute/pr16660-2.c: New testcase.
> > > >         * gcc.dg/pr16660-1.c: New testcase.
> > > >
> > > >
> > > >
> > > >
> > >
> > >
> >
> >
>
>

Attachment: fixpr16660.diff.txt
Description: Text document