public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning
@ 2011-08-13  0:01 arthur.j.odwyer at gmail dot com
  2011-08-14  9:49 ` [Bug tree-optimization/50067] [4.7 Regression] " rguenth at gcc dot gnu.org
                   ` (15 more replies)
  0 siblings, 16 replies; 17+ messages in thread
From: arthur.j.odwyer at gmail dot com @ 2011-08-13  0:01 UTC (permalink / raw)
  To: gcc-bugs

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

             Bug #: 50067
           Summary: Wrong code with -fpredictive-commoning
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: arthur.j.odwyer@gmail.com


Created attachment 24994
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=24994
Output of "ajo-gcc -std=c99 -O -fpredictive-commoning test.c -v"

This failure reproduces for me with svn revision 177688 (2011-08-11). I'm on
Ubuntu 10.10, x86-64.


cat >test.c <<EOF
#include <stdio.h>
int a[6] = {0, 0, 0, 0, 7, 0};
static int *p = &a[4];
int main (int argc, char* argv[]) {
    for (int i=0; i < 4; ++i) {
        a[i+1] = (a[i+2] > i);
        *p &= ~1;
    }
    printf("%d\n", a[4]);
}
EOF
gcc -std=c99 -O test.c ; ./a.out
gcc -std=c99 -O -fpredictive-commoning test.c ; ./a.out


The first command line above correctly produces "0".
The second command line, with -fpredictive-commoning, produces "6".
Note that -fpredictive-commoning is enabled as part of -O3.


This test case is reduced from the output of Csmith 2.1.0 (git hash 01aa8b04,
https://github.com/Quuxplusone/csmith/), using the following command line:
csmith --paranoid --no-longlong --pointers --arrays --jumps --no-consts
--volatiles --checksum --no-divs --muls --no-bitfields --no-packed-struct -s
278348999


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
@ 2011-08-14  9:49 ` rguenth at gcc dot gnu.org
  2011-08-18 11:22 ` jakub at gcc dot gnu.org
                   ` (14 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-14  9:49 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.7.0
            Summary|Wrong code with             |[4.7 Regression] Wrong code
                   |-fpredictive-commoning      |with -fpredictive-commoning

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-14 09:45:37 UTC ---
Probably caused by allowing invariant DRs.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
  2011-08-14  9:49 ` [Bug tree-optimization/50067] [4.7 Regression] " rguenth at gcc dot gnu.org
@ 2011-08-18 11:22 ` jakub at gcc dot gnu.org
  2011-08-18 11:31 ` jakub at gcc dot gnu.org
                   ` (13 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2011-08-18 11:22 UTC (permalink / raw)
  To: gcc-bugs

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-08-18
                 CC|                            |jakub at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-08-18 11:21:45 UTC ---
Indeed, started failing with
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=175704


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
  2011-08-14  9:49 ` [Bug tree-optimization/50067] [4.7 Regression] " rguenth at gcc dot gnu.org
  2011-08-18 11:22 ` jakub at gcc dot gnu.org
@ 2011-08-18 11:31 ` jakub at gcc dot gnu.org
  2011-08-18 14:14 ` jakub at gcc dot gnu.org
                   ` (12 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2011-08-18 11:31 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-08-18 11:22:54 UTC ---
Smaller testcase:
extern void abort (void);
int a[6] = { 0, 0, 0, 0, 7, 0 };
static int *p = &a[4];

int
main ()
{
  int i;
  for (i = 0; i < 4; ++i)
    {
      a[i + 1] = a[i + 2] > i;
      *p &= ~1;
    }
  if (a[4] != 0)
    abort ();
  return 0;
}


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (2 preceding siblings ...)
  2011-08-18 11:31 ` jakub at gcc dot gnu.org
@ 2011-08-18 14:14 ` jakub at gcc dot gnu.org
  2011-08-19  9:18 ` rguenth at gcc dot gnu.org
                   ` (11 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2011-08-18 14:14 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-08-18 13:38:16 UTC ---
The problem seems to be probably in dr_analyze_indices.  When it is looking at
a[i + 1] or a[i + 2] DR_REF is ARRAY_REF and the computed access_fn is like
{2, +, 1}_1, which means index into a array, while for the MEM_REF[&a + 16B]
accesses the computed access_fn is 16 (i.e. byte index into a array, rather
than
comparable index 4).
So probably the off for the MEM_REF case needs to be divided by the size of the
array entry, the question is what to do with unaligned MEM_REF[&a + 15B] and
similar DR_REFs...


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (3 preceding siblings ...)
  2011-08-18 14:14 ` jakub at gcc dot gnu.org
@ 2011-08-19  9:18 ` rguenth at gcc dot gnu.org
  2011-08-19 11:24 ` rguenth at gcc dot gnu.org
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-19  9:18 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-19 09:14:20 UTC ---
Mine.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (4 preceding siblings ...)
  2011-08-19  9:18 ` rguenth at gcc dot gnu.org
@ 2011-08-19 11:24 ` rguenth at gcc dot gnu.org
  2011-08-19 11:52 ` jakub at gcc dot gnu.org
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-19 11:24 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |spop at gcc dot gnu.org

--- Comment #6 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-19 11:16:41 UTC ---
I think the bug is that we have

#(Data Ref: 
#  bb: 3 
#  stmt: D.2736_10 = MEM[(int *)&a + 16B];
#  ref: MEM[(int *)&a + 16B];
#  base_object: a
#  Access function 0: 16B

thus, an access function with a byte offset.  I don't see how this couldn't
happen with the 4.5 code-base and the indirect-ref handling as well.

Thus, the code in dr_analyze_indices that pushes an access-function for

  if (nest
      && (INDIRECT_REF_P (aref)
          || TREE_CODE (aref) == MEM_REF))
    {

probably shouldn't do so if the base analysis

      op = TREE_OPERAND (aref, 0);
      access_fn = analyze_scalar_evolution (loop, op);
      access_fn = instantiate_scev (before_loop, loop, access_fn);

isn't a POLYNOMIAL_CHREC.  Because then we have no idea if the
steps / element sizes are compatible.

For example for

int a[256] = { 0, 0, 0, 0, 7, 9, 0, 0, };
int main()
{
  int i;
  for (i = 0; i < 256; ++i)
    {
      a[i] = (*((char(*)[8])&a[4]))[i];
    }
}

we create

Creating dr for MEM[(char[8] *)&a + 16B][i_12]
analyze_innermost: success.
        base_address: &a
        offset from base address: 0
        constant offset from base address: 16
        step: 1
        aligned to: 128
        base_object: MEM[(char[8] *)&a]
Creating dr for a[i_12]
analyze_innermost: success.
        base_address: &a
        offset from base address: 0
        constant offset from base address: 0
        step: 4
        aligned to: 128
        base_object: a
(compute_affine_dependence
  (stmt_a =
D.2727_4 = MEM[(char[8] *)&a + 16B][i_12];
)
  (stmt_b =
a[i_12] = D.2728_5;
)
)
(Data Dep:
#(Data Ref:
#  bb: 3
#  stmt: D.2727_4 = MEM[(char[8] *)&a + 16B][i_12];
#  ref: MEM[(char[8] *)&a + 16B][i_12];
#  base_object: MEM[(char[8] *)&a];
#  Access function 0: {0, +, 1}_1
#  Access function 1: 16B
#)
#(Data Ref:
#  bb: 3
#  stmt: a[i_12] = D.2728_5;
#  ref: a[i_12];
#  base_object: a
#  Access function 0: {0, +, 1}_1
#)
    (don't know)

with a mismatched number of access functions.

On the 4.5 branch we seem to do the same and we seem to rely on the
mismatch in the number of access functions to say "don't know" for
dependence analysis.  Testcase:

extern void abort (void);
int a[6] = { 0, 0, 0, 0, 7, 0 };

int
main ()
{
  int i;
  int (*p)[1] = &a[4];
  for (i = 0; i < 4; ++i)
    {
      a[i + 1] = a[i + 2] > i;
      (*p)[i] &= ~1;
    }
  if (a[4] != 0)
    abort ();
  return 0;
}

#(Data Ref:
#  stmt: D.2727_9 = (*p_3)[i_19];
#  ref: (*p_3)[i_19];
#  base_object: (*(int[1] *) &a)[0];
#  Access function 0: {0, +, 1}_1
#  Access function 1: 16B

so the new "issue" seems to be that with invariant DRs now allowed
we have a spurious match in access functions, because with a
non-indirect ref base we do not add a access function for the base
(which would be simply 0B).

Of course then still

(Data Dep:
#(Data Ref:
#  bb: 3
#  stmt: D.2727_4 = MEM[(char[8] *)&a + 16B][i_12];
#  ref: MEM[(char[8] *)&a + 16B][i_12];
#  base_object: a
#  Access function 0: {0, +, 1}_1
#  Access function 1: 16B
#)
#(Data Ref:
#  bb: 3
#  stmt: a[i_12] = D.2728_5;
#  ref: a[i_12];
#  base_object: a
#  Access function 0: {0, +, 1}_1
#  Access function 1: 0
#)
    (no dependence)

because the code assumes that access functions affect "independent"
index spaces, if you look at

int a[256] = { 0, 0, 0, 0, 7, 9, 0, 0, };
int main()
{
  int i;
  for (i = 0; i < 256; ++i)
    {
      a[i] = (*((char(*)[8])&a[4]))[i];
    }
}

so it seems that rather than adding a separate access function for
the base pointer object (which could be dependent on i as well!)
we shouldn't do that at all.

Sebastian - do you remember anything about this code?

I'd simply do

Index: tree-data-ref.c
===================================================================
--- tree-data-ref.c     (revision 177891)
+++ tree-data-ref.c     (working copy)
@@ -862,34 +862,6 @@ dr_analyze_indices (struct data_referenc
       aref = TREE_OPERAND (aref, 0);
     }

-  if (nest
-      && (INDIRECT_REF_P (aref)
-         || TREE_CODE (aref) == MEM_REF))
-    {
-      op = TREE_OPERAND (aref, 0);
-      access_fn = analyze_scalar_evolution (loop, op);
-      access_fn = instantiate_scev (before_loop, loop, access_fn);
-      base = initial_condition (access_fn);
-      split_constant_offset (base, &base, &off);
-      if (TREE_CODE (aref) == MEM_REF)
-       off = size_binop (PLUS_EXPR, off,
-                         fold_convert (ssizetype, TREE_OPERAND (aref, 1)));
-      access_fn = chrec_replace_initial_condition (access_fn,
-                       fold_convert (TREE_TYPE (base), off));
-
-      TREE_OPERAND (aref, 0) = base;
-      VEC_safe_push (tree, heap, access_fns, access_fn);
-    }
-
-  if (TREE_CODE (aref) == MEM_REF)
-    TREE_OPERAND (aref, 1)
-      = build_int_cst (TREE_TYPE (TREE_OPERAND (aref, 1)), 0);


as that seems what is conservatively correct.

Now that will loose any index analysis on pointers ... but with
some obfuscation we can arbitrarily mix array and pointer-based
code, so ...

Relying on SCEV or access function count mismatches looks incredibly
fragile ...

For different array-views on the same array we also get bogus dependences:

int a[256] = { 0, 0, 0, 0, 7, 9, 0, 0, };
int main()
{
  int i;
  for (i = 0; i < 128; ++i)
    {
      a[i] = (*((char(*)[128])&a[0]))[i+128];
    }
}

(Data Dep:
#(Data Ref:
#  bb: 3
#  stmt: D.2728_5 = MEM[(char[128] *)&a][D.2727_4];
#  ref: MEM[(char[128] *)&a][D.2727_4];
#  base_object: a
#  Access function 0: {128, +, 1}_1
#)
#(Data Ref:
#  bb: 3
#  stmt: a[i_13] = D.2729_6;
#  ref: a[i_13];
#  base_object: a
#  Access function 0: {0, +, 1}_1
#)
    (no dependence)

so what the access_fns assume is that if ever two base objects are
the same then the access functions, in order of appearance in the
vector, are compatible (iff there is the same number of access functions).


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (5 preceding siblings ...)
  2011-08-19 11:24 ` rguenth at gcc dot gnu.org
@ 2011-08-19 11:52 ` jakub at gcc dot gnu.org
  2011-08-19 11:55 ` rguenth at gcc dot gnu.org
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: jakub at gcc dot gnu.org @ 2011-08-19 11:52 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2011-08-19 11:36:42 UTC ---
Can't we just standardize on some particular element size (say derived from
DR_BASE_ADDRESS's type or even record it in the data_reference structure), use
it if it can be expressed in those units and give up otherwise (both for
ARRAY_REF and for MEM_REF offsets)?


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (6 preceding siblings ...)
  2011-08-19 11:52 ` jakub at gcc dot gnu.org
@ 2011-08-19 11:55 ` rguenth at gcc dot gnu.org
  2011-08-19 12:59 ` rguenth at gcc dot gnu.org
                   ` (7 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-19 11:55 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #8 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-19 11:51:07 UTC ---
We seem to be "safe" in more cases also because of weird stripping of
MEM_REFs in dr_analyze_indices.

  if (TREE_CODE (ref) == MEM_REF
      && TREE_CODE (TREE_OPERAND (ref, 0)) == ADDR_EXPR
      && integer_zerop (TREE_OPERAND (ref, 1)))
    ref = TREE_OPERAND (TREE_OPERAND (ref, 0), 0);

if you move that after stripping zero-offset ARRAY_REFs (to also
strip the mem-ref to the base object for (*&a)[i]) then we miscompile
the following testcase because we vectorize it

extern int memcmp(const void *, const void *, __SIZE_TYPE__);
extern void abort (void);
short a[32] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31 };
short b[32] = { 4, 0, 5, 0, 6, 0, 7, 0, 8, 0, };
int main()
{
  int i;
  for (i = 0; i < 32; ++i)
    {
      a[i] = (*((char(*)[32])&a[0]))[i+8];
    }
  if (memcmp (&a, &b, sizeof (a)) != 0)
    abort ();
  return 0;
}

when we leave two mem-refs around with

extern int memcmp(const void *, const void *, __SIZE_TYPE__);
extern void abort (void);
short a[32] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31 };
short b[32] = { 4, 0, 5, 0, 6, 0, 7, 0, 8, 0, };
int main()
{
  int i;
  for (i = 0; i < 32; ++i)
    {
      (*((unsigned short(*)[32])&a[0]))[i] = (*((char(*)[32])&a[0]))[i+8];
    }
  if (memcmp (&a, &b, sizeof (a)) != 0)
    abort ();
  return 0;
}

then the base-object comparison fails.

I wonder if the C access-as character lvalue covers doing so through
arbitrary dimensioned char arrays ...

I'll be "incrementally" trying to fix some oddities in dr_analyze_indices.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (7 preceding siblings ...)
  2011-08-19 11:55 ` rguenth at gcc dot gnu.org
@ 2011-08-19 12:59 ` rguenth at gcc dot gnu.org
  2011-08-19 13:46 ` rguenth at gcc dot gnu.org
                   ` (6 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-19 12:59 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #9 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-19 12:00:11 UTC ---
I think the element sizes of the access functions need to be reflected in
the base object, which we require to be operand_equal_p to any other
base to which we compare our access functions.  I'm not yet 100%
convinced that is enough though (if you view two same-shaped two-dimensional
slices from a 3d array, for example, (int (*)[16][16])&a[i][j][k] and
(int (*)[16][16])&a[l][m][n] and index them with [p][q] it should be
possible to choose i,j,k,l,m,n so that data-dependence analysis thinks
they do not overlap while they do - well, hopefully not ;)).

I'm checking what regressions it will cause to remove

  if (TREE_CODE (ref) == MEM_REF
      && TREE_CODE (TREE_OPERAND (ref, 0)) == ADDR_EXPR
      && integer_zerop (TREE_OPERAND (ref, 1)))
    ref = TREE_OPERAND (TREE_OPERAND (ref, 0), 0);

which should "fix" this part for the particular case of an outermost-only
mem-ref (that's the case where it doesn't help too much anyway, and
moving it to a more useful place creates even more issues, see comment #8).


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (8 preceding siblings ...)
  2011-08-19 12:59 ` rguenth at gcc dot gnu.org
@ 2011-08-19 13:46 ` rguenth at gcc dot gnu.org
  2011-08-19 14:26 ` rguenth at gcc dot gnu.org
                   ` (5 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-19 13:46 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #10 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-19 13:36:49 UTC ---
Testcase showing that stripping the offset for the indirect base is bogus:

extern int memcmp(const void *, const void *, __SIZE_TYPE__);
extern void abort (void);
short a[33] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31 };
short b[33] = { 0, };
int main()
{
  int i;
  for (i = 0; i < 64; ++i)
    {
      (*((char(*)[])&a[1]))[i] = (*((char(*)[])&a[0]))[i+1];
    }
  if (memcmp (&a, &b, sizeof (a)) != 0)
    abort ();
  return 0;
}

is vectorized because:

Creating dr for MEM[(char[<unknown>] *)&a][i_5]
analyze_innermost: success.
        base_address: &a
        offset from base address: 0
        constant offset from base address: 1
        step: 1
        aligned to: 128
        base_object: MEM[(char[<unknown>] *)&a]
Creating dr for MEM[(char[<unknown>] *)&a + 2B][i_15]
analyze_innermost: success.
        base_address: &a
        offset from base address: 0
        constant offset from base address: 2
        step: 1
        aligned to: 128
        base_object: MEM[(char[<unknown>] *)&a]
(compute_affine_dependence
  (stmt_a =
D.2739_6 = MEM[(char[<unknown>] *)&a][i_5];
)
  (stmt_b =
MEM[(char[<unknown>] *)&a + 2B][i_15] = D.2739_6;
)
(subscript_dependence_tester
(analyze_overlapping_iterations
  (chrec_a = {1, +, 1}_1)
  (chrec_b = {0, +, 1}_1)
(analyze_siv_subscript
(analyze_subscript_affine_affine
  (overlaps_a = [0 + 1 * x_1]
)
  (overlaps_b = [1 + 1 * x_1]
)
)
)
  (overlap_iterations_a = [0 + 1 * x_1]
)
  (overlap_iterations_b = [1 + 1 * x_1]
)
)
(analyze_overlapping_iterations
  (chrec_a = 0B)
  (chrec_b = 2B)
(analyze_ziv_subscript
)
  (overlap_iterations_a = no dependence
)
  (overlap_iterations_b = no dependence
)
)
(dependence classified: scev_known)
)
)

well - that access_fn created for the indirect base is thought to be
"independent" on any of the other access_fns:

(Data Dep:
#(Data Ref:
#  bb: 3
#  stmt: D.2739_20 = MEM[(char[<unknown>] *)&a][i_19];
#  ref: MEM[(char[<unknown>] *)&a][i_19];
#  base_object: MEM[(char[<unknown>] *)&a];
#  Access function 0: {1, +, 1}_2
#  Access function 1: 0B
#)
#(Data Ref:
#  bb: 3
#  stmt: MEM[(char[<unknown>] *)&a + 2B][i_10] = D.2739_20;
#  ref: MEM[(char[<unknown>] *)&a + 2B][i_10];
#  base_object: MEM[(char[<unknown>] *)&a];
#  Access function 0: {0, +, 1}_2
#  Access function 1: 2B
#)
    (no dependence)

that's clearly bogus.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (9 preceding siblings ...)
  2011-08-19 13:46 ` rguenth at gcc dot gnu.org
@ 2011-08-19 14:26 ` rguenth at gcc dot gnu.org
  2011-08-19 14:29 ` rguenth at gcc dot gnu.org
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-19 14:26 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #11 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-19 14:23:51 UTC ---
That is exposed by my 2nd posted patch as well, for
gcc.c-torture/execute/pr33870-1.c.  Testing a fix.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (10 preceding siblings ...)
  2011-08-19 14:26 ` rguenth at gcc dot gnu.org
@ 2011-08-19 14:29 ` rguenth at gcc dot gnu.org
  2011-08-22  8:53 ` rguenth at gcc dot gnu.org
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-19 14:29 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #12 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-19 14:26:19 UTC ---
Author: rguenth
Date: Fri Aug 19 14:26:13 2011
New Revision: 177903

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=177903
Log:
2011-08-19  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/50067
    * tree-data-ref.c (dr_analyze_indices): Simplify, strip MEM_REF
    offset only if we accounted for it.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-data-ref.c


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (11 preceding siblings ...)
  2011-08-19 14:29 ` rguenth at gcc dot gnu.org
@ 2011-08-22  8:53 ` rguenth at gcc dot gnu.org
  2011-08-22  9:40 ` rguenth at gcc dot gnu.org
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-22  8:53 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #13 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-22 08:39:18 UTC ---
*sigh*, and when we fix it like
http://gcc.gnu.org/ml/gcc-patches/2011-08/msg01582.html then
gcc.dg/torture/pr44913.c is vectorized bogously because
dr_may_alias_p returns false for a[1] and a[-1] (and we don't compute any
difference vector).

I suppose dr_may_alias_p is completely wrong and does not want to know
whether

/* Returns false if we can prove that data references A and B do not alias,
   true otherwise.  */

but instead whether the data references may refer to the same base object
(plus, I guess, whether the references may alias using TBAA - though as
soon as we'd start to do strided stores that will break as well for
struct { int i; float f; } a[]).

But of course DR_BASE_OBJECT is not what we should adjust to "fix"
the dr_may_alias_p answer, but we need to adjust dr_may_alias_p instead
(and hope all users do mean to get the same kind of answer... other
than ddr init it is graphite that calls it, I will simply ignore that).


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (12 preceding siblings ...)
  2011-08-22  8:53 ` rguenth at gcc dot gnu.org
@ 2011-08-22  9:40 ` rguenth at gcc dot gnu.org
  2011-08-24 10:12 ` rguenth at gcc dot gnu.org
  2011-08-24 10:17 ` rguenth at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-22  9:40 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #14 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-22 09:38:43 UTC ---
Testcase which shows that mixing array and base access fns leads to bogus
results (obfuscated, SCEV doesn't handle ADDR_EXPRs well ...):

extern int memcmp(const void *, const void *, __SIZE_TYPE__);
extern void abort (void);
short a[33] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17,
18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31 };
short b[33] = { 0, };
volatile char *ap_ = &a[0];
int main()
{
  int i;
  char *ap = ap_;
  for (i = 0; i < 64; ++i)
    {
      (*((char(*)[])&ap[i+2]))[0] = (*((char(*)[])&ap[0]))[i+1];
    }
  if (memcmp (&a, &b, sizeof (a)) != 0)
    abort ();
  return 0;
}

Here we will end up with

(Data Dep:
#(Data Ref:
#  bb: 3
#  stmt: D.2743_8 = MEM[(char[<unknown>] *)ap_2][i_7];
#  ref: MEM[(char[<unknown>] *)ap_2][i_7];
#  base_object: MEM[(char[<unknown>] *)ap_2];
#  Access function 0: {1, +, 1}_1
#  Access function 1: 0B
#)
#(Data Ref:
#  bb: 3
#  stmt: MEM[(char[<unknown>] *)D.2741_6][0] = D.2743_8;
#  ref: MEM[(char[<unknown>] *)D.2741_6][0];
#  base_object: MEM[(char[<unknown>] *)ap_2];
#  Access function 0: 0
#  Access function 1: {2B, +, 1}_1
#)
    (no dependence)

which is obviously broken - those two access functions for the
respective ref are _not_ independent.


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (13 preceding siblings ...)
  2011-08-22  9:40 ` rguenth at gcc dot gnu.org
@ 2011-08-24 10:12 ` rguenth at gcc dot gnu.org
  2011-08-24 10:17 ` rguenth at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-24 10:12 UTC (permalink / raw)
  To: gcc-bugs

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

--- Comment #15 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-24 10:07:24 UTC ---
Author: rguenth
Date: Wed Aug 24 10:07:20 2011
New Revision: 178028

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=178028
Log:
2011-08-24  Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/50067
    * tree-data-ref.c (dr_analyze_indices): Do not add an access
    function for a MEM_REF base that has no evolution in the loop
    nest or that is not analyzable.

    * gcc.dg/torture/pr50067-3.c: New testcase.
    * gcc.dg/torture/pr50067-4.c: Likewise.
    * gcc.dg/torture/pr50067-5.c: Likewise.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr50067-3.c
    trunk/gcc/testsuite/gcc.dg/torture/pr50067-4.c
    trunk/gcc/testsuite/gcc.dg/torture/pr50067-5.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-data-ref.c


^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Bug tree-optimization/50067] [4.7 Regression] Wrong code with -fpredictive-commoning
  2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
                   ` (14 preceding siblings ...)
  2011-08-24 10:12 ` rguenth at gcc dot gnu.org
@ 2011-08-24 10:17 ` rguenth at gcc dot gnu.org
  15 siblings, 0 replies; 17+ messages in thread
From: rguenth at gcc dot gnu.org @ 2011-08-24 10:17 UTC (permalink / raw)
  To: gcc-bugs

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

Richard Guenther <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #16 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-24 10:14:29 UTC ---
Ok, "fixed" now.  There are still tons of issues, but I'll stop creating
artificial testcases now as I'm somewhat tired after looking at this
and thinking about this code for three days ...

I eventually expect fallouts.

No idea what pieces would qualify for a backport, but at least this
particular bug isn't exposed on the 4.6 branch anyway.


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2011-08-24 10:17 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-13  0:01 [Bug tree-optimization/50067] New: Wrong code with -fpredictive-commoning arthur.j.odwyer at gmail dot com
2011-08-14  9:49 ` [Bug tree-optimization/50067] [4.7 Regression] " rguenth at gcc dot gnu.org
2011-08-18 11:22 ` jakub at gcc dot gnu.org
2011-08-18 11:31 ` jakub at gcc dot gnu.org
2011-08-18 14:14 ` jakub at gcc dot gnu.org
2011-08-19  9:18 ` rguenth at gcc dot gnu.org
2011-08-19 11:24 ` rguenth at gcc dot gnu.org
2011-08-19 11:52 ` jakub at gcc dot gnu.org
2011-08-19 11:55 ` rguenth at gcc dot gnu.org
2011-08-19 12:59 ` rguenth at gcc dot gnu.org
2011-08-19 13:46 ` rguenth at gcc dot gnu.org
2011-08-19 14:26 ` rguenth at gcc dot gnu.org
2011-08-19 14:29 ` rguenth at gcc dot gnu.org
2011-08-22  8:53 ` rguenth at gcc dot gnu.org
2011-08-22  9:40 ` rguenth at gcc dot gnu.org
2011-08-24 10:12 ` rguenth at gcc dot gnu.org
2011-08-24 10:17 ` rguenth at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).