[Bug middle-end/39954] [4.5 Regression] Revision 146817 caused unaligned access in gcc.dg/torture/pr26565.c

["matz at suse dot de" <gcc-bugzilla at gcc dot gnu dot org>]

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