This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR71824
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!
Sebastian
> 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.