| Message ID | 20120209113833.GR18768@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Feb 9, 2012 at 12:38 PM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > DECL_USER_ALIGN bit used to be formerly in tree_decl_common structure > directly, thus the memcpy merge_decls performs used to copy also the > DECL_USER_ALIGN (newdecl) bit to DECL_USER_ALIGN (olddecl). But > it has been moved into tree_base, which is not copied that way. > This means that in C if olddecl has normal alignment and newdecl has bigger > alignment and DECL_USER_ALIGN, it will have the bigger DECL_ALIGN (which > is copied with memcpy), but won't have DECL_USER_ALIGN set, so when we call > relayout_decl on it this DECL_ALIGN is reset back to the smaller value. > Fixed by copying that bit. > > After writing a larger testcase for this I've noticed that the C++ FE > has a bug in this too, but the other way around, i.e. if olddecl had > user alignment and newdecl does not, newdecl will not inherit the user > alignment. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? I'd say this needs to be backported to at least the point where we moved the align fields. Richard. > 2012-02-09 Jakub Jelinek <jakub@redhat.com> > > PR c/52181 > * c-decl.c (merge_decls): Copy DECL_USER_ALIGN bit from olddecl to > newdecl. > > * decl.c (duplicate_decls): If olddecl has bigger DECL_ALIGN than > newdecl, copy DECL_ALIGN to newdecl and or DECL_USER_ALIGN bits. > > * c-c++-common/pr52181.c: New test. > > --- gcc/c-decl.c.jj 2012-01-15 20:59:56.000000000 +0100 > +++ gcc/c-decl.c 2012-02-09 09:33:35.955067726 +0100 > @@ -2449,6 +2449,7 @@ merge_decls (tree newdecl, tree olddecl, > memcpy ((char *) olddecl + sizeof (struct tree_common), > (char *) newdecl + sizeof (struct tree_common), > sizeof (struct tree_decl_common) - sizeof (struct tree_common)); > + DECL_USER_ALIGN (olddecl) = DECL_USER_ALIGN (newdecl); > switch (TREE_CODE (olddecl)) > { > case FUNCTION_DECL: > --- gcc/cp/decl.c.jj 2012-01-26 09:22:19.000000000 +0100 > +++ gcc/cp/decl.c 2012-02-09 09:56:42.129108618 +0100 > @@ -2214,7 +2214,12 @@ duplicate_decls (tree newdecl, tree oldd > SET_DECL_INIT_PRIORITY (olddecl, DECL_INIT_PRIORITY (newdecl)); > DECL_HAS_INIT_PRIORITY_P (olddecl) = 1; > } > - /* Likewise for DECL_USER_ALIGN and DECL_PACKED. */ > + /* Likewise for DECL_ALIGN, DECL_USER_ALIGN and DECL_PACKED. */ > + if (DECL_ALIGN (olddecl) > DECL_ALIGN (newdecl)) > + { > + DECL_ALIGN (newdecl) = DECL_ALIGN (olddecl); > + DECL_USER_ALIGN (newdecl) |= DECL_USER_ALIGN (olddecl); > + } > DECL_USER_ALIGN (olddecl) = DECL_USER_ALIGN (newdecl); > if (TREE_CODE (newdecl) == FIELD_DECL) > DECL_PACKED (olddecl) = DECL_PACKED (newdecl); > --- gcc/testsuite/c-c++-common/pr52181.c.jj 2012-02-09 09:36:36.332028377 +0100 > +++ gcc/testsuite/c-c++-common/pr52181.c 2012-02-09 09:36:04.000000000 +0100 > @@ -0,0 +1,13 @@ > +/* PR c/52181 */ > +/* { dg-do compile } */ > + > +extern const int v1[]; > +const int __attribute__((aligned(16))) v1[] = { 1 }; > +extern const int __attribute__((aligned(16))) v2[]; > +const int v2[] = { 1 }; > +extern const int __attribute__((aligned(16))) v3[]; > +const int __attribute__((aligned(16))) v3[] = { 1 }; > +const int __attribute__((aligned(16))) v4[] = { 1 }; > +int test[(__alignof__ (v4) != __alignof__ (v1) /* { dg-bogus "is negative" } */ > + || __alignof__ (v4) != __alignof__ (v2) > + || __alignof__ (v4) != __alignof__ (v3)) ? -1 : 0]; > > Jakub
On Thu, Feb 09, 2012 at 12:54:59PM +0100, Richard Guenther wrote: > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > I'd say this needs to be backported to at least the point where we moved > the align fields. Agreed, but I'd wait with the backports a little bit. Jakub
On Thu, 9 Feb 2012, Jakub Jelinek wrote: > 2012-02-09 Jakub Jelinek <jakub@redhat.com> > > PR c/52181 > * c-decl.c (merge_decls): Copy DECL_USER_ALIGN bit from olddecl to > newdecl. > > * decl.c (duplicate_decls): If olddecl has bigger DECL_ALIGN than > newdecl, copy DECL_ALIGN to newdecl and or DECL_USER_ALIGN bits. > > * c-c++-common/pr52181.c: New test. The C change and testcase are OK.
OK. Jason
--- gcc/c-decl.c.jj 2012-01-15 20:59:56.000000000 +0100 +++ gcc/c-decl.c 2012-02-09 09:33:35.955067726 +0100 @@ -2449,6 +2449,7 @@ merge_decls (tree newdecl, tree olddecl, memcpy ((char *) olddecl + sizeof (struct tree_common), (char *) newdecl + sizeof (struct tree_common), sizeof (struct tree_decl_common) - sizeof (struct tree_common)); + DECL_USER_ALIGN (olddecl) = DECL_USER_ALIGN (newdecl); switch (TREE_CODE (olddecl)) { case FUNCTION_DECL: --- gcc/cp/decl.c.jj 2012-01-26 09:22:19.000000000 +0100 +++ gcc/cp/decl.c 2012-02-09 09:56:42.129108618 +0100 @@ -2214,7 +2214,12 @@ duplicate_decls (tree newdecl, tree oldd SET_DECL_INIT_PRIORITY (olddecl, DECL_INIT_PRIORITY (newdecl)); DECL_HAS_INIT_PRIORITY_P (olddecl) = 1; } - /* Likewise for DECL_USER_ALIGN and DECL_PACKED. */ + /* Likewise for DECL_ALIGN, DECL_USER_ALIGN and DECL_PACKED. */ + if (DECL_ALIGN (olddecl) > DECL_ALIGN (newdecl)) + { + DECL_ALIGN (newdecl) = DECL_ALIGN (olddecl); + DECL_USER_ALIGN (newdecl) |= DECL_USER_ALIGN (olddecl); + } DECL_USER_ALIGN (olddecl) = DECL_USER_ALIGN (newdecl); if (TREE_CODE (newdecl) == FIELD_DECL) DECL_PACKED (olddecl) = DECL_PACKED (newdecl); --- gcc/testsuite/c-c++-common/pr52181.c.jj 2012-02-09 09:36:36.332028377 +0100 +++ gcc/testsuite/c-c++-common/pr52181.c 2012-02-09 09:36:04.000000000 +0100 @@ -0,0 +1,13 @@ +/* PR c/52181 */ +/* { dg-do compile } */ + +extern const int v1[]; +const int __attribute__((aligned(16))) v1[] = { 1 }; +extern const int __attribute__((aligned(16))) v2[]; +const int v2[] = { 1 }; +extern const int __attribute__((aligned(16))) v3[]; +const int __attribute__((aligned(16))) v3[] = { 1 }; +const int __attribute__((aligned(16))) v4[] = { 1 }; +int test[(__alignof__ (v4) != __alignof__ (v1) /* { dg-bogus "is negative" } */ + || __alignof__ (v4) != __alignof__ (v2) + || __alignof__ (v4) != __alignof__ (v3)) ? -1 : 0];