diff mbox

Sign extend before converting constants to GMP values.

Message ID 1309382231-19263-1-git-send-email-sebpop@gmail.com
State New
Headers show

Commit Message

Sebastian Pop June 29, 2011, 9:17 p.m. UTC
Hi,

This patch fixes PR47653 by sign extending the double int constants
before converting them to a GMP value.  There still are some places
where we should not sign extend the values converted: upper bounds of
unsigned types should for example not be sign extended.

The patch passed make -k check RUNTESTFLAGS=graphite.exp on c,c++,fortran.
Regstrap with all languages in progress on amd64-linux.  Ok for trunk?

Thanks,
Sebastian


2011-06-29  Sebastian Pop  <sebastian.pop@amd.com>

	PR tree-optimization/47653
	* graphite-ppl.h (tree_int_to_gmp): Sign extend before converting
	constants to GMP values.  Add a sext parameter.
	(ppl_set_inhomogeneous_tree): Add sext parameter.
	(ppl_set_coef_tree): Removed.
	* graphite-sese-to-poly.c (scan_tree_for_params_right_scev): Adjust
	call to tree_int_to_gmp.
	(scan_tree_for_params_int): Use tree_int_to_gmp.
	(scan_tree_for_params): Adjust call to tree_int_to_gmp.
	(build_loop_iteration_domains): Adjust call to
	ppl_set_inhomogeneous_tree.
	(add_param_constraints): Same.
	(pdr_add_data_dimensions): Same.

	* gcc.dg/graphite/run-id-pr47653.c: New.
---
 gcc/ChangeLog                                  |   16 ++++++++++++
 gcc/graphite-ppl.h                             |   30 +++++++++--------------
 gcc/graphite-sese-to-poly.c                    |   27 ++++++++++-----------
 gcc/testsuite/ChangeLog                        |    5 ++++
 gcc/testsuite/gcc.dg/graphite/run-id-pr47653.c |   17 +++++++++++++
 5 files changed, 63 insertions(+), 32 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/graphite/run-id-pr47653.c

Comments

Richard Biener June 30, 2011, 10:19 a.m. UTC | #1
On Wed, 29 Jun 2011, Sebastian Pop wrote:

> Hi,
> 
> This patch fixes PR47653 by sign extending the double int constants
> before converting them to a GMP value.  There still are some places
> where we should not sign extend the values converted: upper bounds of
> unsigned types should for example not be sign extended.
> 
> The patch passed make -k check RUNTESTFLAGS=graphite.exp on c,c++,fortran.
> Regstrap with all languages in progress on amd64-linux.  Ok for trunk?
> 
> Thanks,
> Sebastian
> 
> 
> 2011-06-29  Sebastian Pop  <sebastian.pop@amd.com>
> 
> 	PR tree-optimization/47653
> 	* graphite-ppl.h (tree_int_to_gmp): Sign extend before converting
> 	constants to GMP values.  Add a sext parameter.
> 	(ppl_set_inhomogeneous_tree): Add sext parameter.
> 	(ppl_set_coef_tree): Removed.
> 	* graphite-sese-to-poly.c (scan_tree_for_params_right_scev): Adjust
> 	call to tree_int_to_gmp.
> 	(scan_tree_for_params_int): Use tree_int_to_gmp.
> 	(scan_tree_for_params): Adjust call to tree_int_to_gmp.
> 	(build_loop_iteration_domains): Adjust call to
> 	ppl_set_inhomogeneous_tree.
> 	(add_param_constraints): Same.
> 	(pdr_add_data_dimensions): Same.
> 
> 	* gcc.dg/graphite/run-id-pr47653.c: New.
> ---
>  gcc/ChangeLog                                  |   16 ++++++++++++
>  gcc/graphite-ppl.h                             |   30 +++++++++--------------
>  gcc/graphite-sese-to-poly.c                    |   27 ++++++++++-----------
>  gcc/testsuite/ChangeLog                        |    5 ++++
>  gcc/testsuite/gcc.dg/graphite/run-id-pr47653.c |   17 +++++++++++++
>  5 files changed, 63 insertions(+), 32 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/graphite/run-id-pr47653.c
> 
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index e37d823..bed0070 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,19 @@
> +2011-06-29  Sebastian Pop  <sebastian.pop@amd.com>
> +
> +	PR tree-optimization/47653
> +	* graphite-ppl.h (tree_int_to_gmp): Sign extend before converting
> +	constants to GMP values.  Add a sext parameter.
> +	(ppl_set_inhomogeneous_tree): Add sext parameter.
> +	(ppl_set_coef_tree): Removed.
> +	* graphite-sese-to-poly.c (scan_tree_for_params_right_scev): Adjust
> +	call to tree_int_to_gmp.
> +	(scan_tree_for_params_int): Use tree_int_to_gmp.
> +	(scan_tree_for_params): Adjust call to tree_int_to_gmp.
> +	(build_loop_iteration_domains): Adjust call to
> +	ppl_set_inhomogeneous_tree.
> +	(add_param_constraints): Same.
> +	(pdr_add_data_dimensions): Same.
> +
>  2011-06-29  Eric Botcazou  <ebotcazou@adacore.com>
>  
>  	PR tree-optimization/49539
> diff --git a/gcc/graphite-ppl.h b/gcc/graphite-ppl.h
> index 695d01f..4ae9f63 100644
> --- a/gcc/graphite-ppl.h
> +++ b/gcc/graphite-ppl.h
> @@ -50,13 +50,18 @@ void debug_gmp_value (mpz_t);
>  bool ppl_powerset_is_empty (ppl_Pointset_Powerset_C_Polyhedron_t);
>  
>  
> -/* Assigns to RES the value of the INTEGER_CST T.  */
> +/* Assigns to RES the value of the INTEGER_CST T.  When SEXT is true,
> +   sign extend the value of T to not get "-1 = 2^n - 1".  */
>  
>  static inline void
> -tree_int_to_gmp (tree t, mpz_t res)
> +tree_int_to_gmp (tree t, mpz_t res, bool sext)
>  {
> +  tree type = TREE_TYPE (t);
>    double_int di = tree_to_double_int (t);
> -  mpz_set_double_int (res, di, TYPE_UNSIGNED (TREE_TYPE (t)));
> +
> +  if (sext)
> +    di = double_int_sext (di, TYPE_PRECISION (type));
> +  mpz_set_double_int (res, di, false);

That looks odd.  So you given -1U as input you sign-extend that to -1
and then set the mpz to -1ULLL.  In fact it looks like you either
have non-canoncial INTEGER_CSTs from the start or the type of the
integer constants is wrong.

So no, this patch looks completely bogus to me.

Richard.
Sebastian Pop June 30, 2011, 2:34 p.m. UTC | #2
On Thu, Jun 30, 2011 at 05:19, Richard Guenther <rguenther@suse.de> wrote:
> That looks odd.  So you given -1U as input you sign-extend that to -1

correct

> and then set the mpz to -1ULLL.

and then it sets the mpz to signed -1, given that I'm passing false to UNS:

/* Sets RESULT to VAL, taken unsigned if UNS is true and as signed
   otherwise.  */

void
mpz_set_double_int (mpz_t result, double_int val, bool uns)

> In fact it looks like you either
> have non-canoncial INTEGER_CSTs from the start or the type of the
> integer constants is wrong.

I don't know what a canonical INTEGER_CST is.

Here are the cases that can occur:
- translating to mpz a -1U as the upper bound of an unsigned type,
in which case I do not want it to be sign extended,
- translating to mpz a -1U constant in an unsigned expression "x + -1U"
in which I do want it to be sign extended.

In my opinion there is not enough information to decide whether
to sign extend or not the value in tree_int_to_gmp.

Sebastian
Richard Biener June 30, 2011, 2:46 p.m. UTC | #3
On Thu, 30 Jun 2011, Sebastian Pop wrote:

> On Thu, Jun 30, 2011 at 05:19, Richard Guenther <rguenther@suse.de> wrote:
> > That looks odd.  So you given -1U as input you sign-extend that to -1
> 
> correct
> 
> > and then set the mpz to -1ULLL.
> 
> and then it sets the mpz to signed -1, given that I'm passing false to UNS:
>
> /* Sets RESULT to VAL, taken unsigned if UNS is true and as signed
>    otherwise.  */
> 
> void
> mpz_set_double_int (mpz_t result, double_int val, bool uns)
> 
> > In fact it looks like you either
> > have non-canoncial INTEGER_CSTs from the start or the type of the
> > integer constants is wrong.
> 
> I don't know what a canonical INTEGER_CST is.

Canonically extended according to TYPE_UNSIGNED I mean.  So what you
do is always create signed mpzs - that should simply work without
doing anything to the double-int.  Thus, why not do

static inline void
tree_int_to_gmp (tree t, mpz_t res)
{
  double_int di = tree_to_double_int (t);
  mpz_set_double_int (res, di, false);
}

?  I don't see why you'd want to sign-extend unsigned values
(for example an unsigned char 255 would get sign-extended to -1).
As double_ints are always correctly extended according to the
sign of the INTEGER_CST they come from (iff they are properly
canonical) you can embed all INTEGER_CSTs with precision of max.
HOST_BITS_PER_WISE_INT * 2 into a signed mpz with precision
HOST_BITS_PER_WISE_INT * 2 + 1.

> Here are the cases that can occur:
> - translating to mpz a -1U as the upper bound of an unsigned type,
> in which case I do not want it to be sign extended,
> - translating to mpz a -1U constant in an unsigned expression "x + -1U"
> in which I do want it to be sign extended.

I don't think you want the 2nd.  I think you want to preserve the
value.

Richard.
Sebastian Pop June 30, 2011, 2:57 p.m. UTC | #4
On Thu, Jun 30, 2011 at 09:46, Richard Guenther <rguenther@suse.de> wrote:
> On Thu, 30 Jun 2011, Sebastian Pop wrote:
>
>> On Thu, Jun 30, 2011 at 05:19, Richard Guenther <rguenther@suse.de> wrote:
>> > That looks odd.  So you given -1U as input you sign-extend that to -1
>>
>> correct
>>
>> > and then set the mpz to -1ULLL.
>>
>> and then it sets the mpz to signed -1, given that I'm passing false to UNS:
>>
>> /* Sets RESULT to VAL, taken unsigned if UNS is true and as signed
>>    otherwise.  */
>>
>> void
>> mpz_set_double_int (mpz_t result, double_int val, bool uns)
>>
>> > In fact it looks like you either
>> > have non-canoncial INTEGER_CSTs from the start or the type of the
>> > integer constants is wrong.
>>
>> I don't know what a canonical INTEGER_CST is.
>
> Canonically extended according to TYPE_UNSIGNED I mean.  So what you
> do is always create signed mpzs - that should simply work without
> doing anything to the double-int.  Thus, why not do
>
> static inline void
> tree_int_to_gmp (tree t, mpz_t res)
> {
>  double_int di = tree_to_double_int (t);
>  mpz_set_double_int (res, di, false);
> }
>
> ?  I don't see why you'd want to sign-extend unsigned values
> (for example an unsigned char 255 would get sign-extended to -1).
> As double_ints are always correctly extended according to the
> sign of the INTEGER_CST they come from (iff they are properly
> canonical) you can embed all INTEGER_CSTs with precision of max.
> HOST_BITS_PER_WISE_INT * 2 into a signed mpz with precision
> HOST_BITS_PER_WISE_INT * 2 + 1.
>
>> Here are the cases that can occur:
>> - translating to mpz a -1U as the upper bound of an unsigned type,
>> in which case I do not want it to be sign extended,
>> - translating to mpz a -1U constant in an unsigned expression "x + -1U"
>> in which I do want it to be sign extended.
>
> I don't think you want the 2nd.

Supposing that we have a loop iterating over unsigned char
for (unsigned char i = 100; i > 0; i--)
that would have a decrement "i = i + -1U", then the -1U would end up
translated as 255 in mpz, and that would be really interpreted as a stride
of 255 in the rest of graphite.

> I think you want to preserve the value.

Then we should also deal with the modulo arithmetic in graphite.

Sebastian
Sebastian Pop June 30, 2011, 3:02 p.m. UTC | #5
On Thu, Jun 30, 2011 at 09:57, Sebastian Pop <sebpop@gmail.com> wrote:
>> Canonically extended according to TYPE_UNSIGNED I mean.  So what you
>> do is always create signed mpzs - that should simply work without
>> doing anything to the double-int.  Thus, why not do
>>
>> static inline void
>> tree_int_to_gmp (tree t, mpz_t res)
>> {
>>  double_int di = tree_to_double_int (t);
>>  mpz_set_double_int (res, di, false);
>> }
>>
>> ?

I tried that and just passing false does not fix PR47653.
You still have the problem of the unsigned decrementing induction variable
that I described.

Sebastian
Richard Biener June 30, 2011, 3:03 p.m. UTC | #6
On Thu, 30 Jun 2011, Sebastian Pop wrote:

> On Thu, Jun 30, 2011 at 09:46, Richard Guenther <rguenther@suse.de> wrote:
> > On Thu, 30 Jun 2011, Sebastian Pop wrote:
> >
> >> On Thu, Jun 30, 2011 at 05:19, Richard Guenther <rguenther@suse.de> wrote:
> >> > That looks odd.  So you given -1U as input you sign-extend that to -1
> >>
> >> correct
> >>
> >> > and then set the mpz to -1ULLL.
> >>
> >> and then it sets the mpz to signed -1, given that I'm passing false to UNS:
> >>
> >> /* Sets RESULT to VAL, taken unsigned if UNS is true and as signed
> >>    otherwise.  */
> >>
> >> void
> >> mpz_set_double_int (mpz_t result, double_int val, bool uns)
> >>
> >> > In fact it looks like you either
> >> > have non-canoncial INTEGER_CSTs from the start or the type of the
> >> > integer constants is wrong.
> >>
> >> I don't know what a canonical INTEGER_CST is.
> >
> > Canonically extended according to TYPE_UNSIGNED I mean.  So what you
> > do is always create signed mpzs - that should simply work without
> > doing anything to the double-int.  Thus, why not do
> >
> > static inline void
> > tree_int_to_gmp (tree t, mpz_t res)
> > {
> >  double_int di = tree_to_double_int (t);
> >  mpz_set_double_int (res, di, false);
> > }
> >
> > ?  I don't see why you'd want to sign-extend unsigned values
> > (for example an unsigned char 255 would get sign-extended to -1).
> > As double_ints are always correctly extended according to the
> > sign of the INTEGER_CST they come from (iff they are properly
> > canonical) you can embed all INTEGER_CSTs with precision of max.
> > HOST_BITS_PER_WISE_INT * 2 into a signed mpz with precision
> > HOST_BITS_PER_WISE_INT * 2 + 1.
> >
> >> Here are the cases that can occur:
> >> - translating to mpz a -1U as the upper bound of an unsigned type,
> >> in which case I do not want it to be sign extended,
> >> - translating to mpz a -1U constant in an unsigned expression "x + -1U"
> >> in which I do want it to be sign extended.
> >
> > I don't think you want the 2nd.
> 
> Supposing that we have a loop iterating over unsigned char
> for (unsigned char i = 100; i > 0; i--)
> that would have a decrement "i = i + -1U", then the -1U would end up
> translated as 255 in mpz, and that would be really interpreted as a stride
> of 255 in the rest of graphite.
> 
> > I think you want to preserve the value.
> 
> Then we should also deal with the modulo arithmetic in graphite.

But what do you do for

 for (unsigned char i = 128; i < 255; ++i)

?  You change 128 to -128 which is wrong.  So yes, you also have to
deal with modulo arithmetic in graphite.

Richard.
Richard Biener June 30, 2011, 3:05 p.m. UTC | #7
On Thu, 30 Jun 2011, Sebastian Pop wrote:

> On Thu, Jun 30, 2011 at 09:57, Sebastian Pop <sebpop@gmail.com> wrote:
> >> Canonically extended according to TYPE_UNSIGNED I mean.  So what you
> >> do is always create signed mpzs - that should simply work without
> >> doing anything to the double-int.  Thus, why not do
> >>
> >> static inline void
> >> tree_int_to_gmp (tree t, mpz_t res)
> >> {
> >>  double_int di = tree_to_double_int (t);
> >>  mpz_set_double_int (res, di, false);
> >> }
> >>
> >> ?
> 
> I tried that and just passing false does not fix PR47653.
> You still have the problem of the unsigned decrementing induction variable
> that I described.

Well, you have to properly translate unsigned (modulo arithmetic)
IVs to signed mpz arithmetic.  Which isn't possible in all cases
(where modulo arithmetic matters).  I don't think that
tree_int_to_gmp is the place where this translation should happen.

Esp. because it then no longer is tree_int_to_gmp.

Richard.
Sebastian Pop June 30, 2011, 4:01 p.m. UTC | #8
On Thu, Jun 30, 2011 at 10:03, Richard Guenther <rguenther@suse.de> wrote:
> But what do you do for
>
>  for (unsigned char i = 128; i < 255; ++i)
>
> ?  You change 128 to -128 which is wrong.

Yes, 128 gets translated to -128.
And 255 gets translated to -1.
And so the loop iteration domain gets translated in the polyhedral form
as going from -128 to -1 with strides of 1.
So this particular program is not miscompiled by graphite:

int main ()
{
  unsigned char j;
  int x[300];
  for (j = 128; j < 255; j++)
    x[j] = j;

  for (j = 128; j < 255; j++)
    if (x[j] != j)
      __builtin_abort ();

  return 0;
}

I was trying to build a program that fails to attach to the PR.

> So yes, you also have to
> deal with modulo arithmetic in graphite.

I'm trying to understand where we have to deal with the modulo arithmetic.
Tobias, what are you doing in Polly?
Do you insert the loop iteration domain constraints with the modulo
of the types?

Sebastian
Tobias Grosser June 30, 2011, 6:18 p.m. UTC | #9
On 06/30/2011 11:01 AM, Sebastian Pop wrote:
> On Thu, Jun 30, 2011 at 10:03, Richard Guenther<rguenther@suse.de>  wrote:
>> But what do you do for
>>
>>   for (unsigned char i = 128; i<  255; ++i)
>>
>> ?  You change 128 to -128 which is wrong.
>
> Yes, 128 gets translated to -128.
> And 255 gets translated to -1.
> And so the loop iteration domain gets translated in the polyhedral form
> as going from -128 to -1 with strides of 1.
> So this particular program is not miscompiled by graphite:
>
> int main ()
> {
>    unsigned char j;
>    int x[300];
>    for (j = 128; j<  255; j++)
>      x[j] = j;
>
>    for (j = 128; j<  255; j++)
>      if (x[j] != j)
>        __builtin_abort ();
>
>    return 0;
> }
>
> I was trying to build a program that fails to attach to the PR.
>
>> So yes, you also have to
>> deal with modulo arithmetic in graphite.
>
> I'm trying to understand where we have to deal with the modulo arithmetic.
> Tobias, what are you doing in Polly?
> Do you insert the loop iteration domain constraints with the modulo
> of the types?

Hi Sebastian,

this is neither handed correctly in Polly.

Just to be able to understand Polly and Graphite:
LLVM has a different way of representing integer arithmetic. GCC uses 
explicit signed and unsigned types, whereas LLVM uses integer types that 
do not specify signedness, but on which either signed or unsigned 
operations are applied (For operations like add, subtract or multiply 
the signed and unsigned operations are actually the same). All 
operations in LLVM follow modulo arithmetics, except when they are 
marked 'no signed overflow' or 'no unsigned overflow'.

For Polly the correct solution is that as long as the operations are 
marked 'no signed overflow' or 'no unsigned overflow' we do not need 
modulo arithmetics. Modulo semantics are needed for operations that can 
have overflow and for most casts. At the moment we do not allow casts in 
Polly and we just assume that no overflow will occur. This is incorrect, 
but, as most C code works on signed integers which have undefined 
overflow semantics, it works most of the time. My next project is to add 
modulo arithmetics to Polly as soon as we cannot prove that no overflow 
will occur. This is the last major bug in Polly.

For Graphite I am not sure. Can overflows occur in the signed types of 
GCC or follow they the C signed types where overflows are undefined? In 
case overflow in signed types is undefined we should be safe as long as 
we disallow typecasts. As soon as unsigned types with defined overflow 
semantics occur, we most probably need modulo semantics to handle this 
correctly.

As a first step to make Graphite correct, I would just bail out if 
overflows can occur. Those games with sign extensions may work for the 
case where 255 is used as a -1, but in general I do not believe they are 
sufficient to express modulo semantics.

Adding real support for modulo semantics is probably the way to go, 
however it might be difficult. The first problem is that I do not know 
of a convenient way to express modulo semantics in PPL. isl provides so 
called existentially quantified dimensions, that allow us to easily 
express modulo semantics. In PPL we can express the same by adding 
additional dimensions. However, these additional dimensions are not 
automatically managed, such that we must ensure manually that they are 
handled correctly in the various polyhedral operations we perform.
Also even if we would be able to express modulo semantics, there is 
still the problem that we will step into untested terrain. Neither CLooG 
nor the other tools have been tested with this. I personally do not 
expect a lot of correctness problems, however due to the introduction of 
large integer constants and additional dimensions we may face compile 
time problems or the creation of non-optimal code. Research wise an 
interesting topic.

So as a first step. Would it be OK to limit Graphite to signed types for 
the moment and remove the sext hack?

Cheers
Tobi
diff mbox

Patch

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index e37d823..bed0070 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,19 @@ 
+2011-06-29  Sebastian Pop  <sebastian.pop@amd.com>
+
+	PR tree-optimization/47653
+	* graphite-ppl.h (tree_int_to_gmp): Sign extend before converting
+	constants to GMP values.  Add a sext parameter.
+	(ppl_set_inhomogeneous_tree): Add sext parameter.
+	(ppl_set_coef_tree): Removed.
+	* graphite-sese-to-poly.c (scan_tree_for_params_right_scev): Adjust
+	call to tree_int_to_gmp.
+	(scan_tree_for_params_int): Use tree_int_to_gmp.
+	(scan_tree_for_params): Adjust call to tree_int_to_gmp.
+	(build_loop_iteration_domains): Adjust call to
+	ppl_set_inhomogeneous_tree.
+	(add_param_constraints): Same.
+	(pdr_add_data_dimensions): Same.
+
 2011-06-29  Eric Botcazou  <ebotcazou@adacore.com>
 
 	PR tree-optimization/49539
diff --git a/gcc/graphite-ppl.h b/gcc/graphite-ppl.h
index 695d01f..4ae9f63 100644
--- a/gcc/graphite-ppl.h
+++ b/gcc/graphite-ppl.h
@@ -50,13 +50,18 @@  void debug_gmp_value (mpz_t);
 bool ppl_powerset_is_empty (ppl_Pointset_Powerset_C_Polyhedron_t);
 
 
-/* Assigns to RES the value of the INTEGER_CST T.  */
+/* Assigns to RES the value of the INTEGER_CST T.  When SEXT is true,
+   sign extend the value of T to not get "-1 = 2^n - 1".  */
 
 static inline void
-tree_int_to_gmp (tree t, mpz_t res)
+tree_int_to_gmp (tree t, mpz_t res, bool sext)
 {
+  tree type = TREE_TYPE (t);
   double_int di = tree_to_double_int (t);
-  mpz_set_double_int (res, di, TYPE_UNSIGNED (TREE_TYPE (t)));
+
+  if (sext)
+    di = double_int_sext (di, TYPE_PRECISION (type));
+  mpz_set_double_int (res, di, false);
 }
 
 /* Converts a GMP constant VAL to a tree and returns it.  */
@@ -88,14 +93,15 @@  ppl_set_inhomogeneous (ppl_Linear_Expression_t e, int x)
   mpz_clear (v);
 }
 
-/* Set the inhomogeneous term of E to the tree X.  */
+/* Set the inhomogeneous term of E to the tree X.  When SEXT is true,
+   sign extend the value of X.  */
 
 static inline void
-ppl_set_inhomogeneous_tree (ppl_Linear_Expression_t e, tree x)
+ppl_set_inhomogeneous_tree (ppl_Linear_Expression_t e, tree x, bool sext)
 {
   mpz_t v;
   mpz_init (v);
-  tree_int_to_gmp (x, v);
+  tree_int_to_gmp (x, v, sext);
   ppl_set_inhomogeneous_gmp (e, v);
   mpz_clear (v);
 }
@@ -112,18 +118,6 @@  ppl_set_coef (ppl_Linear_Expression_t e, ppl_dimension_type i, int x)
   mpz_clear (v);
 }
 
-/* Set E[I] to tree X.  */
-
-static inline void
-ppl_set_coef_tree (ppl_Linear_Expression_t e, ppl_dimension_type i, tree x)
-{
-  mpz_t v;
-  mpz_init (v);
-  tree_int_to_gmp (x, v);
-  ppl_set_coef_gmp (e, i, v);
-  mpz_clear (v);
-}
-
 /* Sets RES to the max of V1 and V2.  */
 
 static inline void
diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
index 7e23c9d..5f8188b 100644
--- a/gcc/graphite-sese-to-poly.c
+++ b/gcc/graphite-sese-to-poly.c
@@ -633,7 +633,7 @@  scan_tree_for_params_right_scev (sese s, tree e, int var,
       gcc_assert (TREE_CODE (e) == INTEGER_CST);
 
       mpz_init (val);
-      tree_int_to_gmp (e, val);
+      tree_int_to_gmp (e, val, true);
       add_value_to_dim (l, expr, val);
       mpz_clear (val);
     }
@@ -647,14 +647,9 @@  scan_tree_for_params_int (tree cst, ppl_Linear_Expression_t expr, mpz_t k)
 {
   mpz_t val;
   ppl_Coefficient_t coef;
-  tree type = TREE_TYPE (cst);
 
   mpz_init (val);
-
-  /* Necessary to not get "-1 = 2^n - 1". */
-  mpz_set_double_int (val, double_int_sext (tree_to_double_int (cst),
-					    TYPE_PRECISION (type)), false);
-
+  tree_int_to_gmp (cst, val, true);
   mpz_mul (val, val, k);
   ppl_new_Coefficient (&coef);
   ppl_assign_Coefficient_from_mpz_t (coef, val);
@@ -731,7 +726,7 @@  scan_tree_for_params (sese s, tree e, ppl_Linear_Expression_t c,
 	      mpz_t val;
 	      gcc_assert (host_integerp (TREE_OPERAND (e, 1), 0));
 	      mpz_init (val);
-	      tree_int_to_gmp (TREE_OPERAND (e, 1), val);
+	      tree_int_to_gmp (TREE_OPERAND (e, 1), val, true);
 	      mpz_mul (val, val, k);
 	      scan_tree_for_params (s, TREE_OPERAND (e, 0), c, val);
 	      mpz_clear (val);
@@ -746,7 +741,7 @@  scan_tree_for_params (sese s, tree e, ppl_Linear_Expression_t c,
 	      mpz_t val;
 	      gcc_assert (host_integerp (TREE_OPERAND (e, 0), 0));
 	      mpz_init (val);
-	      tree_int_to_gmp (TREE_OPERAND (e, 0), val);
+	      tree_int_to_gmp (TREE_OPERAND (e, 0), val, true);
 	      mpz_mul (val, val, k);
 	      scan_tree_for_params (s, TREE_OPERAND (e, 1), c, val);
 	      mpz_clear (val);
@@ -1072,7 +1067,9 @@  build_loop_iteration_domains (scop_p scop, struct loop *loop,
 
       /* loop_i <= cst_nb_iters */
       ppl_set_coef (ub_expr, nb, -1);
-      ppl_set_inhomogeneous_tree (ub_expr, nb_iters);
+      /* The number of iterations should not be sign extended as it is
+	 a positive number.  */
+      ppl_set_inhomogeneous_tree (ub_expr, nb_iters, false);
       ppl_new_Constraint (&ub, ub_expr, PPL_CONSTRAINT_TYPE_GREATER_OR_EQUAL);
       ppl_Polyhedron_add_constraint (ph, ub);
       ppl_delete_Linear_Expression (ub_expr);
@@ -1448,7 +1445,7 @@  add_param_constraints (scop_p scop, ppl_Polyhedron_t context, graphite_dim_t p)
     {
       ppl_new_Linear_Expression_with_dimension (&le, scop_nb_params (scop));
       ppl_set_coef (le, p, -1);
-      ppl_set_inhomogeneous_tree (le, lb);
+      ppl_set_inhomogeneous_tree (le, lb, true);
       ppl_new_Constraint (&cstr, le, PPL_CONSTRAINT_TYPE_LESS_OR_EQUAL);
       ppl_Polyhedron_add_constraint (context, cstr);
       ppl_delete_Linear_Expression (le);
@@ -1459,7 +1456,9 @@  add_param_constraints (scop_p scop, ppl_Polyhedron_t context, graphite_dim_t p)
     {
       ppl_new_Linear_Expression_with_dimension (&le, scop_nb_params (scop));
       ppl_set_coef (le, p, -1);
-      ppl_set_inhomogeneous_tree (le, ub);
+      /* Do not sign extend the upper bound of an unsigned type, as
+	 that would become -1.  */
+      ppl_set_inhomogeneous_tree (le, ub, !TYPE_UNSIGNED (type));
       ppl_new_Constraint (&cstr, le, PPL_CONSTRAINT_TYPE_GREATER_OR_EQUAL);
       ppl_Polyhedron_add_constraint (context, cstr);
       ppl_delete_Linear_Expression (le);
@@ -1641,7 +1640,7 @@  pdr_add_data_dimensions (ppl_Polyhedron_t accesses, data_reference_p dr,
 	  ppl_set_coef (expr, subscript, 1);
 
 	  minus_low = fold_build1 (NEGATE_EXPR, TREE_TYPE (low), low);
-	  ppl_set_inhomogeneous_tree (expr, minus_low);
+	  ppl_set_inhomogeneous_tree (expr, minus_low, true);
 
 	  ppl_new_Constraint (&cstr, expr, PPL_CONSTRAINT_TYPE_GREATER_OR_EQUAL);
 	  ppl_Polyhedron_add_constraint (accesses, cstr);
@@ -1661,7 +1660,7 @@  pdr_add_data_dimensions (ppl_Polyhedron_t accesses, data_reference_p dr,
 	  ppl_new_Linear_Expression_with_dimension (&expr, accessp_nb_dims);
 	  ppl_set_coef (expr, subscript, -1);
 
-	  ppl_set_inhomogeneous_tree (expr, high);
+	  ppl_set_inhomogeneous_tree (expr, high, true);
 
 	  ppl_new_Constraint (&cstr, expr, PPL_CONSTRAINT_TYPE_GREATER_OR_EQUAL);
 	  ppl_Polyhedron_add_constraint (accesses, cstr);
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index d30d8b0..6d48df4 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@ 
+2011-06-29  Sebastian Pop  <sebastian.pop@amd.com>
+
+	PR tree-optimization/47653
+	* gcc.dg/graphite/run-id-pr47653.c: New.
+
 2011-06-29  Jason Merrill  <jason@redhat.com>
 
 	PR c++/45923
diff --git a/gcc/testsuite/gcc.dg/graphite/run-id-pr47653.c b/gcc/testsuite/gcc.dg/graphite/run-id-pr47653.c
new file mode 100644
index 0000000..b62b891
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/graphite/run-id-pr47653.c
@@ -0,0 +1,17 @@ 
+/* { dg-options "-O -fstack-check=generic -ftree-pre -fgraphite-identity" } */
+
+int main ()
+{
+  int i, j;
+  int x[8][8];
+  for (i = 0; i < 8; i++)
+    for (j = i; j < 8; j++)
+      x[i][j] = 4;
+
+  for (i = 0; i < 8; i++)
+    for (j = i; j < 8; j++)
+      if (x[i][j] != 4)
+	__builtin_abort ();
+
+  return 0;
+}