Patchwork PATCH: PR middle-end/45678: [4.4/4.5/4.6 Regression] crash on vector code with -m32 -msse

login
register
mail settings
Submitter Jakub Jelinek
Date Sept. 20, 2010, 5:04 p.m.
Message ID <20100920170418.GO1269@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/65227/
State New
Headers show

Comments

Jakub Jelinek - Sept. 20, 2010, 5:04 p.m.
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.



	Jakub
H.J. Lu - Sept. 20, 2010, 5:12 p.m.
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.

Patch

--- 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;