On Tue, Jan 31, 2017 at 9:06 AM, Richard Biener <rguent...@suse.de> wrote: > On Tue, 31 Jan 2017, Sebastian Pop 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?
We are supposed to verify that the merged regions are still a valid scop. So we are probably missing in merge_sese an extra call to verify that all the loops can be represented in the combined region.
