diff mbox

Fix PR71824

Message ID alpine.LSU.2.20.1702010902580.12993@r111.fhfr.qr
State New
Headers show

Commit Message

Richard Biener Feb. 1, 2017, 8:03 a.m. UTC 
On Tue, 31 Jan 2017, Sebastian Pop wrote:

> On Tue, Jan 31, 2017 at 9:11 AM, Richard Biener <rguenther@suse.de> wrote:
> > On Tue, 31 Jan 2017, Richard Biener wrote:
> >
> >> On Tue, 31 Jan 2017, Sebastian Pop wrote:
> >>
> >> > Resend as plain text to please gcc-patches@
> >> >
> >> > On Tue, Jan 31, 2017 at 8:39 AM, Sebastian Pop <sebpop@gmail.com> wrote:
> >> > >
> >> > >
> >> > > On Tue, Jan 31, 2017 at 7:49 AM, Richard Biener <rguenther@suse.de> wrote:
> >> > >>
> >> > >>
> >> > >> The following fixes an ICE that happens because instantiate_scev
> >> > >> doesn't really work as expected for SESE regions (a FIXME comment
> >> > >> hints at this already).  So instead of asserting all goes well
> >> > >> just bail out (add_loop_constraints seems to add constraints not
> >> > >> required for correctness?).
> >> > >
> >> > >
> >> > > The conditions under which a loop is executed are required for correctness.
> >> > > There is a similar check in scop_detection::can_represent_loop_1
> >> > >
> >> > >     && (niter = number_of_latch_executions (loop))
> >> > >     && !chrec_contains_undetermined (niter)
> >> > >
> >> > > that is supposed to filter out all these loops where this assert does not
> >> > > hold.
> >> > > The question is: why scop detection has not rejected this loop?
> >> > >
> >> > > Well, I see that we do not check that niter can be analyzed in the region:
> >> > > so we would need another check like this:
> >> > >
> >> > > diff --git a/gcc/graphite-scop-detection.c b/gcc/graphite-scop-detection.c
> >> > > index 3860693..8e14412 100644
> >> > > --- a/gcc/graphite-scop-detection.c
> >> > > +++ b/gcc/graphite-scop-detection.c
> >> > > @@ -931,6 +931,7 @@ scop_detection::can_represent_loop_1 (loop_p loop,
> >> > > sese_l scop)
> >> > >      && niter_desc.control.no_overflow
> >> > >      && (niter = number_of_latch_executions (loop))
> >> > >      && !chrec_contains_undetermined (niter)
> >> > > +    && !chrec_contains_undetermined (scalar_evolution_in_region (scop,
> >> > > loop, niter))
> >> > >      && graphite_can_represent_expr (scop, loop, niter);
> >> > >  }
> >> > >
> >> > > Could you please try this patch and see whether it fixes the problem?
> >>
> >> It doesn't.  It seems we test the above before the regions are
> >> eventually merged?  That is, the above enters with
> >>
> >> $46 = (const sese_l &) @0x7fffffffd6f0: {
> >>   entry = <edge 0x7ffff68af508 (6 -> 7)>,
> >>   exit = <edge 0x7ffff68af5b0 (7 -> 8)>}
> >>
> >> but the failing case with
> >>
> >> $15 = (const sese_l &) @0x298b420: {entry = <edge 0x7ffff68af738 (15 ->
> >> 3)>,
> >>   exit = <edge 0x7ffff68af930 (14 -> 15)>}
> >
> > Index: graphite-scop-detection.c
> > ===================================================================
> > --- graphite-scop-detection.c   (revision 245064)
> > +++ graphite-scop-detection.c   (working copy)
> > @@ -905,7 +905,9 @@ scop_detection::build_scop_breadth (sese
> >
> >    sese_l combined = merge_sese (s1, s2);
> >
> > -  if (combined)
> > +  if (combined
> > +      && loop_is_valid_in_scop (loop, combined)
> > +      && loop_is_valid_in_scop (loop->next, combined))
> 
> Looks good.  Thanks for the fix!

Applied as follows, bootstrapped & tested on x86_64-unknown-linux-gnu.

Richard.

2017-02-01  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/71824
	* graphite-scop-detection.c (scop_detection::build_scop_breadth):
	Verify the loops are valid in the merged SESE region.
	(scop_detection::can_represent_loop_1): Check analyzing the
	evolution of the number of iterations in the region succeeds.

	* gcc.dg/graphite/pr71824.c: New testcase.
diff mbox

Patch

Index: gcc/graphite-scop-detection.c
===================================================================
--- gcc/graphite-scop-detection.c	(revision 245064)
+++ gcc/graphite-scop-detection.c	(working copy)
@@ -905,7 +905,9 @@  scop_detection::build_scop_breadth (sese
 
   sese_l combined = merge_sese (s1, s2);
 
-  if (combined)
+  if (combined
+      && loop_is_valid_in_scop (loop, combined)
+      && loop_is_valid_in_scop (loop->next, combined))
     s1 = combined;
   else
     add_scop (s2);
@@ -931,6 +933,8 @@  scop_detection::can_represent_loop_1 (lo
     && niter_desc.control.no_overflow
     && (niter = number_of_latch_executions (loop))
     && !chrec_contains_undetermined (niter)
+    && !chrec_contains_undetermined (scalar_evolution_in_region (scop,
+								 loop, niter))
     && graphite_can_represent_expr (scop, loop, niter);
 }
 
Index: gcc/testsuite/gcc.dg/graphite/pr71824.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/pr71824.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/graphite/pr71824.c	(working copy)
@@ -0,0 +1,17 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -floop-nest-optimize" } */
+
+int a, b, d;
+int **c;
+int fn1() {
+    while (a)
+      if (d) {
+	  int e = -d;
+	  for (; b < e; b++)
+	    c[b] = &a;
+      } else {
+	  for (; b; b++)
+	    c[b] = &b;
+	  d = 0;
+      }
+}