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)?
Thanks,
Martin
2013-08-05 Martin Jambor <mjam...@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;
+ }
