[PATCH] DATA_ALIGNMENT vs. DATA_ABI_ALIGNMENT (PR target/56564)

Alan Modra amodra@gmail.com
Fri Jun 14 10:42:00 GMT 2013


On Fri, Jun 14, 2013 at 10:59:52AM +0200, Jakub Jelinek wrote:
> On Fri, Jun 14, 2013 at 08:18:19AM +0930, Alan Modra wrote:
> > It is handling !DECL_P trees, which must be local.  I know I saw
> > STRING_CST here when I wrote offsettable_ok_by_alignment, hence the
> > use of CONSTANT_ALIGNMENT.  I'm not so sure about the need for
> > DATA_ALIGNMENT now, but if it was correct before then we ought to
> > be using both DATA_ABI_ALIGNMENT and DATA_ALIGNMENT after your
> > changes.
> 
> Yeah, then it makes sense.  Sorry for not looking up earlier that this is
> the !DECL_P case.

Your comment prodded me into looking at whether the !DECL_P code is
needed in 4.9, and it looks like we never see !DECL_P trees..
Bootstrap and regression tests all langs on powerpc64 didn't hit a
gcc_unreachable() I put there, both with -fsection-anchors and
-fno-section-anchors.  David, please consider that piece of the patch
retracted.

> As for the 
> typedef int vec_align __attribute__ ((vector_size(16), aligned(32)));                                                                            
> vec_align x = { 0, 0, 0, 0 };                                                                                                                    
> changes, that is ABI changing bugfix, so the question is, are you fine with
> breaking the ABI (between 4.8 and 4.9, or if you wanted to backport it to
> 4.8 too (I certainly plan to backport the non-ppc DATA_ABI_ALIGNMENT changes
> to 4.8.2, already am using it in our compilers))?  The other option is
> to fix the ABI, but keep things backwards ABI compatible.  That would be
> done by decreasing the alignment as it used to do before in DATA_ABI_ALIGNMENT,
> and increasing it to the desirable level only in DATA_ALIGNMENT.  That has
> the effect that when emitting the decls into assembly e.g. the above will
> now be correctly 32 byte aligned, but accesses to such decl in compiler
> generated code will only assume that alignment if
> decl_binds_to_current_def_p, otherwise they will keep assuming the old
> (broken) lowered alignment.  At least for 4.8 backport IMHO that would be a
> better idea (but of course would need big comment explaning it).

I see your point, but for there to be a real problem we'd need
a) A library exporting such a type with (supposed) increased
   alignment, and,
b) gcc would need to make use of the increased alignment.

(a) must be rare or non-existent or you'd think we would have had a
bug report about lack of user alignment in vector typedefs.  The code
has been like this since 2001-11-07, so users have had a long time to
discover it.  (Of course, this is an argument for just ignoring the
bug too.)

(b) doesn't happen in the rs6000 backend as far as I'm aware.  Do you
know whether there is some optimisation based on alignment in generic
parts of gcc?  A quick test like

typedef int vec_align __attribute__ ((vector_size(16), aligned(32)));
vec_align x = { 0, 0, 0, 0 };

long f1 (void)
{
  return (long) &x & -32;
}

static int y __attribute__ ((aligned(32)));

long f2 (void)
{
  return (long) &y & -32;
}

shows the "& -32" in both functions isn't optimised away.

-- 
Alan Modra
Australia Development Lab, IBM



More information about the Gcc-patches mailing list