| Message ID | 20100920170418.GO1269@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Sep 20, 2010 at 10:04 AM, Jakub Jelinek <jakub@redhat.com> wrote: > On Fri, Sep 17, 2010 at 11:40:28PM +0200, Jakub Jelinek wrote: >> On Fri, Sep 17, 2010 at 02:31:35PM -0700, H.J. Lu wrote: >> > > Using stack offset for local variable alignment is a bad idea. My patch >> > > caused: >> >> It isn't a bad idea, as long as the base of the stack slots is going to be >> as aligned as the code expects. Then it is a nice optimization, which you >> are killing in your patch. All that is IMHO needed is to be a little bit >> more conservative in the estimations. Haven't tested any of the >> possibilities I've listed, but I bet all of them would fix >> > > >> > > FAIL: gcc.target/i386/incoming-9.c scan-assembler-not andl[\\t >> > > ]*\\$-16,[\\t ]*%esp >> >> this. > > Here is a fix for that. On !SUPPORTS_STACK_ALIGNMENT targets > STACK_BOUNDARY == PREFERRED_STACK_BOUNDARY == > crtl->max_used_stack_slot_alignment > so it doesn't make a difference, on the rest of targets usually > get_decl_align_unit will be called on local vars before first > expand_one_stack_var_at will be called, so in most cases it will be as big > as can be safely assumed. Doing > MAX (crtl->max_used_stack_slot_alignment, STACK_BOUNDARY) wouldn't > make sense, as crtl->max_used_stack_slot_alignment is initialized > to STACK_BOUNDARY at the start of expansion and only incremented afterwards > during expansion (but never incremented to more than > MAX_SUPPORTED_STACK_ALIGNMENT). > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk/4.5/4.4? > > 2010-09-20 Jakub Jelinek <jakub@redhat.com> > > PR target/44542 > * cfgexpand.c (expand_one_stack_var_at): Use > crtl->max_used_stack_slot_alignment as max_align, instead > of maximum of that and PREFERRED_STACK_BOUNDARY. > > --- gcc/cfgexpand.c.jj 2010-09-18 19:50:56.000000000 +0200 > +++ gcc/cfgexpand.c 2010-09-20 16:46:52.124526071 +0200 > @@ -738,8 +738,7 @@ expand_one_stack_var_at (tree decl, HOST > offset -= frame_phase; > align = offset & -offset; > align *= BITS_PER_UNIT; > - max_align = MAX (crtl->max_used_stack_slot_alignment, > - PREFERRED_STACK_BOUNDARY); > + max_align = crtl->max_used_stack_slot_alignment; > if (align == 0 || align > max_align) > align = max_align; > You should remove update_stack_alignment (align); a few line down since it will never increase stack alignment.
--- gcc/cfgexpand.c.jj 2010-09-18 19:50:56.000000000 +0200 +++ gcc/cfgexpand.c 2010-09-20 16:46:52.124526071 +0200 @@ -738,8 +738,7 @@ expand_one_stack_var_at (tree decl, HOST offset -= frame_phase; align = offset & -offset; align *= BITS_PER_UNIT; - max_align = MAX (crtl->max_used_stack_slot_alignment, - PREFERRED_STACK_BOUNDARY); + max_align = crtl->max_used_stack_slot_alignment; if (align == 0 || align > max_align) align = max_align;