| Message ID | 20170819201822.GA4866@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Aug 19, 2017 at 10:18 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > Don't override alignment specified by user with the same value to > preserve TYPE_USER_ALIGN. This fixes PR 53037 tests on Sparc. > > Does it look right? Doesn't match do_type_align so it introduces inconsistencies. The documentation for TYPE_USER_ALIGN doesn't specify when both cases conflict: /* 1 if the alignment for this type was requested by "aligned" attribute, 0 if it is the default for this type. */ Note that for example the vectorizer looks at DECL_USER_ALIGN (for non-field-decls) to decide whether it can increase alignment. Richard. > > H.J. > -- > * stor-layout.c (finalize_type_size): Don't override alignment > specified by user with the same value. > --- > gcc/stor-layout.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c > index 3028d55773a..6dd605810ac 100644 > --- a/gcc/stor-layout.c > +++ b/gcc/stor-layout.c > @@ -1784,7 +1784,7 @@ finalize_type_size (tree type) > > /* Don't override a larger alignment requirement coming from a user > alignment of one of the fields. */ > - if (mode_align >= TYPE_ALIGN (type)) > + if (mode_align > TYPE_ALIGN (type)) > { > SET_TYPE_ALIGN (type, mode_align); > TYPE_USER_ALIGN (type) = 0; > -- > 2.13.5 >
On Mon, Aug 21, 2017 at 12:59 AM, Richard Biener <richard.guenther@gmail.com> wrote: > On Sat, Aug 19, 2017 at 10:18 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> Don't override alignment specified by user with the same value to >> preserve TYPE_USER_ALIGN. This fixes PR 53037 tests on Sparc. >> >> Does it look right? > > Doesn't match do_type_align so it introduces inconsistencies. The documentation > for TYPE_USER_ALIGN doesn't specify when both cases conflict: > > /* 1 if the alignment for this type was requested by "aligned" attribute, > 0 if it is the default for this type. */ > > Note that for example the vectorizer looks at DECL_USER_ALIGN (for > non-field-decls) > to decide whether it can increase alignment. > > Richard. > >> >> H.J. >> -- >> * stor-layout.c (finalize_type_size): Don't override alignment >> specified by user with the same value. >> --- >> gcc/stor-layout.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c >> index 3028d55773a..6dd605810ac 100644 >> --- a/gcc/stor-layout.c >> +++ b/gcc/stor-layout.c >> @@ -1784,7 +1784,7 @@ finalize_type_size (tree type) >> >> /* Don't override a larger alignment requirement coming from a user >> alignment of one of the fields. */ >> - if (mode_align >= TYPE_ALIGN (type)) >> + if (mode_align > TYPE_ALIGN (type)) >> { >> SET_TYPE_ALIGN (type, mode_align); >> TYPE_USER_ALIGN (type) = 0; >> -- >> 2.13.5 >> According to Eric: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037#c32 "if (mode_align >= TYPE_ALIGN (type))" was intentional. I am not familiar with STRICT_ALIGNMENT target and there is no testcase to show why it is needed.
> According to Eric: > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53037#c32 > > "if (mode_align >= TYPE_ALIGN (type))" was intentional. I am > not familiar with STRICT_ALIGNMENT target and there is no > testcase to show why it is needed. Yes, the mode promotion triggers the alignment promotion on strict-alignment targets and the specified alignment is thus equal to the default one, so the code works as intended and clears DECL_USER_ALIGN. Not very clear what to do, but the most robust approach would be to use lookup_attribute on the type.
diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c index 3028d55773a..6dd605810ac 100644 --- a/gcc/stor-layout.c +++ b/gcc/stor-layout.c @@ -1784,7 +1784,7 @@ finalize_type_size (tree type) /* Don't override a larger alignment requirement coming from a user alignment of one of the fields. */ - if (mode_align >= TYPE_ALIGN (type)) + if (mode_align > TYPE_ALIGN (type)) { SET_TYPE_ALIGN (type, mode_align); TYPE_USER_ALIGN (type) = 0;