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))
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);
}
seems to fix it.
Richard.
