[PATCH] Sign extend before converting constants to GMP values.

[Richard Guenther]

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.