[Bug middle-end/46758] New: [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[Bug middle-end/46758] New: [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[zsojka at seznam dot cz]

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

           Summary: [4.5/4.6 Regression] -fgraphite-identity produces
                    wrong code when using 64bit constants
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: middle-end
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: zsojka@seznam.cz
                CC: spop@gcc.gnu.org
              Host: x86_64-pc-linux-gnu
            Target: x86_64-pc-linux-gnu


Created attachment 22597
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=22597
reduced testcase (from gcc.c-torture/execute/20100416-1.c)

Output:
$ gcc -O -fgraphite-identity pr46758.c
$ ./a.out 
Aborted

Tested revisions:
r167331 - fail
r153685 - fail
4.5 r166509 - fail
4.4 r166509 - OK

[Bug middle-end/46758] [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.5.2

[Bug middle-end/46758] [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[amonakov at gcc dot gnu.org]

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

Alexander Monakov <amonakov at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2010.12.03 15:27:13
                 CC|                            |amonakov at gcc dot gnu.org
     Ever Confirmed|0                           |1

--- Comment #1 from Alexander Monakov <amonakov at gcc dot gnu.org> 2010-12-03 15:27:13 UTC ---
In this particular example, the problem is in scan_tree_for_params_int:

  int v = int_cst_value (cst);

  mpz_init (val);
  mpz_set_si (val, 0);

  /* Necessary to not get "-1 = 2^n - 1". */
  if (v < 0)
    mpz_sub_ui (val, val, -v);
  else
    mpz_add_ui (val, val, v);

(gdb) p cst
$8 = (tree) 0x7ffff5a4e848
(gdb) pt
 <integer_cst 0x7ffff5a4e848 type <integer_type 0x7ffff7ed0738 long long int>
constant -8589934593>
(gdb) p v
$9 = -1
(gdb) 

There are multiple places where graphite uses int_cst_value.  Note that the
problem may be more severe on 32-bit hosts as there the tree may hold a 64-bit
constant but mpz_*_{u,s}i accepts 'long's.

[Bug middle-end/46758] [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

--- Comment #2 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-12-05 11:57:20 UTC ---
Graphite needs to properly use double_int functions (mpz_set_double_int is
your friend).

[Bug middle-end/46758] [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[amonakov at gcc dot gnu.org]

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

Alexander Monakov <amonakov at gcc dot gnu.org> changed:

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

--- Comment #3 from Alexander Monakov <amonakov at gcc dot gnu.org> 2010-12-08 18:03:57 UTC ---
I'll work on a patch

[Bug middle-end/46758] [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[amonakov at gcc dot gnu.org]

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

Alexander Monakov <amonakov at gcc dot gnu.org> changed:

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

--- Comment #4 from Alexander Monakov <amonakov at gcc dot gnu.org> 2010-12-13 17:24:59 UTC ---
Ouch.  The "obvious" fix:

diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
index 5036fba..8fda288 100644
--- a/gcc/graphite-sese-to-poly.c
+++ b/gcc/graphite-sese-to-poly.c
@@ -626,16 +626,9 @@ scan_tree_for_params_int (tree cst,
ppl_Linear_Expression_t expr, mpz_t k)
 {
   mpz_t val;
   ppl_Coefficient_t coef;
-  int v = int_cst_value (cst);

   mpz_init (val);
-  mpz_set_si (val, 0);
-
-  /* Necessary to not get "-1 = 2^n - 1". */
-  if (v < 0)
-    mpz_sub_ui (val, val, -v);
-  else
-    mpz_add_ui (val, val, v);
+  tree_int_to_gmp (cst, val);

   mpz_mul (val, val, k);
   ppl_new_Coefficient (&coef);

Makes Graphite fail most tests because this scanning machinery curiously
depends on signed/unsigned conversions/overflow.  For example, when an IV upper
bound N-1 is expressed as (unsigned)(N + 0xFFFFFFFFU), the patch makes Graphite
actually generate N + 0xFFFFFFFFU as upper bound in the corresponding PPL
expression:

(gdb) p nb_iters
$2 = (tree) 0x7ffff7ff9ee0
(gdb) pt
 <plus_expr 0x7ffff7ff9ee0
    type <integer_type 0x7ffff7ed0540 unsigned int public unsigned SI
        size <integer_cst 0x7ffff7ebd6e0 constant 32>
        unit size <integer_cst 0x7ffff7ebd3e8 constant 4>
        align 32 symtab 0 alias set -1 canonical type 0x7ffff7ed0540 precision
32 min <integer_cst 0x7ffff7ebd708 0> max <integer_cst 0x7ffff7ebd6b8
4294967295>
        pointer_to_this <pointer_type 0x7ffff7ee5150>>

    arg 0 <nop_expr 0x7ffff5a72420 type <integer_type 0x7ffff7ed0540 unsigned
int>

        arg 0 <ssa_name 0x7ffff5a73370 type <integer_type 0x7ffff7ed0498 int>
            visited var <parm_decl 0x7ffff7ec6880 y>def_stmt GIMPLE_NOP

            version 6>>
    arg 1 <integer_cst 0x7ffff7ebd6b8 type <integer_type 0x7ffff7ed0540
unsigned int> constant 4294967295>>
(gdb) call debug_ppl_linear_expr (ub_expr)
1 5
0 0 1 0 4294967295 

I don't know how to fix this; thus, unassigning.  Sorry.

[Bug middle-end/46758] [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.5.2                       |4.5.3

--- Comment #5 from Richard Guenther <rguenth at gcc dot gnu.org> 2010-12-16 13:03:45 UTC ---
GCC 4.5.2 is being released, adjusting target milestone.

[Bug middle-end/46758] [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[spop at gcc dot gnu.org]

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

Sebastian Pop <spop at gcc dot gnu.org> changed:

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

--- Comment #6 from Sebastian Pop <spop at gcc dot gnu.org> 2010-12-23 00:19:19 UTC ---
Patch:
http://gcc.gnu.org/ml/gcc-patches/2010-12/msg01743.html

[Bug middle-end/46758] [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[spop at gcc dot gnu.org]

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

--- Comment #7 from Sebastian Pop <spop at gcc dot gnu.org> 2010-12-23 16:26:17 UTC ---
Author: spop
Date: Thu Dec 23 16:26:11 2010
New Revision: 168211

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168211
Log:
Fix PR46758: Do not use int_cst_value.

2010-12-23  Sebastian Pop  <sebastian.pop@amd.com>
        Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/46758
    * graphite-sese-to-poly.c (scan_tree_for_params_right_scev): Use
    tree_int_to_gmp instead of int_cst_value.
    (scan_tree_for_params_int): Same.
    (scan_tree_for_params): Same.
    (pdr_add_data_dimensions): Use ppl_set_inhomogeneous_tree.

    * gcc.dg/graphite/run-id-pr46758.c: New.

Added:
    trunk/gcc/testsuite/gcc.dg/graphite/run-id-pr46758.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/graphite-sese-to-poly.c
    trunk/gcc/testsuite/ChangeLog

[Bug middle-end/46758] [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[spop at gcc dot gnu.org]

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

Sebastian Pop <spop at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.6.0
            Version|4.6.0                       |4.5.3
      Known to fail|4.6.0                       |

--- Comment #8 from Sebastian Pop <spop at gcc dot gnu.org> 2010-12-23 16:35:00 UTC ---
Fixed on 4.6, I am backporting this to 4.5.

[Bug middle-end/46758] [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[spop at gcc dot gnu.org]

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

--- Comment #9 from Sebastian Pop <spop at gcc dot gnu.org> 2010-12-23 18:52:15 UTC ---
Author: spop
Date: Thu Dec 23 18:52:12 2010
New Revision: 168214

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=168214
Log:
Fix PR46758: Do not use int_cst_value.

2010-12-23  Sebastian Pop  <sebastian.pop@amd.com>
        Richard Guenther  <rguenther@suse.de>

    PR tree-optimization/46758
    * graphite-sese-to-poly.c (scan_tree_for_params_right_scev): Use
    tree_int_to_gmp instead of int_cst_value.
    (scan_tree_for_params_int): Same.
    (scan_tree_for_params): Same.
    (pdr_add_data_dimensions): Use ppl_set_inhomogeneous_tree.

    * gcc.dg/graphite/run-id-pr46758.c: New.

Added:
    branches/gcc-4_5-branch/gcc/testsuite/gcc.dg/graphite/run-id-pr46758.c
Modified:
    branches/gcc-4_5-branch/gcc/ChangeLog
    branches/gcc-4_5-branch/gcc/graphite-sese-to-poly.c
    branches/gcc-4_5-branch/gcc/testsuite/ChangeLog

[Bug middle-end/46758] [4.5/4.6 Regression] -fgraphite-identity produces wrong code when using 64bit constants

[spop at gcc dot gnu.org]

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

Sebastian Pop <spop at gcc dot gnu.org> changed:

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

--- Comment #10 from Sebastian Pop <spop at gcc dot gnu.org> 2010-12-23 18:53:48 UTC ---
Fixed.