[PATCH, PR 58041] Make gimple-ssa-strenght-reduction.c create MEM_REFs with alignment information

[Richard Biener]

Martin Jambor <mjambor@suse.cz> wrote:
>Hi,
>
>PR 58041 is a misalignment bug caused by replace_ref in
>gimple-ssa-strength-reduction.c because it does not make sure that the
>MEM_REFs it creates has the proper alignment encoded in them.
>
>I'd like to fix this with the patch below, which basically does the
>same thing SRA does, it is only slightly simpler because we are
>replacing an access to memory with an access to the exact same memory,
>so we don't have to bother computing offset alignments.
>
>I was wondering whether it would make sense to put some common part of
>the code to a function and call it from replace_ref and SRA but such a
>function would probably only contain the align < TYPE_ALIGN check and
>the fold_build_2 call so I was not sure it was worth it.  However, I
>may add it as a followup, it may make future users more aware of the
>need to take care of alignment.
>
>I have bootstrapped and tested the patch below on on x86_64-linux,
>Bill Schmidt did the same on powerpc64-unknown-linux-gnu and it also
>got some testing on ARM.  OK for trunk (and I think also the 4.8
>branch needs it)?

Ok.

Thanks,
Richard.

>Thanks,
>
>Martin
>
>
>2013-08-05  Martin Jambor  <mjambor@suse.cz>
>
>	PR middle-end/58041
>	* gimple-ssa-strength-reduction.c (replace_ref): Make sure built
>	MEM_REF has proper alignment information.
>
>testsuite/
>	* gcc.dg/torture/pr58041.c: New test.
>	* gcc.target/arm/pr58041.c: Likewise.
>
>*** /tmp/5wmq7D_gimple-ssa-strength-reduction.c	Mon Aug  5 19:05:06
>2013
>--- gcc/gimple-ssa-strength-reduction.c	Mon Aug  5 18:56:10 2013
>*************** dump_incr_vec (void)
>*** 1728,1738 ****
>  static void
>  replace_ref (tree *expr, slsr_cand_t c)
>  {
>!   tree add_expr = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE
>(c->base_expr),
>! 			       c->base_expr, c->stride);
>!   tree mem_ref = fold_build2 (MEM_REF, TREE_TYPE (*expr), add_expr,
>! 			      double_int_to_tree (c->cand_type, c->index));
>!   
>/* Gimplify the base addressing expression for the new MEM_REF tree. 
>*/
>    gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
>    TREE_OPERAND (mem_ref, 0)
>--- 1728,1750 ----
>  static void
>  replace_ref (tree *expr, slsr_cand_t c)
>  {
>!   tree add_expr, mem_ref, acc_type = TREE_TYPE (*expr);
>!   unsigned HOST_WIDE_INT misalign;
>!   unsigned align;
>! 
>!   /* Ensure the memory reference carries the minimum alignment
>!      requirement for the data type.  See PR58041.  */
>!   get_object_alignment_1 (*expr, &align, &misalign);
>!   if (misalign != 0)
>!     align = (misalign & -misalign);
>!   if (align < TYPE_ALIGN (acc_type))
>!     acc_type = build_aligned_type (acc_type, align);
>! 
>!   add_expr = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE
>(c->base_expr),
>! 			  c->base_expr, c->stride);
>!   mem_ref = fold_build2 (MEM_REF, acc_type, add_expr,
>! 			 double_int_to_tree (c->cand_type, c->index));
>! 
>/* Gimplify the base addressing expression for the new MEM_REF tree. 
>*/
>    gimple_stmt_iterator gsi = gsi_for_stmt (c->cand_stmt);
>    TREE_OPERAND (mem_ref, 0)
>*** /dev/null	Fri Aug  2 16:49:33 2013
>--- gcc/testsuite/gcc.dg/torture/pr58041.c	Mon Aug  5 18:56:10 2013
>***************
>*** 0 ****
>--- 1,35 ----
>+ /* { dg-do run } */
>+ 
>+ typedef long long V
>+   __attribute__ ((vector_size (2 * sizeof (long long)), may_alias));
>+ 
>+ typedef struct S { V v; } P __attribute__((aligned (1)));
>+ 
>+ struct s
>+ {
>+   char u;
>+   V v[2];
>+ } __attribute__((packed,aligned(1)));
>+ 
>+ __attribute__((noinline, noclone))
>+ long long foo(struct s *x, int y, V z)
>+ {
>+   V a = x->v[y];
>+   x->v[y] = z;
>+   return a[1];
>+ }
>+ 
>+ struct s a = {0,{0,0}};
>+ int main()
>+ {
>+   V v1 = {0,1};
>+   V v2 = {0,2};
>+ 
>+   if (foo(&a,0,v1) != 0)
>+     __builtin_abort();
>+   if (foo(&a,0,v2) != 1)
>+     __builtin_abort();
>+   if (foo(&a,1,v1) != 0)
>+     __builtin_abort();
>+   return 0;
>+ }
>*** /dev/null	Fri Aug  2 16:49:33 2013
>--- gcc/testsuite/gcc.target/arm/pr58041.c	Mon Aug  5 19:02:04 2013
>***************
>*** 0 ****
>--- 1,30 ----
>+ /* { dg-do compile } */
>+ /* { dg-options "-Os -mno-unaligned-access" } */
>+ /* { dg-final { scan-assembler "ldrb" } } */
>+ /* { dg-final { scan-assembler "strb" } } */
>+ 
>+ struct s
>+ {
>+   char u;
>+   long long v[2];
>+ } __attribute__((packed,aligned(1)));
>+ 
>+ __attribute__((noinline, noclone))
>+ long long foo(struct s *x, int y, long long z)
>+ {
>+   long long a = x->v[y];
>+   x->v[y] = z;
>+   return a;
>+ }
>+ 
>+ struct s a = {0,{0,0}};
>+ int main()
>+ {
>+   if (foo(&a,0,1) != 0)
>+     __builtin_abort();
>+   if (foo(&a,0,2) != 1)
>+     __builtin_abort();
>+   if (foo(&a,1,1) != 0)
>+     __builtin_abort();
>+   return 0;
>+ }