Martin Jambor <mjam...@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 <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;
>+ }
