public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [Graphite] Fix type problems in loop ivs.
@ 2010-03-05 14:22 Tobias Grosser
  2010-03-05 14:39 ` Richard Guenther
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Tobias Grosser @ 2010-03-05 14:22 UTC (permalink / raw)
  To: gcc-patches, gcc-graphite; +Cc: Tobias Grosser

Hi,

I would like to commit this patch that fixes the loop iv type problems in
graphite.

This resolves two P1 bug reports without any regressions in the graphite.exp
testsuite neither on 64 nor on 32 bit linux. Only two test cases in libgomp
changed, as autopar generates for these files an iv where we cannot yet
prove, that it can be handled safely in graphite. This can be enhanced later on.

The patch was bootstrapped on
Linux tobias 2.6.33 #4 SMP PREEMPT Mon Mar 1 12:59:44 CET 2010 x86_64 Intel(R)
Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel GNU/Linux

Linux tobias 2.6.33 #4 SMP PREEMPT Mon Mar 1 12:59:44 CET 2010 i686 Intel(R)
Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel GNU/Linux

Also tested dealII on 64bit with -O2, which failed before.

OK for trunk, if it passes the automatic spec 2006 tests on graphite branch?

Tobias

PS: Thanks Richi, for proposing "fold_convert (sizetype, name)" for the conversion
from pointers.

Fix pr42644.
Fix pr42130 (dealII).

2010-03-03  Tobias Grosser  <grosser@fim.uni-passau.de>

	* gcc/graphite-clast-to-gimple.c (clast_to_gcc_expression): Also
	handle conversions from pointer to integers.
	(gcc_type_for_cloog_iv): Choose the smalles signed integer as an
	induction variable, to be able to work with code generated by
	CLooG.
	* gcc/graphite-sese-to-poly.c (scop_ivs_can_be_represented): New.
	(build_poly_scop): Bail out if we cannot codegen a loop.
	* gcc/testsuite/gcc.dg/graphite/id-18.c: New.
	* gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c: New.
	* libgomp/testsuite/libgomp.graphite/force-parallel-1.c: Adjust.
	* libgomp/testsuite/libgomp.graphite/force-parallel-2.c: Adjust.
---
 gcc/graphite-clast-to-gimple.c                     |   67 +++++++++++++++++--
 gcc/graphite-sese-to-poly.c                        |   36 +++++++++++
 gcc/testsuite/gcc.dg/graphite/id-18.c              |    7 ++
 gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c     |   32 +++++++++
 .../testsuite/libgomp.graphite/force-parallel-1.c  |    2 +-
 .../testsuite/libgomp.graphite/force-parallel-2.c  |    2 +-
 6 files changed, 137 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/graphite/id-18.c
 create mode 100644 gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c

diff --git a/gcc/graphite-clast-to-gimple.c b/gcc/graphite-clast-to-gimple.c
index 909392a..6ce01d5 100644
--- a/gcc/graphite-clast-to-gimple.c
+++ b/gcc/graphite-clast-to-gimple.c
@@ -282,14 +282,24 @@ clast_to_gcc_expression (tree type, struct clast_expr *e,
 	      {
 		tree name = clast_name_to_gcc (t->var, region, newivs,
 					       newivs_index, params_index);
-		return fold_convert (type, name);
+
+		if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
+		  name = fold_convert (sizetype, name);
+
+		name = fold_convert (type, name);
+		return name;
 	      }
 
 	    else if (value_mone_p (t->val))
 	      {
 		tree name = clast_name_to_gcc (t->var, region, newivs,
 					       newivs_index, params_index);
+
+		if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
+		  name = fold_convert (sizetype, name);
+
 		name = fold_convert (type, name);
+
 		return fold_build1 (NEGATE_EXPR, type, name);
 	      }
 	    else
@@ -297,7 +307,12 @@ clast_to_gcc_expression (tree type, struct clast_expr *e,
 		tree name = clast_name_to_gcc (t->var, region, newivs,
 					       newivs_index, params_index);
 		tree cst = gmp_cst_to_tree (type, t->val);
+
+		if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
+		  name = fold_convert (sizetype, name);
+
 		name = fold_convert (type, name);
+
 		if (!POINTER_TYPE_P (type))
 		  return fold_build2 (MULT_EXPR, type, cst, name);
 
@@ -532,9 +547,16 @@ clast_get_body_of_loop (struct clast_stmt *stmt)
   gcc_unreachable ();
 }
 
-/* Given a CLOOG_IV, returns the type that it should have in GCC land.
-   If the information is not available, i.e. in the case one of the
-   transforms created the loop, just return integer_type_node.  */
+/* Given a CLOOG_IV, return the type that CLOOG_IV should have in GCC
+   land.  The selected type is big enough to include the original loop
+   iteration variable, but signed to work with the subtractions CLooG
+   may have introduced.  If such a type is not available, we fail.
+
+   TODO: Do not always return long_long, but the smallest possible
+   type, that still holds the original type.
+
+   TODO: Get the types using CLooG instead.  This enables further
+   optimizations, but needs CLooG support.  */
 
 static tree
 gcc_type_for_cloog_iv (const char *cloog_iv, gimple_bb_p gbb)
@@ -546,9 +568,40 @@ gcc_type_for_cloog_iv (const char *cloog_iv, gimple_bb_p gbb)
   slot = htab_find_slot (GBB_CLOOG_IV_TYPES (gbb), &tmp, NO_INSERT);
 
   if (slot && *slot)
-    return ((ivtype_map_elt) *slot)->type;
+    {
+      tree type = ((ivtype_map_elt) *slot)->type;
+      int type_precision = TYPE_PRECISION (type);
+
+      /* Find the smallest signed type possible.  */
+      if (!TYPE_UNSIGNED (type))
+	{
+	  if (type_precision <= TYPE_PRECISION (integer_type_node))
+	    return integer_type_node;
+
+	  if (type_precision <= TYPE_PRECISION (long_integer_type_node))
+	    return long_integer_type_node;
+
+	  if (type_precision <= TYPE_PRECISION (long_long_integer_type_node))
+	    return long_long_integer_type_node;
+
+	  gcc_unreachable ();
+	}
+
+      if (type_precision < TYPE_PRECISION (integer_type_node))
+	return integer_type_node;
+
+      if (type_precision < TYPE_PRECISION (long_integer_type_node))
+	return long_integer_type_node;
+
+      if (type_precision < TYPE_PRECISION (long_long_integer_type_node))
+	return long_long_integer_type_node;
+
+      /* There is no signed type available, that is large enough to hold the
+	 original value.  */
+      gcc_unreachable ();
+    }
 
-  return integer_type_node;
+  return long_long_integer_type_node;
 }
 
 /* Returns the induction variable for the loop that gets translated to
@@ -1046,7 +1099,7 @@ compute_cloog_iv_types_1 (poly_bb_p pbb, struct clast_user_stmt *user_stmt)
       if (slot && !*slot)
 	{
 	  tree oldiv = pbb_to_depth_to_oldiv (pbb, index);
-	  tree type = oldiv ? TREE_TYPE (oldiv) : integer_type_node;
+	  tree type = TREE_TYPE (oldiv);
 	  *slot = new_ivtype_map_elt (tmp.cloog_iv, type);
 	}
     }
diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
index 3ee431f..279a905 100644
--- a/gcc/graphite-sese-to-poly.c
+++ b/gcc/graphite-sese-to-poly.c
@@ -2893,6 +2893,38 @@ scop_canonicalize_loops (scop_p scop)
       graphite_loop_normal_form (loop);
 }
 
+/* Can all ivs be represented by a signed integer?
+   As CLooG might generate negative values in its expressions, signed loop ivs
+   are required in the backend. */
+static bool
+scop_ivs_can_be_represented (scop_p scop)
+{
+  loop_iterator li;
+  loop_p loop;
+
+  FOR_EACH_LOOP (li, loop, 0)
+    {
+      tree type;
+      int precision;
+
+      if (!loop_in_sese_p (loop, SCOP_REGION (scop)))
+	continue;
+
+      if (!loop->single_iv)
+	continue;
+
+      type = TREE_TYPE(loop->single_iv);
+      precision = TYPE_PRECISION (type);
+
+      if (TYPE_UNSIGNED (type)
+	  && precision >= TYPE_PRECISION (long_long_integer_type_node))
+	return false;
+    }
+
+  return true;
+}
+
+
 /* Builds the polyhedral representation for a SESE region.  */
 
 bool
@@ -2915,6 +2947,10 @@ build_poly_scop (scop_p scop)
     return false;
 
   scop_canonicalize_loops (scop);
+
+  if (!scop_ivs_can_be_represented (scop))
+    return false;
+
   build_sese_loop_nests (region);
   build_sese_conditions (region);
   find_scop_parameters (scop);
diff --git a/gcc/testsuite/gcc.dg/graphite/id-18.c b/gcc/testsuite/gcc.dg/graphite/id-18.c
new file mode 100644
index 0000000..77628fa
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/graphite/id-18.c
@@ -0,0 +1,7 @@
+long do_hash (const char * lo, const char * hi)
+{
+	int val = 0;
+	for (; lo < hi; ++lo)
+		val = *lo;
+	return val;
+}
diff --git a/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c b/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c
new file mode 100644
index 0000000..2e90064
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c
@@ -0,0 +1,32 @@
+/* Testcase extracted from test 183.equake in SPEC CPU2000.  */
+double Ke[2], ds[2];
+
+void foo(double Ke[2], int i, double ds[],  int column)
+{
+  double tt, ts;
+  int j;
+
+  for (j = 0; j < 2; j++)
+    {
+      ++column;
+      ts = ds[i];
+      if (i == j)
+	tt = 123;
+      else
+	tt = 0;
+      Ke[column] = Ke[column] + ts + tt;
+    }
+}
+
+int
+main ()
+{
+  int i, j;
+
+  ds[0] = 1.0;
+  ds[1] = 1.0;
+
+  foo(Ke, 0, ds, -1);
+
+  return (int) Ke[0] != 124;
+}
diff --git a/libgomp/testsuite/libgomp.graphite/force-parallel-1.c b/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
index 7f043d8..1ad4b41 100644
--- a/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
+++ b/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
@@ -23,7 +23,7 @@ int main(void)
 }
 
 /* Check that parallel code generation part make the right answer.  */
-/* { dg-final { scan-tree-dump-times "1 loops carried no dependency" 2 "graphite" } } */
+/* { dg-final { scan-tree-dump-times "1 loops carried no dependency" 1 "graphite" } } */
 /* { dg-final { cleanup-tree-dump "graphite" } } */
 /* { dg-final { scan-tree-dump-times "loopfn" 5 "optimized" } } */
 /* { dg-final { cleanup-tree-dump "parloops" } } */
diff --git a/libgomp/testsuite/libgomp.graphite/force-parallel-2.c b/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
index 03d8236..1ce0feb 100644
--- a/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
+++ b/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
@@ -23,7 +23,7 @@ int main(void)
 }
 
 /* Check that parallel code generation part make the right answer.  */
-/* { dg-final { scan-tree-dump-times "2 loops carried no dependency" 2 "graphite" } } */
+/* { dg-final { scan-tree-dump-times "2 loops carried no dependency" 1 "graphite" } } */
 /* { dg-final { cleanup-tree-dump "graphite" } } */
 /* { dg-final { scan-tree-dump-times "loopfn" 5 "optimized" } } */
 /* { dg-final { cleanup-tree-dump "parloops" } } */
-- 
1.6.4.4

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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-03-05 14:22 [Graphite] Fix type problems in loop ivs Tobias Grosser
@ 2010-03-05 14:39 ` Richard Guenther
  2010-03-05 20:49   ` Sebastian Pop
  2010-03-05 14:40 ` Eric Botcazou
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Richard Guenther @ 2010-03-05 14:39 UTC (permalink / raw)
  To: Tobias Grosser; +Cc: gcc-patches, gcc-graphite

On Fri, Mar 5, 2010 at 3:14 PM, Tobias Grosser
<grosser@fim.uni-passau.de> wrote:
> Hi,
>
> I would like to commit this patch that fixes the loop iv type problems in
> graphite.
>
> This resolves two P1 bug reports without any regressions in the graphite.exp
> testsuite neither on 64 nor on 32 bit linux. Only two test cases in libgomp
> changed, as autopar generates for these files an iv where we cannot yet
> prove, that it can be handled safely in graphite. This can be enhanced later on.
>
> The patch was bootstrapped on
> Linux tobias 2.6.33 #4 SMP PREEMPT Mon Mar 1 12:59:44 CET 2010 x86_64 Intel(R)
> Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel GNU/Linux
>
> Linux tobias 2.6.33 #4 SMP PREEMPT Mon Mar 1 12:59:44 CET 2010 i686 Intel(R)
> Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel GNU/Linux
>
> Also tested dealII on 64bit with -O2, which failed before.
>
> OK for trunk, if it passes the automatic spec 2006 tests on graphite branch?

This variant looks reasonable.  It's still ugly, but well - I hope
somebody is working on enhancing CLooG for a nicer fix.

The patch is ok if it passes testing.

Thanks,
Richard.

> Tobias
>
> PS: Thanks Richi, for proposing "fold_convert (sizetype, name)" for the conversion
> from pointers.
>
> Fix pr42644.
> Fix pr42130 (dealII).
>
> 2010-03-03  Tobias Grosser  <grosser@fim.uni-passau.de>
>
>        * gcc/graphite-clast-to-gimple.c (clast_to_gcc_expression): Also
>        handle conversions from pointer to integers.
>        (gcc_type_for_cloog_iv): Choose the smalles signed integer as an
>        induction variable, to be able to work with code generated by
>        CLooG.
>        * gcc/graphite-sese-to-poly.c (scop_ivs_can_be_represented): New.
>        (build_poly_scop): Bail out if we cannot codegen a loop.
>        * gcc/testsuite/gcc.dg/graphite/id-18.c: New.
>        * gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c: New.
>        * libgomp/testsuite/libgomp.graphite/force-parallel-1.c: Adjust.
>        * libgomp/testsuite/libgomp.graphite/force-parallel-2.c: Adjust.
> ---
>  gcc/graphite-clast-to-gimple.c                     |   67 +++++++++++++++++--
>  gcc/graphite-sese-to-poly.c                        |   36 +++++++++++
>  gcc/testsuite/gcc.dg/graphite/id-18.c              |    7 ++
>  gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c     |   32 +++++++++
>  .../testsuite/libgomp.graphite/force-parallel-1.c  |    2 +-
>  .../testsuite/libgomp.graphite/force-parallel-2.c  |    2 +-
>  6 files changed, 137 insertions(+), 9 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/graphite/id-18.c
>  create mode 100644 gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c
>
> diff --git a/gcc/graphite-clast-to-gimple.c b/gcc/graphite-clast-to-gimple.c
> index 909392a..6ce01d5 100644
> --- a/gcc/graphite-clast-to-gimple.c
> +++ b/gcc/graphite-clast-to-gimple.c
> @@ -282,14 +282,24 @@ clast_to_gcc_expression (tree type, struct clast_expr *e,
>              {
>                tree name = clast_name_to_gcc (t->var, region, newivs,
>                                               newivs_index, params_index);
> -               return fold_convert (type, name);
> +
> +               if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
> +                 name = fold_convert (sizetype, name);
> +
> +               name = fold_convert (type, name);
> +               return name;
>              }
>
>            else if (value_mone_p (t->val))
>              {
>                tree name = clast_name_to_gcc (t->var, region, newivs,
>                                               newivs_index, params_index);
> +
> +               if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
> +                 name = fold_convert (sizetype, name);
> +
>                name = fold_convert (type, name);
> +
>                return fold_build1 (NEGATE_EXPR, type, name);
>              }
>            else
> @@ -297,7 +307,12 @@ clast_to_gcc_expression (tree type, struct clast_expr *e,
>                tree name = clast_name_to_gcc (t->var, region, newivs,
>                                               newivs_index, params_index);
>                tree cst = gmp_cst_to_tree (type, t->val);
> +
> +               if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
> +                 name = fold_convert (sizetype, name);
> +
>                name = fold_convert (type, name);
> +
>                if (!POINTER_TYPE_P (type))
>                  return fold_build2 (MULT_EXPR, type, cst, name);
>
> @@ -532,9 +547,16 @@ clast_get_body_of_loop (struct clast_stmt *stmt)
>   gcc_unreachable ();
>  }
>
> -/* Given a CLOOG_IV, returns the type that it should have in GCC land.
> -   If the information is not available, i.e. in the case one of the
> -   transforms created the loop, just return integer_type_node.  */
> +/* Given a CLOOG_IV, return the type that CLOOG_IV should have in GCC
> +   land.  The selected type is big enough to include the original loop
> +   iteration variable, but signed to work with the subtractions CLooG
> +   may have introduced.  If such a type is not available, we fail.
> +
> +   TODO: Do not always return long_long, but the smallest possible
> +   type, that still holds the original type.
> +
> +   TODO: Get the types using CLooG instead.  This enables further
> +   optimizations, but needs CLooG support.  */
>
>  static tree
>  gcc_type_for_cloog_iv (const char *cloog_iv, gimple_bb_p gbb)
> @@ -546,9 +568,40 @@ gcc_type_for_cloog_iv (const char *cloog_iv, gimple_bb_p gbb)
>   slot = htab_find_slot (GBB_CLOOG_IV_TYPES (gbb), &tmp, NO_INSERT);
>
>   if (slot && *slot)
> -    return ((ivtype_map_elt) *slot)->type;
> +    {
> +      tree type = ((ivtype_map_elt) *slot)->type;
> +      int type_precision = TYPE_PRECISION (type);
> +
> +      /* Find the smallest signed type possible.  */
> +      if (!TYPE_UNSIGNED (type))
> +       {
> +         if (type_precision <= TYPE_PRECISION (integer_type_node))
> +           return integer_type_node;
> +
> +         if (type_precision <= TYPE_PRECISION (long_integer_type_node))
> +           return long_integer_type_node;
> +
> +         if (type_precision <= TYPE_PRECISION (long_long_integer_type_node))
> +           return long_long_integer_type_node;
> +
> +         gcc_unreachable ();
> +       }
> +
> +      if (type_precision < TYPE_PRECISION (integer_type_node))
> +       return integer_type_node;
> +
> +      if (type_precision < TYPE_PRECISION (long_integer_type_node))
> +       return long_integer_type_node;
> +
> +      if (type_precision < TYPE_PRECISION (long_long_integer_type_node))
> +       return long_long_integer_type_node;
> +
> +      /* There is no signed type available, that is large enough to hold the
> +        original value.  */
> +      gcc_unreachable ();
> +    }
>
> -  return integer_type_node;
> +  return long_long_integer_type_node;
>  }
>
>  /* Returns the induction variable for the loop that gets translated to
> @@ -1046,7 +1099,7 @@ compute_cloog_iv_types_1 (poly_bb_p pbb, struct clast_user_stmt *user_stmt)
>       if (slot && !*slot)
>        {
>          tree oldiv = pbb_to_depth_to_oldiv (pbb, index);
> -         tree type = oldiv ? TREE_TYPE (oldiv) : integer_type_node;
> +         tree type = TREE_TYPE (oldiv);
>          *slot = new_ivtype_map_elt (tmp.cloog_iv, type);
>        }
>     }
> diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
> index 3ee431f..279a905 100644
> --- a/gcc/graphite-sese-to-poly.c
> +++ b/gcc/graphite-sese-to-poly.c
> @@ -2893,6 +2893,38 @@ scop_canonicalize_loops (scop_p scop)
>       graphite_loop_normal_form (loop);
>  }
>
> +/* Can all ivs be represented by a signed integer?
> +   As CLooG might generate negative values in its expressions, signed loop ivs
> +   are required in the backend. */
> +static bool
> +scop_ivs_can_be_represented (scop_p scop)
> +{
> +  loop_iterator li;
> +  loop_p loop;
> +
> +  FOR_EACH_LOOP (li, loop, 0)
> +    {
> +      tree type;
> +      int precision;
> +
> +      if (!loop_in_sese_p (loop, SCOP_REGION (scop)))
> +       continue;
> +
> +      if (!loop->single_iv)
> +       continue;
> +
> +      type = TREE_TYPE(loop->single_iv);
> +      precision = TYPE_PRECISION (type);
> +
> +      if (TYPE_UNSIGNED (type)
> +         && precision >= TYPE_PRECISION (long_long_integer_type_node))
> +       return false;
> +    }
> +
> +  return true;
> +}
> +
> +
>  /* Builds the polyhedral representation for a SESE region.  */
>
>  bool
> @@ -2915,6 +2947,10 @@ build_poly_scop (scop_p scop)
>     return false;
>
>   scop_canonicalize_loops (scop);
> +
> +  if (!scop_ivs_can_be_represented (scop))
> +    return false;
> +
>   build_sese_loop_nests (region);
>   build_sese_conditions (region);
>   find_scop_parameters (scop);
> diff --git a/gcc/testsuite/gcc.dg/graphite/id-18.c b/gcc/testsuite/gcc.dg/graphite/id-18.c
> new file mode 100644
> index 0000000..77628fa
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/graphite/id-18.c
> @@ -0,0 +1,7 @@
> +long do_hash (const char * lo, const char * hi)
> +{
> +       int val = 0;
> +       for (; lo < hi; ++lo)
> +               val = *lo;
> +       return val;
> +}
> diff --git a/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c b/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c
> new file mode 100644
> index 0000000..2e90064
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c
> @@ -0,0 +1,32 @@
> +/* Testcase extracted from test 183.equake in SPEC CPU2000.  */
> +double Ke[2], ds[2];
> +
> +void foo(double Ke[2], int i, double ds[],  int column)
> +{
> +  double tt, ts;
> +  int j;
> +
> +  for (j = 0; j < 2; j++)
> +    {
> +      ++column;
> +      ts = ds[i];
> +      if (i == j)
> +       tt = 123;
> +      else
> +       tt = 0;
> +      Ke[column] = Ke[column] + ts + tt;
> +    }
> +}
> +
> +int
> +main ()
> +{
> +  int i, j;
> +
> +  ds[0] = 1.0;
> +  ds[1] = 1.0;
> +
> +  foo(Ke, 0, ds, -1);
> +
> +  return (int) Ke[0] != 124;
> +}
> diff --git a/libgomp/testsuite/libgomp.graphite/force-parallel-1.c b/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
> index 7f043d8..1ad4b41 100644
> --- a/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
> +++ b/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
> @@ -23,7 +23,7 @@ int main(void)
>  }
>
>  /* Check that parallel code generation part make the right answer.  */
> -/* { dg-final { scan-tree-dump-times "1 loops carried no dependency" 2 "graphite" } } */
> +/* { dg-final { scan-tree-dump-times "1 loops carried no dependency" 1 "graphite" } } */
>  /* { dg-final { cleanup-tree-dump "graphite" } } */
>  /* { dg-final { scan-tree-dump-times "loopfn" 5 "optimized" } } */
>  /* { dg-final { cleanup-tree-dump "parloops" } } */
> diff --git a/libgomp/testsuite/libgomp.graphite/force-parallel-2.c b/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
> index 03d8236..1ce0feb 100644
> --- a/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
> +++ b/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
> @@ -23,7 +23,7 @@ int main(void)
>  }
>
>  /* Check that parallel code generation part make the right answer.  */
> -/* { dg-final { scan-tree-dump-times "2 loops carried no dependency" 2 "graphite" } } */
> +/* { dg-final { scan-tree-dump-times "2 loops carried no dependency" 1 "graphite" } } */
>  /* { dg-final { cleanup-tree-dump "graphite" } } */
>  /* { dg-final { scan-tree-dump-times "loopfn" 5 "optimized" } } */
>  /* { dg-final { cleanup-tree-dump "parloops" } } */
> --
> 1.6.4.4
>
>

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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-03-05 14:22 [Graphite] Fix type problems in loop ivs Tobias Grosser
  2010-03-05 14:39 ` Richard Guenther
@ 2010-03-05 14:40 ` Eric Botcazou
       [not found] ` <11654_1267799959_4B911797_11654_9062_1_201003051538.47829.ebotcazou@adacore.com>
  2010-03-10 21:23 ` Sebastian Pop
  3 siblings, 0 replies; 14+ messages in thread
From: Eric Botcazou @ 2010-03-05 14:40 UTC (permalink / raw)
  To: Tobias Grosser; +Cc: gcc-patches, gcc-graphite

> 2010-03-03  Tobias Grosser  <grosser@fim.uni-passau.de>
>
> 	* gcc/graphite-clast-to-gimple.c (clast_to_gcc_expression): Also
> 	handle conversions from pointer to integers.
> 	(gcc_type_for_cloog_iv): Choose the smalles signed integer as an
> 	induction variable, to be able to work with code generated by
> 	CLooG.
> 	* gcc/graphite-sese-to-poly.c (scop_ivs_can_be_represented): New.
> 	(build_poly_scop): Bail out if we cannot codegen a loop.
> 	* gcc/testsuite/gcc.dg/graphite/id-18.c: New.
> 	* gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c: New.
> 	* libgomp/testsuite/libgomp.graphite/force-parallel-1.c: Adjust.
> 	* libgomp/testsuite/libgomp.graphite/force-parallel-2.c: Adjust.

Paths in ChangeLog entries must be relative to the ChangeLog file:


2010-03-05  Tobias Grosser  <grosser@fim.uni-passau.de>

        * graphite-clast-to-gimple.c (clast_to_gcc_expression): Also
        handle conversions from pointer to integers.
        (gcc_type_for_cloog_iv): Choose the smalles signed integer as an
        induction variable, to be able to work with code generated by
        CLooG.
        * graphite-sese-to-poly.c (scop_ivs_can_be_represented): New.
        (build_poly_scop): Bail out if we cannot codegen a loop.


2010-03-05  Tobias Grosser  <grosser@fim.uni-passau.de>

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


2010-03-05  Tobias Grosser  <grosser@fim.uni-passau.de>

        * testsuite/libgomp.graphite/force-parallel-1.c: Adjust.
        * testsuite/libgomp.graphite/force-parallel-2.c: Adjust.


-- 
Eric Botcazou

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

* Re: [Graphite] Fix type problems in loop ivs.
       [not found] ` <11654_1267799959_4B911797_11654_9062_1_201003051538.47829.ebotcazou@adacore.com>
@ 2010-03-05 15:38   ` Tobias Grosser
  0 siblings, 0 replies; 14+ messages in thread
From: Tobias Grosser @ 2010-03-05 15:38 UTC (permalink / raw)
  To: Eric Botcazou; +Cc: gcc-patches, gcc-graphite

On 03/05/2010 03:38 PM, Eric Botcazou wrote:
>> 2010-03-03  Tobias Grosser<grosser@fim.uni-passau.de>
>>
>> 	* gcc/graphite-clast-to-gimple.c (clast_to_gcc_expression): Also
>> 	handle conversions from pointer to integers.
>> 	(gcc_type_for_cloog_iv): Choose the smalles signed integer as an
>> 	induction variable, to be able to work with code generated by
>> 	CLooG.
>> 	* gcc/graphite-sese-to-poly.c (scop_ivs_can_be_represented): New.
>> 	(build_poly_scop): Bail out if we cannot codegen a loop.
>> 	* gcc/testsuite/gcc.dg/graphite/id-18.c: New.
>> 	* gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c: New.
>> 	* libgomp/testsuite/libgomp.graphite/force-parallel-1.c: Adjust.
>> 	* libgomp/testsuite/libgomp.graphite/force-parallel-2.c: Adjust.
>
> Paths in ChangeLog entries must be relative to the ChangeLog file:

Hi Eric,

you are right. I just forgot. I will correct it before merging to trunk.

Thanks

Tobias

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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-03-05 14:39 ` Richard Guenther
@ 2010-03-05 20:49   ` Sebastian Pop
  0 siblings, 0 replies; 14+ messages in thread
From: Sebastian Pop @ 2010-03-05 20:49 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Tobias Grosser, gcc-patches, gcc-graphite

[-- Attachment #1: Type: text/plain, Size: 1553 bytes --]

On Fri, Mar 5, 2010 at 08:22, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Fri, Mar 5, 2010 at 3:14 PM, Tobias Grosser
> <grosser@fim.uni-passau.de> wrote:
>> Hi,
>>
>> I would like to commit this patch that fixes the loop iv type problems in
>> graphite.
>>
>> This resolves two P1 bug reports without any regressions in the graphite.exp
>> testsuite neither on 64 nor on 32 bit linux. Only two test cases in libgomp
>> changed, as autopar generates for these files an iv where we cannot yet
>> prove, that it can be handled safely in graphite. This can be enhanced later on.
>>
>> The patch was bootstrapped on
>> Linux tobias 2.6.33 #4 SMP PREEMPT Mon Mar 1 12:59:44 CET 2010 x86_64 Intel(R)
>> Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel GNU/Linux
>>
>> Linux tobias 2.6.33 #4 SMP PREEMPT Mon Mar 1 12:59:44 CET 2010 i686 Intel(R)
>> Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel GNU/Linux
>>
>> Also tested dealII on 64bit with -O2, which failed before.
>>
>> OK for trunk, if it passes the automatic spec 2006 tests on graphite branch?
>

Thanks Tobias for having fixed this.

> This variant looks reasonable.  It's still ugly, but well - I hope
> somebody is working on enhancing CLooG for a nicer fix.
>

We do not need CLooG support for this: I have a preliminary patch for
computing the type based on the min and max values of the transformed
iteration domain.  I did not had yet time to fix all the testcases
that started to fail with this patch, but I may finish this next week.

Sebastian

[-- Attachment #2: 0001-Compute-min-and-max-bounds-for-IVs-and-infer-types.patch --]
[-- Type: text/x-diff, Size: 4071 bytes --]

From 485ffd5d06037a455ffbe0ca3eaf90766eb436ca Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Tue, 2 Mar 2010 16:36:14 -0600
Subject: [PATCH] Compute min and max bounds for IVs and infer types.

---
 gcc/graphite-clast-to-gimple.c |   94 +++++++++++++++++++++++++++++++++++++---
 1 files changed, 87 insertions(+), 7 deletions(-)

diff --git a/gcc/graphite-clast-to-gimple.c b/gcc/graphite-clast-to-gimple.c
index 909392a..b1194f3 100644
--- a/gcc/graphite-clast-to-gimple.c
+++ b/gcc/graphite-clast-to-gimple.c
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "value-prof.h"
 #include "pointer-set.h"
 #include "gimple.h"
+#include "langhooks.h"
 #include "sese.h"
 
 #ifdef HAVE_cloog
@@ -1018,6 +1019,88 @@ find_cloog_iv_in_expr (struct clast_expr *expr)
   return NULL;
 }
 
+/* Compute the lower bound LOW and upper bound UP for the induction
+   variable at LEVEL for the statement PBB, based on the transformed
+   scattering of PBB: T|I|G|Cst, with T the scattering transform, I
+   the iteration domain, and G the context parameters.  */
+
+static void
+compute_bounds_for_level (poly_bb_p pbb, int level, Value low, Value up)
+{
+  ppl_Pointset_Powerset_C_Polyhedron_t ps;
+  ppl_Linear_Expression_t le;
+
+  combine_context_id_scat (&ps, pbb, false);
+
+  /* Prepare the linear expression corresponding to the level that we
+     want to maximize/minimize.  */
+  {
+    ppl_dimension_type dim = pbb_nb_scattering_transform (pbb)
+      + pbb_dim_iter_domain (pbb) + pbb_nb_params (pbb);
+
+    ppl_new_Linear_Expression_with_dimension (&le, dim);
+    ppl_set_coef (le, 2 * level + 1, 1);
+  }
+
+  ppl_max_for_le_pointset (ps, le, up);
+  ppl_min_for_le_pointset (ps, le, low);
+}
+
+/* Compute the type for the induction variable at LEVEL for the
+   statement PBB, based on the transformed schedule of PBB.  */
+
+static tree
+compute_type_for_level (poly_bb_p pbb, int level)
+{
+  tree oldiv = pbb_to_depth_to_oldiv (pbb, level);
+  Value low, up, diff, x, two;
+  bool unsigned_p = true;
+  int precision;
+  tree type = TREE_TYPE (oldiv);
+
+  if (type && POINTER_TYPE_P (type))
+    /* FIXME: In the case of a pointer type, check as well that the
+       low and up bounds fit to the oldiv pointer type.  */
+    return type;
+
+  value_init (low);
+  value_init (up);
+
+  compute_bounds_for_level (pbb, level, low, up);
+
+  if (value_neg_p (low))
+    unsigned_p = false;
+
+  value_init (diff);
+  value_subtract (diff, up, low);
+  value_clear (low);
+  value_clear (up);
+
+  value_init (x);
+  value_init (two);
+  value_set_si (x, 256);
+  value_set_si (two, 2);
+  precision = 8;
+  while (value_gt (diff, x))
+    {
+      value_multiply (x, x, two);
+      precision++;
+    }
+
+  value_clear (diff);
+  value_clear (x);
+  value_clear (two);
+
+  type = lang_hooks.types.type_for_size (precision, unsigned_p);
+  if (!type)
+    {
+      gloog_error = true;
+      return integer_type_node;
+    }
+
+  return type;
+}
+
 /* Build for USER_STMT a map between the CLAST induction variables and
    the corresponding GCC old induction variables.  This information is
    stored on each GRAPHITE_BB.  */
@@ -1027,9 +1110,9 @@ compute_cloog_iv_types_1 (poly_bb_p pbb, struct clast_user_stmt *user_stmt)
 {
   gimple_bb_p gbb = PBB_BLACK_BOX (pbb);
   struct clast_stmt *t;
-  int index = 0;
+  int level = 0;
 
-  for (t = user_stmt->substitutions; t; t = t->next, index++)
+  for (t = user_stmt->substitutions; t; t = t->next, level++)
     {
       PTR *slot;
       struct ivtype_map_elt_s tmp;
@@ -1044,11 +1127,8 @@ compute_cloog_iv_types_1 (poly_bb_p pbb, struct clast_user_stmt *user_stmt)
       slot = htab_find_slot (GBB_CLOOG_IV_TYPES (gbb), &tmp, INSERT);
 
       if (slot && !*slot)
-	{
-	  tree oldiv = pbb_to_depth_to_oldiv (pbb, index);
-	  tree type = oldiv ? TREE_TYPE (oldiv) : integer_type_node;
-	  *slot = new_ivtype_map_elt (tmp.cloog_iv, type);
-	}
+	*slot = new_ivtype_map_elt (tmp.cloog_iv,
+				    compute_type_for_level (pbb, level));
     }
 }
 
-- 
1.6.3.3


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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-03-05 14:22 [Graphite] Fix type problems in loop ivs Tobias Grosser
                   ` (2 preceding siblings ...)
       [not found] ` <11654_1267799959_4B911797_11654_9062_1_201003051538.47829.ebotcazou@adacore.com>
@ 2010-03-10 21:23 ` Sebastian Pop
  2010-03-11 12:31   ` Richard Guenther
  3 siblings, 1 reply; 14+ messages in thread
From: Sebastian Pop @ 2010-03-10 21:23 UTC (permalink / raw)
  To: Tobias Grosser; +Cc: gcc-patches, gcc-graphite

Hi,

On Fri, Mar 5, 2010 at 08:14, Tobias Grosser <grosser@fim.uni-passau.de> wrote:
> Hi,
>
> I would like to commit this patch that fixes the loop iv type problems in
> graphite.
>
> This resolves two P1 bug reports without any regressions in the graphite.exp
> testsuite neither on 64 nor on 32 bit linux. Only two test cases in libgomp
> changed, as autopar generates for these files an iv where we cannot yet
> prove, that it can be handled safely in graphite. This can be enhanced later on.
>
> The patch was bootstrapped on
> Linux tobias 2.6.33 #4 SMP PREEMPT Mon Mar 1 12:59:44 CET 2010 x86_64 Intel(R)
> Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel GNU/Linux
>
> Linux tobias 2.6.33 #4 SMP PREEMPT Mon Mar 1 12:59:44 CET 2010 i686 Intel(R)
> Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel GNU/Linux
>
> Also tested dealII on 64bit with -O2, which failed before.
>
> OK for trunk, if it passes the automatic spec 2006 tests on graphite branch?
>
> Tobias
>
> PS: Thanks Richi, for proposing "fold_convert (sizetype, name)" for the conversion
> from pointers.
>
> Fix pr42644.
> Fix pr42130 (dealII).
>
> 2010-03-03  Tobias Grosser  <grosser@fim.uni-passau.de>
>
>        * gcc/graphite-clast-to-gimple.c (clast_to_gcc_expression): Also
>        handle conversions from pointer to integers.
>        (gcc_type_for_cloog_iv): Choose the smalles signed integer as an
>        induction variable, to be able to work with code generated by
>        CLooG.
>        * gcc/graphite-sese-to-poly.c (scop_ivs_can_be_represented): New.
>        (build_poly_scop): Bail out if we cannot codegen a loop.
>        * gcc/testsuite/gcc.dg/graphite/id-18.c: New.
>        * gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c: New.
>        * libgomp/testsuite/libgomp.graphite/force-parallel-1.c: Adjust.
>        * libgomp/testsuite/libgomp.graphite/force-parallel-2.c: Adjust.
> ---
>  gcc/graphite-clast-to-gimple.c                     |   67 +++++++++++++++++--
>  gcc/graphite-sese-to-poly.c                        |   36 +++++++++++
>  gcc/testsuite/gcc.dg/graphite/id-18.c              |    7 ++
>  gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c     |   32 +++++++++
>  .../testsuite/libgomp.graphite/force-parallel-1.c  |    2 +-
>  .../testsuite/libgomp.graphite/force-parallel-2.c  |    2 +-
>  6 files changed, 137 insertions(+), 9 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/graphite/id-18.c
>  create mode 100644 gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c
>
> diff --git a/gcc/graphite-clast-to-gimple.c b/gcc/graphite-clast-to-gimple.c
> index 909392a..6ce01d5 100644
> --- a/gcc/graphite-clast-to-gimple.c
> +++ b/gcc/graphite-clast-to-gimple.c
> @@ -282,14 +282,24 @@ clast_to_gcc_expression (tree type, struct clast_expr *e,
>              {
>                tree name = clast_name_to_gcc (t->var, region, newivs,
>                                               newivs_index, params_index);
> -               return fold_convert (type, name);
> +
> +               if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
> +                 name = fold_convert (sizetype, name);
> +
> +               name = fold_convert (type, name);
> +               return name;
>              }
>
>            else if (value_mone_p (t->val))
>              {
>                tree name = clast_name_to_gcc (t->var, region, newivs,
>                                               newivs_index, params_index);
> +
> +               if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
> +                 name = fold_convert (sizetype, name);
> +
>                name = fold_convert (type, name);
> +
>                return fold_build1 (NEGATE_EXPR, type, name);
>              }
>            else
> @@ -297,7 +307,12 @@ clast_to_gcc_expression (tree type, struct clast_expr *e,
>                tree name = clast_name_to_gcc (t->var, region, newivs,
>                                               newivs_index, params_index);
>                tree cst = gmp_cst_to_tree (type, t->val);
> +
> +               if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
> +                 name = fold_convert (sizetype, name);
> +
>                name = fold_convert (type, name);
> +
>                if (!POINTER_TYPE_P (type))
>                  return fold_build2 (MULT_EXPR, type, cst, name);
>
> @@ -532,9 +547,16 @@ clast_get_body_of_loop (struct clast_stmt *stmt)
>   gcc_unreachable ();
>  }
>
> -/* Given a CLOOG_IV, returns the type that it should have in GCC land.
> -   If the information is not available, i.e. in the case one of the
> -   transforms created the loop, just return integer_type_node.  */
> +/* Given a CLOOG_IV, return the type that CLOOG_IV should have in GCC
> +   land.  The selected type is big enough to include the original loop
> +   iteration variable, but signed to work with the subtractions CLooG
> +   may have introduced.  If such a type is not available, we fail.
> +
> +   TODO: Do not always return long_long, but the smallest possible
> +   type, that still holds the original type.
> +
> +   TODO: Get the types using CLooG instead.  This enables further
> +   optimizations, but needs CLooG support.  */
>
>  static tree
>  gcc_type_for_cloog_iv (const char *cloog_iv, gimple_bb_p gbb)
> @@ -546,9 +568,40 @@ gcc_type_for_cloog_iv (const char *cloog_iv, gimple_bb_p gbb)
>   slot = htab_find_slot (GBB_CLOOG_IV_TYPES (gbb), &tmp, NO_INSERT);
>
>   if (slot && *slot)
> -    return ((ivtype_map_elt) *slot)->type;
> +    {
> +      tree type = ((ivtype_map_elt) *slot)->type;
> +      int type_precision = TYPE_PRECISION (type);
> +
> +      /* Find the smallest signed type possible.  */
> +      if (!TYPE_UNSIGNED (type))
> +       {
> +         if (type_precision <= TYPE_PRECISION (integer_type_node))
> +           return integer_type_node;
> +
> +         if (type_precision <= TYPE_PRECISION (long_integer_type_node))
> +           return long_integer_type_node;
> +
> +         if (type_precision <= TYPE_PRECISION (long_long_integer_type_node))
> +           return long_long_integer_type_node;
> +
> +         gcc_unreachable ();
> +       }
> +
> +      if (type_precision < TYPE_PRECISION (integer_type_node))
> +       return integer_type_node;
> +
> +      if (type_precision < TYPE_PRECISION (long_integer_type_node))
> +       return long_integer_type_node;
> +
> +      if (type_precision < TYPE_PRECISION (long_long_integer_type_node))
> +       return long_long_integer_type_node;
> +
> +      /* There is no signed type available, that is large enough to hold the
> +        original value.  */
> +      gcc_unreachable ();
> +    }
>
> -  return integer_type_node;
> +  return long_long_integer_type_node;
>  }
>
>  /* Returns the induction variable for the loop that gets translated to
> @@ -1046,7 +1099,7 @@ compute_cloog_iv_types_1 (poly_bb_p pbb, struct clast_user_stmt *user_stmt)
>       if (slot && !*slot)
>        {
>          tree oldiv = pbb_to_depth_to_oldiv (pbb, index);
> -         tree type = oldiv ? TREE_TYPE (oldiv) : integer_type_node;
> +         tree type = TREE_TYPE (oldiv);
>          *slot = new_ivtype_map_elt (tmp.cloog_iv, type);
>        }
>     }
> diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
> index 3ee431f..279a905 100644
> --- a/gcc/graphite-sese-to-poly.c
> +++ b/gcc/graphite-sese-to-poly.c
> @@ -2893,6 +2893,38 @@ scop_canonicalize_loops (scop_p scop)
>       graphite_loop_normal_form (loop);
>  }
>
> +/* Can all ivs be represented by a signed integer?
> +   As CLooG might generate negative values in its expressions, signed loop ivs
> +   are required in the backend. */
> +static bool
> +scop_ivs_can_be_represented (scop_p scop)
> +{
> +  loop_iterator li;
> +  loop_p loop;
> +
> +  FOR_EACH_LOOP (li, loop, 0)
> +    {
> +      tree type;
> +      int precision;
> +
> +      if (!loop_in_sese_p (loop, SCOP_REGION (scop)))
> +       continue;
> +
> +      if (!loop->single_iv)
> +       continue;
> +
> +      type = TREE_TYPE(loop->single_iv);
> +      precision = TYPE_PRECISION (type);
> +
> +      if (TYPE_UNSIGNED (type)
> +         && precision >= TYPE_PRECISION (long_long_integer_type_node))

This breaks Java when Graphite is enabled,
as there is no long_long_integer_type_node defined there.
The compiler stops on this line in the error from the Graphite
auto tester:
http://groups.google.com/group/gcc-graphite-test/browse_thread/thread/f348f4f4d50483

Sebastian

> +       return false;
> +    }
> +
> +  return true;
> +}
> +
> +
>  /* Builds the polyhedral representation for a SESE region.  */
>
>  bool
> @@ -2915,6 +2947,10 @@ build_poly_scop (scop_p scop)
>     return false;
>
>   scop_canonicalize_loops (scop);
> +
> +  if (!scop_ivs_can_be_represented (scop))
> +    return false;
> +
>   build_sese_loop_nests (region);
>   build_sese_conditions (region);
>   find_scop_parameters (scop);
> diff --git a/gcc/testsuite/gcc.dg/graphite/id-18.c b/gcc/testsuite/gcc.dg/graphite/id-18.c
> new file mode 100644
> index 0000000..77628fa
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/graphite/id-18.c
> @@ -0,0 +1,7 @@
> +long do_hash (const char * lo, const char * hi)
> +{
> +       int val = 0;
> +       for (; lo < hi; ++lo)
> +               val = *lo;
> +       return val;
> +}
> diff --git a/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c b/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c
> new file mode 100644
> index 0000000..2e90064
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c
> @@ -0,0 +1,32 @@
> +/* Testcase extracted from test 183.equake in SPEC CPU2000.  */
> +double Ke[2], ds[2];
> +
> +void foo(double Ke[2], int i, double ds[],  int column)
> +{
> +  double tt, ts;
> +  int j;
> +
> +  for (j = 0; j < 2; j++)
> +    {
> +      ++column;
> +      ts = ds[i];
> +      if (i == j)
> +       tt = 123;
> +      else
> +       tt = 0;
> +      Ke[column] = Ke[column] + ts + tt;
> +    }
> +}
> +
> +int
> +main ()
> +{
> +  int i, j;
> +
> +  ds[0] = 1.0;
> +  ds[1] = 1.0;
> +
> +  foo(Ke, 0, ds, -1);
> +
> +  return (int) Ke[0] != 124;
> +}
> diff --git a/libgomp/testsuite/libgomp.graphite/force-parallel-1.c b/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
> index 7f043d8..1ad4b41 100644
> --- a/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
> +++ b/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
> @@ -23,7 +23,7 @@ int main(void)
>  }
>
>  /* Check that parallel code generation part make the right answer.  */
> -/* { dg-final { scan-tree-dump-times "1 loops carried no dependency" 2 "graphite" } } */
> +/* { dg-final { scan-tree-dump-times "1 loops carried no dependency" 1 "graphite" } } */
>  /* { dg-final { cleanup-tree-dump "graphite" } } */
>  /* { dg-final { scan-tree-dump-times "loopfn" 5 "optimized" } } */
>  /* { dg-final { cleanup-tree-dump "parloops" } } */
> diff --git a/libgomp/testsuite/libgomp.graphite/force-parallel-2.c b/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
> index 03d8236..1ce0feb 100644
> --- a/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
> +++ b/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
> @@ -23,7 +23,7 @@ int main(void)
>  }
>
>  /* Check that parallel code generation part make the right answer.  */
> -/* { dg-final { scan-tree-dump-times "2 loops carried no dependency" 2 "graphite" } } */
> +/* { dg-final { scan-tree-dump-times "2 loops carried no dependency" 1 "graphite" } } */
>  /* { dg-final { cleanup-tree-dump "graphite" } } */
>  /* { dg-final { scan-tree-dump-times "loopfn" 5 "optimized" } } */
>  /* { dg-final { cleanup-tree-dump "parloops" } } */
> --
> 1.6.4.4
>
>

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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-03-10 21:23 ` Sebastian Pop
@ 2010-03-11 12:31   ` Richard Guenther
  2010-03-11 13:56     ` Sebastian Pop
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Guenther @ 2010-03-11 12:31 UTC (permalink / raw)
  To: Sebastian Pop; +Cc: Tobias Grosser, gcc-patches, gcc-graphite

On Wed, Mar 10, 2010 at 9:33 PM, Sebastian Pop <sebpop@gmail.com> wrote:
> Hi,
>
> On Fri, Mar 5, 2010 at 08:14, Tobias Grosser <grosser@fim.uni-passau.de> wrote:
>> Hi,
>>
>> I would like to commit this patch that fixes the loop iv type problems in
>> graphite.
>>
>> This resolves two P1 bug reports without any regressions in the graphite.exp
>> testsuite neither on 64 nor on 32 bit linux. Only two test cases in libgomp
>> changed, as autopar generates for these files an iv where we cannot yet
>> prove, that it can be handled safely in graphite. This can be enhanced later on.
>>
>> The patch was bootstrapped on
>> Linux tobias 2.6.33 #4 SMP PREEMPT Mon Mar 1 12:59:44 CET 2010 x86_64 Intel(R)
>> Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel GNU/Linux
>>
>> Linux tobias 2.6.33 #4 SMP PREEMPT Mon Mar 1 12:59:44 CET 2010 i686 Intel(R)
>> Core(TM) i5 CPU M 520 @ 2.40GHz GenuineIntel GNU/Linux
>>
>> Also tested dealII on 64bit with -O2, which failed before.
>>
>> OK for trunk, if it passes the automatic spec 2006 tests on graphite branch?
>>
>> Tobias
>>
>> PS: Thanks Richi, for proposing "fold_convert (sizetype, name)" for the conversion
>> from pointers.
>>
>> Fix pr42644.
>> Fix pr42130 (dealII).
>>
>> 2010-03-03  Tobias Grosser  <grosser@fim.uni-passau.de>
>>
>>        * gcc/graphite-clast-to-gimple.c (clast_to_gcc_expression): Also
>>        handle conversions from pointer to integers.
>>        (gcc_type_for_cloog_iv): Choose the smalles signed integer as an
>>        induction variable, to be able to work with code generated by
>>        CLooG.
>>        * gcc/graphite-sese-to-poly.c (scop_ivs_can_be_represented): New.
>>        (build_poly_scop): Bail out if we cannot codegen a loop.
>>        * gcc/testsuite/gcc.dg/graphite/id-18.c: New.
>>        * gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c: New.
>>        * libgomp/testsuite/libgomp.graphite/force-parallel-1.c: Adjust.
>>        * libgomp/testsuite/libgomp.graphite/force-parallel-2.c: Adjust.
>> ---
>>  gcc/graphite-clast-to-gimple.c                     |   67 +++++++++++++++++--
>>  gcc/graphite-sese-to-poly.c                        |   36 +++++++++++
>>  gcc/testsuite/gcc.dg/graphite/id-18.c              |    7 ++
>>  gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c     |   32 +++++++++
>>  .../testsuite/libgomp.graphite/force-parallel-1.c  |    2 +-
>>  .../testsuite/libgomp.graphite/force-parallel-2.c  |    2 +-
>>  6 files changed, 137 insertions(+), 9 deletions(-)
>>  create mode 100644 gcc/testsuite/gcc.dg/graphite/id-18.c
>>  create mode 100644 gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c
>>
>> diff --git a/gcc/graphite-clast-to-gimple.c b/gcc/graphite-clast-to-gimple.c
>> index 909392a..6ce01d5 100644
>> --- a/gcc/graphite-clast-to-gimple.c
>> +++ b/gcc/graphite-clast-to-gimple.c
>> @@ -282,14 +282,24 @@ clast_to_gcc_expression (tree type, struct clast_expr *e,
>>              {
>>                tree name = clast_name_to_gcc (t->var, region, newivs,
>>                                               newivs_index, params_index);
>> -               return fold_convert (type, name);
>> +
>> +               if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
>> +                 name = fold_convert (sizetype, name);
>> +
>> +               name = fold_convert (type, name);
>> +               return name;
>>              }
>>
>>            else if (value_mone_p (t->val))
>>              {
>>                tree name = clast_name_to_gcc (t->var, region, newivs,
>>                                               newivs_index, params_index);
>> +
>> +               if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
>> +                 name = fold_convert (sizetype, name);
>> +
>>                name = fold_convert (type, name);
>> +
>>                return fold_build1 (NEGATE_EXPR, type, name);
>>              }
>>            else
>> @@ -297,7 +307,12 @@ clast_to_gcc_expression (tree type, struct clast_expr *e,
>>                tree name = clast_name_to_gcc (t->var, region, newivs,
>>                                               newivs_index, params_index);
>>                tree cst = gmp_cst_to_tree (type, t->val);
>> +
>> +               if (POINTER_TYPE_P (TREE_TYPE (name)) != POINTER_TYPE_P (type))
>> +                 name = fold_convert (sizetype, name);
>> +
>>                name = fold_convert (type, name);
>> +
>>                if (!POINTER_TYPE_P (type))
>>                  return fold_build2 (MULT_EXPR, type, cst, name);
>>
>> @@ -532,9 +547,16 @@ clast_get_body_of_loop (struct clast_stmt *stmt)
>>   gcc_unreachable ();
>>  }
>>
>> -/* Given a CLOOG_IV, returns the type that it should have in GCC land.
>> -   If the information is not available, i.e. in the case one of the
>> -   transforms created the loop, just return integer_type_node.  */
>> +/* Given a CLOOG_IV, return the type that CLOOG_IV should have in GCC
>> +   land.  The selected type is big enough to include the original loop
>> +   iteration variable, but signed to work with the subtractions CLooG
>> +   may have introduced.  If such a type is not available, we fail.
>> +
>> +   TODO: Do not always return long_long, but the smallest possible
>> +   type, that still holds the original type.
>> +
>> +   TODO: Get the types using CLooG instead.  This enables further
>> +   optimizations, but needs CLooG support.  */
>>
>>  static tree
>>  gcc_type_for_cloog_iv (const char *cloog_iv, gimple_bb_p gbb)
>> @@ -546,9 +568,40 @@ gcc_type_for_cloog_iv (const char *cloog_iv, gimple_bb_p gbb)
>>   slot = htab_find_slot (GBB_CLOOG_IV_TYPES (gbb), &tmp, NO_INSERT);
>>
>>   if (slot && *slot)
>> -    return ((ivtype_map_elt) *slot)->type;
>> +    {
>> +      tree type = ((ivtype_map_elt) *slot)->type;
>> +      int type_precision = TYPE_PRECISION (type);
>> +
>> +      /* Find the smallest signed type possible.  */
>> +      if (!TYPE_UNSIGNED (type))
>> +       {
>> +         if (type_precision <= TYPE_PRECISION (integer_type_node))
>> +           return integer_type_node;
>> +
>> +         if (type_precision <= TYPE_PRECISION (long_integer_type_node))
>> +           return long_integer_type_node;
>> +
>> +         if (type_precision <= TYPE_PRECISION (long_long_integer_type_node))
>> +           return long_long_integer_type_node;
>> +
>> +         gcc_unreachable ();
>> +       }
>> +
>> +      if (type_precision < TYPE_PRECISION (integer_type_node))
>> +       return integer_type_node;
>> +
>> +      if (type_precision < TYPE_PRECISION (long_integer_type_node))
>> +       return long_integer_type_node;
>> +
>> +      if (type_precision < TYPE_PRECISION (long_long_integer_type_node))
>> +       return long_long_integer_type_node;
>> +
>> +      /* There is no signed type available, that is large enough to hold the
>> +        original value.  */
>> +      gcc_unreachable ();
>> +    }
>>
>> -  return integer_type_node;
>> +  return long_long_integer_type_node;
>>  }
>>
>>  /* Returns the induction variable for the loop that gets translated to
>> @@ -1046,7 +1099,7 @@ compute_cloog_iv_types_1 (poly_bb_p pbb, struct clast_user_stmt *user_stmt)
>>       if (slot && !*slot)
>>        {
>>          tree oldiv = pbb_to_depth_to_oldiv (pbb, index);
>> -         tree type = oldiv ? TREE_TYPE (oldiv) : integer_type_node;
>> +         tree type = TREE_TYPE (oldiv);
>>          *slot = new_ivtype_map_elt (tmp.cloog_iv, type);
>>        }
>>     }
>> diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
>> index 3ee431f..279a905 100644
>> --- a/gcc/graphite-sese-to-poly.c
>> +++ b/gcc/graphite-sese-to-poly.c
>> @@ -2893,6 +2893,38 @@ scop_canonicalize_loops (scop_p scop)
>>       graphite_loop_normal_form (loop);
>>  }
>>
>> +/* Can all ivs be represented by a signed integer?
>> +   As CLooG might generate negative values in its expressions, signed loop ivs
>> +   are required in the backend. */
>> +static bool
>> +scop_ivs_can_be_represented (scop_p scop)
>> +{
>> +  loop_iterator li;
>> +  loop_p loop;
>> +
>> +  FOR_EACH_LOOP (li, loop, 0)
>> +    {
>> +      tree type;
>> +      int precision;
>> +
>> +      if (!loop_in_sese_p (loop, SCOP_REGION (scop)))
>> +       continue;
>> +
>> +      if (!loop->single_iv)
>> +       continue;
>> +
>> +      type = TREE_TYPE(loop->single_iv);
>> +      precision = TYPE_PRECISION (type);
>> +
>> +      if (TYPE_UNSIGNED (type)
>> +         && precision >= TYPE_PRECISION (long_long_integer_type_node))
>
> This breaks Java when Graphite is enabled,
> as there is no long_long_integer_type_node defined there.
> The compiler stops on this line in the error from the Graphite
> auto tester:
> http://groups.google.com/group/gcc-graphite-test/browse_thread/thread/f348f4f4d50483

Java indeed fails to initialize most of the common trees.
I suggest to use ssizetype if long_long_type_node is NULL.

Richard.

> Sebastian
>
>> +       return false;
>> +    }
>> +
>> +  return true;
>> +}
>> +
>> +
>>  /* Builds the polyhedral representation for a SESE region.  */
>>
>>  bool
>> @@ -2915,6 +2947,10 @@ build_poly_scop (scop_p scop)
>>     return false;
>>
>>   scop_canonicalize_loops (scop);
>> +
>> +  if (!scop_ivs_can_be_represented (scop))
>> +    return false;
>> +
>>   build_sese_loop_nests (region);
>>   build_sese_conditions (region);
>>   find_scop_parameters (scop);
>> diff --git a/gcc/testsuite/gcc.dg/graphite/id-18.c b/gcc/testsuite/gcc.dg/graphite/id-18.c
>> new file mode 100644
>> index 0000000..77628fa
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/graphite/id-18.c
>> @@ -0,0 +1,7 @@
>> +long do_hash (const char * lo, const char * hi)
>> +{
>> +       int val = 0;
>> +       for (; lo < hi; ++lo)
>> +               val = *lo;
>> +       return val;
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c b/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c
>> new file mode 100644
>> index 0000000..2e90064
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/graphite/run-id-pr42644.c
>> @@ -0,0 +1,32 @@
>> +/* Testcase extracted from test 183.equake in SPEC CPU2000.  */
>> +double Ke[2], ds[2];
>> +
>> +void foo(double Ke[2], int i, double ds[],  int column)
>> +{
>> +  double tt, ts;
>> +  int j;
>> +
>> +  for (j = 0; j < 2; j++)
>> +    {
>> +      ++column;
>> +      ts = ds[i];
>> +      if (i == j)
>> +       tt = 123;
>> +      else
>> +       tt = 0;
>> +      Ke[column] = Ke[column] + ts + tt;
>> +    }
>> +}
>> +
>> +int
>> +main ()
>> +{
>> +  int i, j;
>> +
>> +  ds[0] = 1.0;
>> +  ds[1] = 1.0;
>> +
>> +  foo(Ke, 0, ds, -1);
>> +
>> +  return (int) Ke[0] != 124;
>> +}
>> diff --git a/libgomp/testsuite/libgomp.graphite/force-parallel-1.c b/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
>> index 7f043d8..1ad4b41 100644
>> --- a/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
>> +++ b/libgomp/testsuite/libgomp.graphite/force-parallel-1.c
>> @@ -23,7 +23,7 @@ int main(void)
>>  }
>>
>>  /* Check that parallel code generation part make the right answer.  */
>> -/* { dg-final { scan-tree-dump-times "1 loops carried no dependency" 2 "graphite" } } */
>> +/* { dg-final { scan-tree-dump-times "1 loops carried no dependency" 1 "graphite" } } */
>>  /* { dg-final { cleanup-tree-dump "graphite" } } */
>>  /* { dg-final { scan-tree-dump-times "loopfn" 5 "optimized" } } */
>>  /* { dg-final { cleanup-tree-dump "parloops" } } */
>> diff --git a/libgomp/testsuite/libgomp.graphite/force-parallel-2.c b/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
>> index 03d8236..1ce0feb 100644
>> --- a/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
>> +++ b/libgomp/testsuite/libgomp.graphite/force-parallel-2.c
>> @@ -23,7 +23,7 @@ int main(void)
>>  }
>>
>>  /* Check that parallel code generation part make the right answer.  */
>> -/* { dg-final { scan-tree-dump-times "2 loops carried no dependency" 2 "graphite" } } */
>> +/* { dg-final { scan-tree-dump-times "2 loops carried no dependency" 1 "graphite" } } */
>>  /* { dg-final { cleanup-tree-dump "graphite" } } */
>>  /* { dg-final { scan-tree-dump-times "loopfn" 5 "optimized" } } */
>>  /* { dg-final { cleanup-tree-dump "parloops" } } */
>> --
>> 1.6.4.4
>>
>>
>

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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-03-11 12:31   ` Richard Guenther
@ 2010-03-11 13:56     ` Sebastian Pop
  2010-03-11 14:20       ` Richard Guenther
  0 siblings, 1 reply; 14+ messages in thread
From: Sebastian Pop @ 2010-03-11 13:56 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Tobias Grosser, gcc-patches, gcc-graphite

[-- Attachment #1: Type: text/plain, Size: 264 bytes --]

On Thu, Mar 11, 2010 at 04:48, Richard Guenther
<richard.guenther@gmail.com> wrote:
> Java indeed fails to initialize most of the common trees.
> I suggest to use ssizetype if long_long_type_node is NULL.

Here is the fix.  Does this look good?

Thanks,
Sebastian

[-- Attachment #2: 0001-Use-ssizetype-when-long_long_integer_type_node-is-NU.patch --]
[-- Type: text/x-diff, Size: 4043 bytes --]

From 2e4707d2b71859e566ba51c08b54e7f24ceba96b Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Thu, 11 Mar 2010 07:49:36 -0600
Subject: [PATCH] Use ssizetype when long_long_integer_type_node is NULL.

2010-03-11  Sebastian Pop  <sebastian.pop@amd.com>

	* graphite-clast-to-gimple.c (my_long_long): Defined.
	(gcc_type_for_cloog_iv): Use it instead of long_long_integer_type_node.
	* graphite-sese-to-poly.c (my_long_long): Defined.
	(scop_ivs_can_be_represented): Use it.
---
 gcc/ChangeLog.graphite         |    7 +++++++
 gcc/graphite-clast-to-gimple.c |   15 ++++++++++-----
 gcc/graphite-sese-to-poly.c    |    6 +++++-
 3 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/gcc/ChangeLog.graphite b/gcc/ChangeLog.graphite
index bda7a05..5531c2e 100644
--- a/gcc/ChangeLog.graphite
+++ b/gcc/ChangeLog.graphite
@@ -1,3 +1,10 @@
+2010-03-11  Sebastian Pop  <sebastian.pop@amd.com>
+
+	* graphite-clast-to-gimple.c (my_long_long): Defined.
+	(gcc_type_for_cloog_iv): Use it instead of long_long_integer_type_node.
+	* graphite-sese-to-poly.c (my_long_long): Defined.
+	(scop_ivs_can_be_represented): Use it.
+
 2010-03-10  Sebastian Pop  <sebastian.pop@amd.com>
 
 	* doc/invoke.texi: Fix documentation of graphite-max-nb-scop-params,
diff --git a/gcc/graphite-clast-to-gimple.c b/gcc/graphite-clast-to-gimple.c
index 004e120..b155664 100644
--- a/gcc/graphite-clast-to-gimple.c
+++ b/gcc/graphite-clast-to-gimple.c
@@ -547,6 +547,9 @@ clast_get_body_of_loop (struct clast_stmt *stmt)
   gcc_unreachable ();
 }
 
+/* Java does not initialize long_long_integer_type_node.  */
+#define my_long_long (long_long_integer_type_node ? long_long_integer_type_node : ssizetype)
+
 /* Given a CLOOG_IV, return the type that CLOOG_IV should have in GCC
    land.  The selected type is big enough to include the original loop
    iteration variable, but signed to work with the subtractions CLooG
@@ -581,8 +584,8 @@ gcc_type_for_cloog_iv (const char *cloog_iv, gimple_bb_p gbb)
 	  if (type_precision <= TYPE_PRECISION (long_integer_type_node))
 	    return long_integer_type_node;
 
-	  if (type_precision <= TYPE_PRECISION (long_long_integer_type_node))
-	    return long_long_integer_type_node;
+	  if (type_precision <= TYPE_PRECISION (my_long_long))
+	    return my_long_long;
 
 	  gcc_unreachable ();
 	}
@@ -593,17 +596,19 @@ gcc_type_for_cloog_iv (const char *cloog_iv, gimple_bb_p gbb)
       if (type_precision < TYPE_PRECISION (long_integer_type_node))
 	return long_integer_type_node;
 
-      if (type_precision < TYPE_PRECISION (long_long_integer_type_node))
-	return long_long_integer_type_node;
+      if (type_precision < TYPE_PRECISION (my_long_long))
+	return my_long_long;
 
       /* There is no signed type available, that is large enough to hold the
 	 original value.  */
       gcc_unreachable ();
     }
 
-  return long_long_integer_type_node;
+  return my_long_long;
 }
 
+#undef my_long_long
+
 /* Returns the induction variable for the loop that gets translated to
    STMT.  */
 
diff --git a/gcc/graphite-sese-to-poly.c b/gcc/graphite-sese-to-poly.c
index 28ed07c..0f370a8 100644
--- a/gcc/graphite-sese-to-poly.c
+++ b/gcc/graphite-sese-to-poly.c
@@ -2910,6 +2910,9 @@ scop_canonicalize_loops (scop_p scop)
       graphite_loop_normal_form (loop);
 }
 
+/* Java does not initialize long_long_integer_type_node.  */
+#define my_long_long (long_long_integer_type_node ? long_long_integer_type_node : ssizetype)
+
 /* Can all ivs be represented by a signed integer?
    As CLooG might generate negative values in its expressions, signed loop ivs
    are required in the backend. */
@@ -2934,13 +2937,14 @@ scop_ivs_can_be_represented (scop_p scop)
       precision = TYPE_PRECISION (type);
 
       if (TYPE_UNSIGNED (type)
-	  && precision >= TYPE_PRECISION (long_long_integer_type_node))
+	  && precision >= TYPE_PRECISION (my_long_long))
 	return false;
     }
 
   return true;
 }
 
+#undef my_long_long
 
 /* Builds the polyhedral representation for a SESE region.  */
 
-- 
1.6.3.3


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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-03-11 13:56     ` Sebastian Pop
@ 2010-03-11 14:20       ` Richard Guenther
  2010-06-01 23:10         ` Sebastian Pop
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Guenther @ 2010-03-11 14:20 UTC (permalink / raw)
  To: Sebastian Pop; +Cc: Tobias Grosser, gcc-patches, gcc-graphite

On Thu, Mar 11, 2010 at 2:54 PM, Sebastian Pop <sebpop@gmail.com> wrote:
> On Thu, Mar 11, 2010 at 04:48, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> Java indeed fails to initialize most of the common trees.
>> I suggest to use ssizetype if long_long_type_node is NULL.
>
> Here is the fix.  Does this look good?

Hm.  I don't like the #define.  And Java doesn't initialize
long_integer_type_node either, so I think the patch won't fix
the bootstrap issue.  Basically the C kind types are not
really supposed to be used in the middle-end.

What you probably want is to use
smallest_mode_for_size (type_precision + 1, MODE_INT)
and fail if that returns BLKmode (ugh, it doesn't ...).
From the modes precision then build a type using
build_nonstandard_integer_type. And possibly you
want to restrict yourself to maximum BITS_PER_WORD
size to avoid long long or TImode arithmetic.

Richard.

> Thanks,
> Sebastian
>

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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-03-11 14:20       ` Richard Guenther
@ 2010-06-01 23:10         ` Sebastian Pop
  2010-06-02  9:03           ` Richard Guenther
  0 siblings, 1 reply; 14+ messages in thread
From: Sebastian Pop @ 2010-06-01 23:10 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Tobias Grosser, gcc-patches, gcc-graphite

[-- Attachment #1: Type: text/plain, Size: 1241 bytes --]

On Thu, Mar 11, 2010 at 09:16, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Thu, Mar 11, 2010 at 2:54 PM, Sebastian Pop <sebpop@gmail.com> wrote:
>> On Thu, Mar 11, 2010 at 04:48, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> Java indeed fails to initialize most of the common trees.
>>> I suggest to use ssizetype if long_long_type_node is NULL.
>>
>> Here is the fix.  Does this look good?
>
> Hm.  I don't like the #define.  And Java doesn't initialize
> long_integer_type_node either, so I think the patch won't fix
> the bootstrap issue.  Basically the C kind types are not
> really supposed to be used in the middle-end.
>
> What you probably want is to use
> smallest_mode_for_size (type_precision + 1, MODE_INT)
> and fail if that returns BLKmode (ugh, it doesn't ...).
> From the modes precision then build a type using
> build_nonstandard_integer_type. And possibly you
> want to restrict yourself to maximum BITS_PER_WORD
> size to avoid long long or TImode arithmetic.
>

Sorry for having neglected this issue so long.
Here is the patch that fixes this induction variable type computation.
Ok for trunk after the automatic tests in the graphite branch?

Thanks,
Sebastian

[-- Attachment #2: 0001-Use-smallest_mode_for_size-for-computing-the-precisi.patch --]
[-- Type: text/x-diff, Size: 1349 bytes --]

From 9143a7912763ddd2b0fa639d54dbc2c8d9cac7e0 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Tue, 1 Jun 2010 18:03:35 -0500
Subject: [PATCH] Use smallest_mode_for_size for computing the precision types of new graphite IVs.

---
 gcc/graphite-clast-to-gimple.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/gcc/graphite-clast-to-gimple.c b/gcc/graphite-clast-to-gimple.c
index b0e1a94..3240c37 100644
--- a/gcc/graphite-clast-to-gimple.c
+++ b/gcc/graphite-clast-to-gimple.c
@@ -478,6 +478,7 @@ gcc_type_for_interval (mpz_t low, mpz_t up, tree old_type)
   bool unsigned_p = true;
   int precision, prec_up, prec_int;
   tree type;
+  enum machine_mode mode;
 
   gcc_assert (value_le (low, up));
 
@@ -490,7 +491,16 @@ gcc_type_for_interval (mpz_t low, mpz_t up, tree old_type)
   prec_int = precision_for_interval (low, up);
   precision = prec_up > prec_int ? prec_up : prec_int;
 
-  type = lang_hooks.types.type_for_size (precision, unsigned_p);
+  if (precision > BITS_PER_WORD)
+    {
+      gloog_error = true;
+      return integer_type_node;
+    }
+
+  mode = smallest_mode_for_size (precision, MODE_INT);
+  precision = GET_MODE_PRECISION (mode);
+  type = build_nonstandard_integer_type (precision, unsigned_p);
+
   if (!type)
     {
       gloog_error = true;
-- 
1.7.0.4


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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-06-01 23:10         ` Sebastian Pop
@ 2010-06-02  9:03           ` Richard Guenther
  2010-06-07 16:36             ` NightStrike
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Guenther @ 2010-06-02  9:03 UTC (permalink / raw)
  To: Sebastian Pop; +Cc: Tobias Grosser, gcc-patches, gcc-graphite

On Wed, Jun 2, 2010 at 1:09 AM, Sebastian Pop <sebpop@gmail.com> wrote:
> On Thu, Mar 11, 2010 at 09:16, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Thu, Mar 11, 2010 at 2:54 PM, Sebastian Pop <sebpop@gmail.com> wrote:
>>> On Thu, Mar 11, 2010 at 04:48, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> Java indeed fails to initialize most of the common trees.
>>>> I suggest to use ssizetype if long_long_type_node is NULL.
>>>
>>> Here is the fix.  Does this look good?
>>
>> Hm.  I don't like the #define.  And Java doesn't initialize
>> long_integer_type_node either, so I think the patch won't fix
>> the bootstrap issue.  Basically the C kind types are not
>> really supposed to be used in the middle-end.
>>
>> What you probably want is to use
>> smallest_mode_for_size (type_precision + 1, MODE_INT)
>> and fail if that returns BLKmode (ugh, it doesn't ...).
>> From the modes precision then build a type using
>> build_nonstandard_integer_type. And possibly you
>> want to restrict yourself to maximum BITS_PER_WORD
>> size to avoid long long or TImode arithmetic.
>>
>
> Sorry for having neglected this issue so long.
> Here is the patch that fixes this induction variable type computation.
> Ok for trunk after the automatic tests in the graphite branch?

Ok.

Thanks,
Richard.

> Thanks,
> Sebastian
>

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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-06-02  9:03           ` Richard Guenther
@ 2010-06-07 16:36             ` NightStrike
  2010-06-07 16:59               ` Sebastian Pop
  0 siblings, 1 reply; 14+ messages in thread
From: NightStrike @ 2010-06-07 16:36 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Sebastian Pop, Tobias Grosser, gcc-patches, gcc-graphite

On Wed, Jun 2, 2010 at 5:03 AM, Richard Guenther
<richard.guenther@gmail.com> wrote:
> On Wed, Jun 2, 2010 at 1:09 AM, Sebastian Pop <sebpop@gmail.com> wrote:
>> On Thu, Mar 11, 2010 at 09:16, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Thu, Mar 11, 2010 at 2:54 PM, Sebastian Pop <sebpop@gmail.com> wrote:
>>>> On Thu, Mar 11, 2010 at 04:48, Richard Guenther
>>>> <richard.guenther@gmail.com> wrote:
>>>>> Java indeed fails to initialize most of the common trees.
>>>>> I suggest to use ssizetype if long_long_type_node is NULL.
>>>>
>>>> Here is the fix.  Does this look good?
>>>
>>> Hm.  I don't like the #define.  And Java doesn't initialize
>>> long_integer_type_node either, so I think the patch won't fix
>>> the bootstrap issue.  Basically the C kind types are not
>>> really supposed to be used in the middle-end.
>>>
>>> What you probably want is to use
>>> smallest_mode_for_size (type_precision + 1, MODE_INT)
>>> and fail if that returns BLKmode (ugh, it doesn't ...).
>>> From the modes precision then build a type using
>>> build_nonstandard_integer_type. And possibly you
>>> want to restrict yourself to maximum BITS_PER_WORD
>>> size to avoid long long or TImode arithmetic.
>>>
>>
>> Sorry for having neglected this issue so long.
>> Here is the patch that fixes this induction variable type computation.
>> Ok for trunk after the automatic tests in the graphite branch?
>
> Ok.
>
> Thanks,
> Richard.
>
>> Thanks,
>> Sebastian
>>
>

Ping.. was this committed?

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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-06-07 16:36             ` NightStrike
@ 2010-06-07 16:59               ` Sebastian Pop
  2010-06-07 19:00                 ` NightStrike
  0 siblings, 1 reply; 14+ messages in thread
From: Sebastian Pop @ 2010-06-07 16:59 UTC (permalink / raw)
  To: NightStrike; +Cc: Richard Guenther, Tobias Grosser, gcc-patches, gcc-graphite

On Mon, Jun 7, 2010 at 11:36, NightStrike <nightstrike@gmail.com> wrote:
> On Wed, Jun 2, 2010 at 5:03 AM, Richard Guenther
> <richard.guenther@gmail.com> wrote:
>> On Wed, Jun 2, 2010 at 1:09 AM, Sebastian Pop <sebpop@gmail.com> wrote:
>>> On Thu, Mar 11, 2010 at 09:16, Richard Guenther
>>> <richard.guenther@gmail.com> wrote:
>>>> On Thu, Mar 11, 2010 at 2:54 PM, Sebastian Pop <sebpop@gmail.com> wrote:
>>>>> On Thu, Mar 11, 2010 at 04:48, Richard Guenther
>>>>> <richard.guenther@gmail.com> wrote:
>>>>>> Java indeed fails to initialize most of the common trees.
>>>>>> I suggest to use ssizetype if long_long_type_node is NULL.
>>>>>
>>>>> Here is the fix.  Does this look good?
>>>>
>>>> Hm.  I don't like the #define.  And Java doesn't initialize
>>>> long_integer_type_node either, so I think the patch won't fix
>>>> the bootstrap issue.  Basically the C kind types are not
>>>> really supposed to be used in the middle-end.
>>>>
>>>> What you probably want is to use
>>>> smallest_mode_for_size (type_precision + 1, MODE_INT)
>>>> and fail if that returns BLKmode (ugh, it doesn't ...).
>>>> From the modes precision then build a type using
>>>> build_nonstandard_integer_type. And possibly you
>>>> want to restrict yourself to maximum BITS_PER_WORD
>>>> size to avoid long long or TImode arithmetic.
>>>>
>>>
>>> Sorry for having neglected this issue so long.
>>> Here is the patch that fixes this induction variable type computation.
>>> Ok for trunk after the automatic tests in the graphite branch?
>>
>> Ok.
>>
>> Thanks,
>> Richard.
>>
>>> Thanks,
>>> Sebastian
>>>
>>
>
> Ping.. was this committed?
>

Yes, this is committed as rev.160165

Sebastian

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

* Re: [Graphite] Fix type problems in loop ivs.
  2010-06-07 16:59               ` Sebastian Pop
@ 2010-06-07 19:00                 ` NightStrike
  0 siblings, 0 replies; 14+ messages in thread
From: NightStrike @ 2010-06-07 19:00 UTC (permalink / raw)
  To: Sebastian Pop; +Cc: Richard Guenther, Tobias Grosser, gcc-patches, gcc-graphite

On Mon, Jun 7, 2010 at 12:59 PM, Sebastian Pop <sebpop@gmail.com> wrote:
> On Mon, Jun 7, 2010 at 11:36, NightStrike <nightstrike@gmail.com> wrote:
>> On Wed, Jun 2, 2010 at 5:03 AM, Richard Guenther
>> <richard.guenther@gmail.com> wrote:
>>> On Wed, Jun 2, 2010 at 1:09 AM, Sebastian Pop <sebpop@gmail.com> wrote:
>>>> On Thu, Mar 11, 2010 at 09:16, Richard Guenther
>>>> <richard.guenther@gmail.com> wrote:
>>>>> On Thu, Mar 11, 2010 at 2:54 PM, Sebastian Pop <sebpop@gmail.com> wrote:
>>>>>> On Thu, Mar 11, 2010 at 04:48, Richard Guenther
>>>>>> <richard.guenther@gmail.com> wrote:
>>>>>>> Java indeed fails to initialize most of the common trees.
>>>>>>> I suggest to use ssizetype if long_long_type_node is NULL.
>>>>>>
>>>>>> Here is the fix.  Does this look good?
>>>>>
>>>>> Hm.  I don't like the #define.  And Java doesn't initialize
>>>>> long_integer_type_node either, so I think the patch won't fix
>>>>> the bootstrap issue.  Basically the C kind types are not
>>>>> really supposed to be used in the middle-end.
>>>>>
>>>>> What you probably want is to use
>>>>> smallest_mode_for_size (type_precision + 1, MODE_INT)
>>>>> and fail if that returns BLKmode (ugh, it doesn't ...).
>>>>> From the modes precision then build a type using
>>>>> build_nonstandard_integer_type. And possibly you
>>>>> want to restrict yourself to maximum BITS_PER_WORD
>>>>> size to avoid long long or TImode arithmetic.
>>>>>
>>>>
>>>> Sorry for having neglected this issue so long.
>>>> Here is the patch that fixes this induction variable type computation.
>>>> Ok for trunk after the automatic tests in the graphite branch?
>>>
>>> Ok.
>>>
>>> Thanks,
>>> Richard.
>>>
>>>> Thanks,
>>>> Sebastian
>>>>
>>>
>>
>> Ping.. was this committed?
>>
>
> Yes, this is committed as rev.160165
>
> Sebastian
>

Thanks!

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

end of thread, other threads:[~2010-06-07 19:00 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-05 14:22 [Graphite] Fix type problems in loop ivs Tobias Grosser
2010-03-05 14:39 ` Richard Guenther
2010-03-05 20:49   ` Sebastian Pop
2010-03-05 14:40 ` Eric Botcazou
     [not found] ` <11654_1267799959_4B911797_11654_9062_1_201003051538.47829.ebotcazou@adacore.com>
2010-03-05 15:38   ` Tobias Grosser
2010-03-10 21:23 ` Sebastian Pop
2010-03-11 12:31   ` Richard Guenther
2010-03-11 13:56     ` Sebastian Pop
2010-03-11 14:20       ` Richard Guenther
2010-06-01 23:10         ` Sebastian Pop
2010-06-02  9:03           ` Richard Guenther
2010-06-07 16:36             ` NightStrike
2010-06-07 16:59               ` Sebastian Pop
2010-06-07 19:00                 ` NightStrike

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).