diff mbox

[Graphite]

Message ID alpine.LNX.2.00.1201091633070.4999@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Jan. 9, 2012, 3:34 p.m. UTC
This fixes the 2nd P1 ICE.

There is a disconnect on how we analyze data-references during SCOP 
detection
(outermost_loop is the root of the loop tree) and during SESE-to-poly 
where
outermost is determined by outermost_loop_in_sese_1 ().  That influences
the SCEV result and thus we do not break the SCOP at a stmt we have to 
break
it.

The following patch fixes this using a sledgehammer - require the
data-ref to be representable if analyzed with respect to all loops
it can nest in.

Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.

Ok?

Thanks,
Richard.

2012-01-09  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/50913
	* graphite-scop-detection.c (stmt_has_simple_data_refs_p):
	Require data-refs to be representable by Graphite with respect
	to any loop nest.

	* gcc.dg/graphite/interchange-16.c: New testcase.

Comments

Tobias Grosser Jan. 9, 2012, 10:57 p.m. UTC | #1
On 01/09/2012 04:34 PM, Richard Guenther wrote:
>
> This fixes the 2nd P1 ICE.
>
> There is a disconnect on how we analyze data-references during SCOP
> detection
> (outermost_loop is the root of the loop tree) and during SESE-to-poly
> where
> outermost is determined by outermost_loop_in_sese_1 ().  That influences
> the SCEV result and thus we do not break the SCOP at a stmt we have to
> break
> it.
>
> The following patch fixes this using a sledgehammer - require the
> data-ref to be representable if analyzed with respect to all loops
> it can nest in.
>
> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>
> Ok?

This one looks good to me. The new region based scop detection should 
fix this issue, but until we can get the relevant patches in, this looks 
like a good fix. Thanks for fixing the graphite PRs.

Tobi
Richard Biener Jan. 10, 2012, 9:14 a.m. UTC | #2
On Mon, 9 Jan 2012, Tobias Grosser wrote:

> On 01/09/2012 04:34 PM, Richard Guenther wrote:
> > 
> > This fixes the 2nd P1 ICE.
> > 
> > There is a disconnect on how we analyze data-references during SCOP
> > detection
> > (outermost_loop is the root of the loop tree) and during SESE-to-poly
> > where
> > outermost is determined by outermost_loop_in_sese_1 ().  That influences
> > the SCEV result and thus we do not break the SCOP at a stmt we have to
> > break
> > it.
> > 
> > The following patch fixes this using a sledgehammer - require the
> > data-ref to be representable if analyzed with respect to all loops
> > it can nest in.
> > 
> > Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> > 
> > Ok?
> 
> This one looks good to me. The new region based scop detection should fix this
> issue, but until we can get the relevant patches in, this looks like a good
> fix. Thanks for fixing the graphite PRs.

Btw, the patch requires me to XFAIL the following tests:

FAIL: gcc.dg/graphite/scop-20.c scan-tree-dump-times graphite "number of 
SCoPs: 2" 1

huh, we now only detect one SCoP (I suppose not all stmts of a
function will end up in SCoPs?  So we have an unhandled loop here
now.)

FAIL: gfortran.dg/graphite/interchange-1.f  -O  scan-tree-dump-times 
graphite "will be interchanged" 1

No SCoPs detected anymore.

FAIL: gfortran.dg/graphite/block-1.f90  -O  scan-tree-dump-times graphite 
"number of SCoPs: 1" 1

Likewise.

FAIL: gfortran.dg/graphite/block-2.f  -O  scan-tree-dump-times graphite 
"number of SCoPs: 2" 1

Likewise.

But I also saw no easy way to use the "proper" outermost loop during
the analysis phase, so we have to live with this for 4.7 I believe.

Thus, applied.

Thanks,
Richard.
Tobias Grosser Jan. 10, 2012, 1:05 p.m. UTC | #3
On 01/10/2012 10:14 AM, Richard Guenther wrote:
> On Mon, 9 Jan 2012, Tobias Grosser wrote:
>
>> On 01/09/2012 04:34 PM, Richard Guenther wrote:
>>>
>>> This fixes the 2nd P1 ICE.
>>>
>>> There is a disconnect on how we analyze data-references during SCOP
>>> detection
>>> (outermost_loop is the root of the loop tree) and during SESE-to-poly
>>> where
>>> outermost is determined by outermost_loop_in_sese_1 ().  That influences
>>> the SCEV result and thus we do not break the SCOP at a stmt we have to
>>> break
>>> it.
>>>
>>> The following patch fixes this using a sledgehammer - require the
>>> data-ref to be representable if analyzed with respect to all loops
>>> it can nest in.
>>>
>>> Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
>>>
>>> Ok?
>>
>> This one looks good to me. The new region based scop detection should fix this
>> issue, but until we can get the relevant patches in, this looks like a good
>> fix. Thanks for fixing the graphite PRs.
>
> Btw, the patch requires me to XFAIL the following tests:
>
> FAIL: gcc.dg/graphite/scop-20.c scan-tree-dump-times graphite "number of
> SCoPs: 2" 1
>
> huh, we now only detect one SCoP (I suppose not all stmts of a
> function will end up in SCoPs?  So we have an unhandled loop here
> now.)
>
> FAIL: gfortran.dg/graphite/interchange-1.f  -O  scan-tree-dump-times
> graphite "will be interchanged" 1
>
> No SCoPs detected anymore.
>
> FAIL: gfortran.dg/graphite/block-1.f90  -O  scan-tree-dump-times graphite
> "number of SCoPs: 1" 1
>
> Likewise.
>
> FAIL: gfortran.dg/graphite/block-2.f  -O  scan-tree-dump-times graphite
> "number of SCoPs: 2" 1
>
> Likewise.
>
> But I also saw no easy way to use the "proper" outermost loop during
> the analysis phase, so we have to live with this for 4.7 I believe.
>
> Thus, applied.
Alright.

Thanks
Tobi
diff mbox

Patch

Index: gcc/graphite-scop-detection.c
===================================================================
--- gcc/graphite-scop-detection.c	(revision 183013)
+++ gcc/graphite-scop-detection.c	(working copy)
@@ -258,25 +258,33 @@  graphite_can_represent_expr (basic_block
    Graphite.  */
 
 static bool
-stmt_has_simple_data_refs_p (loop_p outermost_loop, gimple stmt)
+stmt_has_simple_data_refs_p (loop_p outermost_loop ATTRIBUTE_UNUSED,
+			     gimple stmt)
 {
   data_reference_p dr;
   unsigned i;
   int j;
   bool res = true;
-  VEC (data_reference_p, heap) *drs = VEC_alloc (data_reference_p, heap, 5);
+  VEC (data_reference_p, heap) *drs = NULL;
+  loop_p outer;
 
-  graphite_find_data_references_in_stmt (outermost_loop,
-					 loop_containing_stmt (stmt),
-					 stmt, &drs);
+  for (outer = loop_containing_stmt (stmt); outer; outer = loop_outer (outer))
+    {
+      graphite_find_data_references_in_stmt (outer,
+					     loop_containing_stmt (stmt),
+					     stmt, &drs);
 
-  FOR_EACH_VEC_ELT (data_reference_p, drs, j, dr)
-    for (i = 0; i < DR_NUM_DIMENSIONS (dr); i++)
-      if (!graphite_can_represent_scev (DR_ACCESS_FN (dr, i)))
-	{
-	  res = false;
-	  goto done;
-	}
+      FOR_EACH_VEC_ELT (data_reference_p, drs, j, dr)
+	  for (i = 0; i < DR_NUM_DIMENSIONS (dr); i++)
+	    if (!graphite_can_represent_scev (DR_ACCESS_FN (dr, i)))
+	      {
+		res = false;
+		goto done;
+	      }
+
+      free_data_refs (drs);
+      drs = NULL;
+    }
 
  done:
   free_data_refs (drs);
Index: gcc/testsuite/gcc.dg/graphite/interchange-16.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/interchange-16.c	(revision 0)
+++ gcc/testsuite/gcc.dg/graphite/interchange-16.c	(revision 0)
@@ -0,0 +1,22 @@ 
+void spread_i1 (int *rptr, int *sptr, int ncopies, int *extent, int rdelta, int m)
+{
+  int *dest;
+  int n;
+
+  while (m--)
+    {
+      dest = rptr;
+      for (n = 0; n < ncopies; n ++)
+	{
+	  *dest = *sptr;
+	  dest += rdelta;
+	}
+      if (extent [n])
+	if (n)
+	  rptr ++;
+    }
+}
+
+int main() { return 0; }
+
+/* { dg-final { cleanup-tree-dump "graphite" } } */