Re: [PATCH, PR69110] Don't return NULL access_fns in dr_analyze_indices

[Tom de Vries <Tom_deVries@mentor.com>]

On 13/01/16 09:42, Richard Biener wrote:
> On Tue, 12 Jan 2016, Tom de Vries wrote:
>
>> On 12/01/16 14:04, Richard Biener wrote:
>>> On Tue, 12 Jan 2016, Tom de Vries wrote:
>>>
>>>> On 12/01/16 12:22, Richard Biener wrote:
>>>>> Doesnt' the same issue apply to
>>>>>
>>>>>>> unsigned int *p;
>>>>>>>
>>>>>>> static void __attribute__((noinline, noclone))
>>>>>>> foo (void)
>>>>>>> {
>>>>>>>     unsigned int z;
>>>>>>>
>>>>>>>     for (z = 0; z < N; ++z)
>>>>>>>       ++(*p);
>>>>>>> }
>>>>> thus when we have a MEM_REF[p_1]?  SCEV will not analyze
>>>>> its evolution to a POLYNOMIAL_CHREC and thus access_fns will
>>>>> be NULL again.
>>>>>
>>>>
>>>> I didn't manage to trigger this scenario, though I could probably make it
>>>> happen by modifying ftree-loop-im to work in one case (the load of the
>>>> value
>>>> of p) but not the other (the *p load and store).
>>>>
>>>>> I think avoiding a NULL access_fns is ok but it should be done
>>>>> unconditionally, not only for the DECL_P case.
>>>>
>>>> Ok, I'll retest and commit this patch.
>>>
>>> Please add a comment as well.
>>
>> Patch updated with comment.
>>
>> During testing however, I ran into two testsuite regressions:
>>
>> 1.
>>
>> -PASS: gfortran.dg/graphite/pr39516.f   -O  (test for excess errors)
>> +FAIL: gfortran.dg/graphite/pr39516.f   -O  (internal compiler error)
>> +FAIL: gfortran.dg/graphite/pr39516.f   -O  (test for excess errors)
>>
>> AFAIU, this is a duplicate of PR68976.
>>
>> Should I wait with committing the patch until PR68976 is fixed?
>
> Yes - we shouldn't introduce new testsuite regressions willingly at this
> point.
>

After r232659 (the fix for pr68692), the ICE no longer occurs.

>> 2.
>>
>> -XFAIL: gcc.dg/graphite/scop-pr66980.c scan-tree-dump-times graphite "number
>> of SCoPs: 1" 1
>> +XPASS: gcc.dg/graphite/scop-pr66980.c scan-tree-dump-times graphite "number
>> of SCoPs: 1" 1
>>
>> AFAIU, this is not a real regression, but the testcase needs to be updated.
>> I'm not sure how. Sebastian, perhaps you have an idea there?
>
> It looks like simply removing the xfail might be ok.  But the comment in
> the testcase doesn't suggest its dependency analysis fault that the
> situation is not handled so I'd like Sebastian to chime in (who also
> should know the dependence code very well).
>

Sebastian,

Ping on the xfail -> xpass issue mentioned above.

I'd like to commit this ( 
https://gcc.gnu.org/ml/gcc-patches/2016-01/msg00762.html  ) patch. I'm 
currently retesting using r232712 as baseline.

Thanks,
- Tom