Jakub Jelinek <ja...@redhat.com> wrote:
>Hi!
>
>On the gomp4 branch I've noticed a miscompilation of the simd-3.C
>testcase I'm adding there, but even say
>int a[1024] __attribute__((aligned (32))) = { 1 };
>int b[1024] __attribute__((aligned (32))) = { 1 };
>unsigned short c[1024] __attribute__((aligned (32))) = { 1 };
>
>__attribute__((noinline, noclone)) void
>foo (int *p)
>{
> int i;
> p = (int *) __builtin_assume_aligned (p, 32);
> for (i = 0; i < 512; i++)
> {
> a[i] *= p[i];
> c[i]++;
> }
>}
>
>int
>main ()
>{
> int i;
> for (i = 0; i < 512; i++)
> {
> a[i] = i - 512;
> b[i] = (i - 51) % 39;
> c[i] = (unsigned short) i;
> }
> foo (b);
> for (i = 0; i < 512; i++)
> if (b[i] != (i - 51) % 39
> || a[i] != (i - 512) * b[i]
> || c[i] != (unsigned short) (i + 1))
> __builtin_abort ();
> return 0;
>}
>without -fopenmp, just -O3 -mavx. The relevant change was just
>that ptr_incr has been initialized to NULL in both vectorizable_store
>and vectorizable_load, because vect_create_data_ref_ptr doesn't
>initialize
>it if only_init is true. Looking at it, we just trigger undefined
>behavior
>and are just lucky that it works by accident on the trunk and branches.
>The problem is that if ncopies > 1, vectorizable_store declares the
>variable
>inside of the loop, for j == 0 it initializes it through
>vect_create_data_ref_ptr, but then on the next iteration the variable
>is
>uninitialized again (but just due to luck contains the value from the
>previous iteration, but say if compiler unrolled the loop, it would
>already
>misbehave) and then it is passed to bump_vector_ptr. Fixed by moving
>the
>var decl outside of the loop. While not strictly necessary, I find it
>cleaner to initialize it to NULL, though if you disagree with it, I can
>keep
>that change local to gomp4 branch for now (i.e. remove " = NULL" from
>the
>first hunk and the third hunk).
>
>Ok for trunk/4.8?
>
Ok
Thanks,
Richard.
>2013-06-26 Jakub Jelinek <ja...@redhat.com>
>
> * tree-vect-stmts.c (vectorizable_store): Move ptr_incr var
> decl before the loop, initialize to NULL.
> (vectorizable_load): Initialize ptr_incr to NULL.
>
>--- gcc/tree-vect-stmts.c.jj 2013-04-22 08:06:41.000000000 +0200
>+++ gcc/tree-vect-stmts.c 2013-06-26 21:34:28.609654773 +0200
>@@ -3796,6 +3796,7 @@ vectorizable_store (gimple stmt, gimple_
> enum vect_def_type dt;
> stmt_vec_info prev_stmt_info = NULL;
> tree dataref_ptr = NULL_TREE;
>+ gimple ptr_incr = NULL;
> int nunits = TYPE_VECTOR_SUBPARTS (vectype);
> int ncopies;
> int j;
>@@ -4041,7 +4042,6 @@ vectorizable_store (gimple stmt, gimple_
> for (j = 0; j < ncopies; j++)
> {
> gimple new_stmt;
>- gimple ptr_incr;
>
> if (j == 0)
> {
>@@ -4314,7 +4314,7 @@ vectorizable_load (gimple stmt, gimple_s
> tree dummy;
> enum dr_alignment_support alignment_support_scheme;
> tree dataref_ptr = NULL_TREE;
>- gimple ptr_incr;
>+ gimple ptr_incr = NULL;
> int nunits = TYPE_VECTOR_SUBPARTS (vectype);
> int ncopies;
> int i, j, group_size, group_gap;
>
>
> Jakub
