------- Comment #21 from matz at suse dot de 2009-05-06 21:39 ------- Subject: Re: [4.5 Regression] Revision 146817 caused unaligned access in gcc.dg/torture/pr26565.c
On Wed, 6 May 2009, rguenther at suse dot de wrote: > > + tree ret; > > + if (TYPE_PACKED (t)) > > + return t; > > Walking the type variants and searching for what we now build would > fix the inefficiency. And of course this function needs a comment ;) I anyway am unsure about using variants of types for this. E.g. some other lookup functions when searching for a qualified type simply walk the list and take the first one with matching qualifiers. Now if there suddenly are multiple variants with same qualifiers but different alignment in there it might chose an overly aligned type accidentally. Or maybe I'm confused, hmm... (but yes, that's the obvious fix for the inefficiency). Can qualified types themself be a new base (TYPE_MAIN_VARIANT) for a new chain? In that case it would work just fine. > > + ret = build_variant_type_copy (t); > > + TYPE_ALIGN (ret) = align * BITS_PER_UNIT; > > + TYPE_USER_ALIGN (ret) = 1; > > It seems that only ever place_field looks at this flag. TYPE_USER_ALIGN? By place_field and update_alignment_for_field, and it's copied into DECL_USER_ALIGN (which is used in more places). TYPE_USER_ALIGN only ever seems to guard calls to ADJUST_FIELD_ALIGN when PCC_BITFIELD_TYPE_MATTERS or BITFIELD_NBYTES_LIMITED. But I have no idea why a user specified alignment should not also be affected by ADJUST_FIELD_ALIGN. It all seems to have to do with the general theme of not overriding user-specified alignment in any way that the compiler normally takes to derive alignments. In any case it seems better to leave this alone, stor-layout.c is filled with sometimes quite arcane conditions and special cases and probably nobody in the world can test all combinations of strange ABIs and funny requirements or backward compatibilities. > How is the effect of setting it here? Well, the user explicitely put "attribute((packed))" there so it seems reasonable to deal with this as if he also specified an explicit alignment. > Overall I like this patch. Much to my surprise it even seems to work up to now, bootstrap was okay testsuite still running. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39954
