On Tue, 31 Jan 2017, Sebastian Pop wrote:
> On Tue, Jan 31, 2017 at 9:11 AM, Richard Biener <rguent...@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 <seb...@gmail.com> wrote:
> >> > >
> >> > >
> >> > > On Tue, Jan 31, 2017 at 7:49 AM, Richard Biener <rguent...@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 <rguent...@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.
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;
+ }
+}
