Re: [PATCH] Fix PR71824

[Sebastian Pop <sebpop at gmail dot com>]

On Tue, Jan 31, 2017 at 9:06 AM, Richard Biener <rguenther@suse.de> wrote:
> On Tue, 31 Jan 2017, Sebastian Pop 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?

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.