[Bug tree-optimization/52395] [4.7 Regression] Too conservative alignment info from SRA

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

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52395

--- Comment #12 from rguenther at suse dot de <rguenther at suse dot de> 2012-02-28 09:02:04 UTC ---
On Mon, 27 Feb 2012, jamborm at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52395
> 
> --- Comment #10 from Martin Jambor <jamborm at gcc dot gnu.org> 2012-02-27 16:25:15 UTC ---
> (In reply to comment #4)
> > Martin, can we make sure that 'base' as passed to build_ref_for_offset
> > is not artificially constructed by any of its callers?  Thus, that it is
> > at most the result of stripping some handled-component-refs from an
> > existing reference tree (that was not wrapped inside an ADDR_EXPR) in the IL?
> 
> As you noticed in PR 52402 IPA-SRA code does not use this function and
> IPA-CP only does so when the address argument is a global declaration,
> which should be always handled well by expand, I hope.
> 
> However, I believe it's the plain ordinary SRA which can introduce,
> accesses which the base did not have before.  Consider function foo in
> the following testcase:
> 
> --------------------------------------------------
> typedef long long __m128i __attribute__ ((__vector_size__ (16),
>                       __may_alias__));
> typedef unsigned int uint32_t;
> extern void abort ();
> 
> typedef struct
> {
>   __m128i b;
>   int a;
> } s_1a;
> 
> typedef s_1a s_1m __attribute__((aligned(1)));
> 
> typedef struct
> {
>   char c;
>   s_1m s;
> } __attribute__((packed)) s_2;
> 
> void __attribute__((noinline, noclone))
> foo (s_1m *p)
> {
>   __m128i v1 = {1,1};
>   s_1a l;
> 
>   l = *p;
>   l.b = v1;
>   *p = l;
> }
> 
> int
> main (int argc, char **argv)
> {
>   __m128i v2 = {3,4};
>   long long z[2];
>   s_2 x;
> 
> 
>   x.s.b = v2;
>   foo (&x.s);
>   __builtin_memcpy (&z, &x.s.b, sizeof (x.s.b));
> 
>   if (z[0] != 1 || z[1] != 1)
>     abort ();
> 
>   return 0;
> }
> --------------------------------------------------
> 
> The original foo:
> 
>   l = *p_2(D);
>   l.b = { 1, 1 };
>   *p_2(D) = l;
> 
> is basically converted into (optimized dump):
> 
>   p_2(D)->b = { 1, 1 };
> 
> so we have introduced a non-BLKmode access into a base which did not
> have it previously.  I am not sure whether this fits your definition
> of base which was already there or not.  On the other hand, this
> testcase passes with the patch from comment 6 (but fails with PR 50444
> fix reverted) and SRA never creates anything more beastly than this,
> i.e. forwarding scalar loads/stores across aggregate assignments.
> 
> Does that answer your question?

Yes, I think so.

Richard.