Re: [PATCH] Possible Fix for PR42644 and PR42130

[Sebastian Pop <sebpop at gmail dot com>]

On Mon, Feb 1, 2010 at 10:45, Ramakrishna Upadrasta
<uramakrishna@gmail.com> wrote:
> Hi Sebastian,
>
> Thanks again for the feedback. I made the changes you suggested.
>
> 2010/1/29 Sebastian Pop <sebpop@gmail.com>:
>> The patch is not ok yet: please remove all the spaces at the end of
>> the line.
>>

You did not removed all the spaces at the end of lines.

Another problem is that you do not indent correctly the braces: you
should indent by 2 spaces under an if, like this:

  if (foo)
    {
      bar;
    }

>> Have you run the testsuite with at least make -k check
>> RUNTESTFLAGS=graphite.exp?
>
> I have, but I am at loss to understand how to properly interpret the
> results. (Even without my change there seem to be unexpected
> failures.)
>

I do not see these.  Could you send out which testcases are failing
for you, and also on what architecture?

>>>> + Â Â Âprintf ("%d ", Â(int) Ke[j]);
>>>> + Â Â Âprintf("# ");
>>
>> You are executing this program, and this would print on stdout: please
>> remove the printfs or #ifdef DEBUG them.
>
> OK. I have changed the testcase. I am not sure how would we compare
> that miscompile does not happen if there is no printf and comparison
> of output. (Maybe I am missing something basic.)
>

Your program prints: "124 1 #".  So to transform your test into a
runtime test, you can sum these up, and test against 125: main should
return 0 for a runtime test to succeed.  See the attached patch.

Sebastian

From 73ced04b2e25b4f8be5713a2a7fc9ccd9e17ea02 Mon Sep 17 00:00:00 2001
From: Sebastian Pop <sebpop@gmail.com>
Date: Mon, 1 Feb 2010 11:22:35 -0600
Subject: [PATCH] Fix PR42644 and PR42130.

2010-01-22  Ramakrishna Upadrasta <Ramakrishna.Upadrasta@inria.fr>
	    Tobias Grosser  <grosser@fim.uni-passau.de>

	PR middle-end/42644
	PR middle-end/42130
	* graphite-clast-to-gimple.c (gcc_type_for_cloog_iv): Always use
	sufficiently large signed type (signed long long).  If IV does not
	fit in signed long long, then bail safely out of graphite.
	* testsuite/gcc.dg/graphite/run-id-3.c: New.
---
 gcc/graphite-clast-to-gimple.c           |   31 ++++++++++++++++++---
 gcc/testsuite/gcc.dg/graphite/run-id-3.c |   43 ++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/graphite/run-id-3.c

diff --git a/gcc/graphite-clast-to-gimple.c b/gcc/graphite-clast-to-gimple.c
index 6651e95..3f0205c 100644
--- a/gcc/graphite-clast-to-gimple.c
+++ b/gcc/graphite-clast-to-gimple.c
@@ -532,23 +532,44 @@ 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, gloog_error
+   is set.
+
+   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)
 {
   struct ivtype_map_elt_s tmp;
   PTR *slot;
+  tree type;
 
   tmp.cloog_iv = cloog_iv;
   slot = htab_find_slot (GBB_CLOOG_IV_TYPES (gbb), &tmp, NO_INSERT);
 
   if (slot && *slot)
-    return ((ivtype_map_elt) *slot)->type;
+    {
+      type = ((ivtype_map_elt) *slot)->type;
+
+      if (!useless_type_conversion_p (long_long_integer_type_node, type))
+	{
+	  /* There is no signed type available, that is large enough
+	     to hold the original value.  */
+	  gloog_error = true;
+	  return long_long_integer_type_node;
+	}
+
+      return type;
+    }
 
-  return integer_type_node;
+  return long_long_integer_type_node;
 }
 
 /* Returns the induction variable for the loop that gets translated to
diff --git a/gcc/testsuite/gcc.dg/graphite/run-id-3.c b/gcc/testsuite/gcc.dg/graphite/run-id-3.c
new file mode 100644
index 0000000..05d9e57
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/graphite/run-id-3.c
@@ -0,0 +1,43 @@
+double Ke[2], ds[2];
+
+void foo (double Ke[2], int i, double ds[], int column)
+{
+  double tt, ts;
+  unsigned long long 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 ()
+{
+  extern int printf (const char *, ...);
+  int i, j;
+
+  ds[0] = 1.0;
+  ds[1] = 1.0;
+
+  foo(Ke, 0, ds, -1);
+
+#ifdef DBG
+  for (i = 0; i < 1; i++)
+    {
+      for (j = 0; j < 2; j++)
+	printf ("%d ", (int) Ke[j]);
+      printf("# ");
+    }
+
+  printf("\n");
+#endif
+
+  return Ke[0] + Ke[1] != 125;
+}
-- 
1.6.3.3