On Fri, Aug 5, 2016 at 3:28 PM, Yuri Rumyantsev <ysrum...@gmail.com> wrote:
> Richard,
>
> Here is updated patch which implements your proposal - I pass loop
> instead of stmt to determine either REF is defined inside LOOP nest or
> not. I checked that for pr70729-nest.cc the reference this->S_n for
> statements which are out of innermost loop are not considered as
> independent as you pointed out.
>
> Regression testing did not show any new failures and both failed tests
> from libgomp.fortran suite now passed.
>
> Is it OK for trunk?
I don't quite understand
+ /* Ignore dependence for loops having greater safelen. */
+ if (new_safelen == safelen
+ && bitmap_bit_p (&ref->dep_loop, LOOP_DEP_BIT (loop->num, stored_p)))
return false;
this seems to suggest (correctly I think) that we cannot rely on the caching
for safelen, neither for optimal results (you seem to address that) but also
not for correctness (we cache the no-dep result from a safelen run and
then happily re-use that info for a ref that is not safe for safelen).
It seems to me we need to avoid any caching if we made things independent
because of safelen and simply not do the dep test afterwards. this means
inlining ref_indep_loop_p_1 partly into _2 (not sure if there's a great way
to do this w/o confusing the control flow).
Richard.
> ChangeLog:
> 2016-08-05 Yuri Rumyantsev <ysrum...@gmail.com>
>
> PR tree-optimization/71734
> * tree-ssa-loop-im.c (ref_indep_loop_p): Add new argument REF_LOOP.
> (outermost_indep_loop): Pass LOOP argumnet where REF was defined to
> ref_indep_loop_p.
> (ref_indep_loop_p_1): Fix commentary.
> (ref_indep_loop_p_2): Add additional argument REF_LOOP, introduce new
> variable NEW_SAFELEN which may have new value for SAFELEN, ignore
> dependencde for loop having greater safelen value, pass REF_LOOP in
> recursive call.
> (can_sm_ref_p): Pass LOOP as additional argument to ref_indep_loop_p.
>
> 2016-08-03 16:44 GMT+03:00 Richard Biener <richard.guent...@gmail.com>:
>> On Fri, Jul 29, 2016 at 4:00 PM, Yuri Rumyantsev <ysrum...@gmail.com> wrote:
>>> Hi Richard.
>>>
>>> It turned out that the fix proposed by you does not work for liggomp
>>> tests simd3 and simd4.
>>> The reason is that we can't change safelen value for references not
>>> defined inside loop. So I add missed check on it to patch.
>>> Is it OK for trunk?
>>
>> Hmm, I don't like the walk of all subloops in ref_defined_in_loop_p as
>> that operation can end up being quadratic in the loop depth/width.
>>
>> But I also wonder about correctness given that LIM "commons"
>> references. So we can have
>>
>> for (;;)
>> .. = ref; (1)
>> for (;;) // safelen == 2 (2)
>> ... = ref;
>>
>> and when looking at the ref at (1) which according to you should not
>> have safelen applied your function will happily return that ref is defined
>> in the inner loop.
>>
>> So it looks like to be able to apply safelen the caller of ref_indep_loop_p
>> needs to pass down a ref plus a location (a stmt). In which case your
>> function can simply use flow_loop_nested_p (loop, gimple_bb
>> (stmt)->loop_father);
>>
>> Richard.
>>
>>> ChangeLog:
>>> 2016-07-29 Yuri Rumyantsev <ysrum...@gmail.com>
>>>
>>> PR tree-optimization/71734
>>> * tree-ssa-loop-im.c (ref_defined_in_loop_p): New function.
>>> (ref_indep_loop_p_2): Change SAFELEN value for REF defined inside LOOP.
>>>
>>> 2016-07-29 13:08 GMT+03:00 Yuri Rumyantsev <ysrum...@gmail.com>:
>>>> Sorry H.J.
>>>>
>>>> I checked both these tests manually but forgot to pass "-fopenmp" option.
>>>> I will fix the issue asap.
>>>>
>>>> 2016-07-29 0:33 GMT+03:00 H.J. Lu <hjl.to...@gmail.com>:
>>>>> On Thu, Jul 28, 2016 at 6:49 AM, Yuri Rumyantsev <ysrum...@gmail.com>
>>>>> wrote:
>>>>>> Richard,
>>>>>>
>>>>>> I prepare a patch which is based on yours. New test is also included.
>>>>>> Bootstrapping and regression testing did not show any new failures.
>>>>>> Is it OK for trunk?
>>>>>>
>>>>>> Thanks.
>>>>>> ChangeLog:
>>>>>> 2016-07-28 Yuri Rumyantsev <ysrum...@gmail.com>
>>>>>>
>>>>>> PR tree-optimization/71734
>>>>>> * tree-ssa-loop-im.c (ref_indep_loop_p_1): Pass value of safelen
>>>>>> attribute instead of REF_LOOP and use it.
>>>>>> (ref_indep_loop_p_2): Use SAFELEN argument instead of REF_LOOP and
>>>>>> set it for Loops having non-zero safelen attribute.
>>>>>> (ref_indep_loop_p): Pass zero as initial value for safelen.
>>>>>> gcc/testsuite/ChangeLog:
>>>>>> * g++.dg/vect/pr70729-nest.cc: New test.
>>>>>>
>>>>>
>>>>> Does this cause
>>>>>
>>>>> FAIL: libgomp.fortran/pr71734-1.f90 -O3 -fomit-frame-pointer
>>>>> -funroll-loops -fpeel-loops -ftracer -finline-functions execution
>>>>> test
>>>>> FAIL: libgomp.fortran/pr71734-1.f90 -O3 -g execution test
>>>>> FAIL: libgomp.fortran/pr71734-2.f90 -O3 -fomit-frame-pointer
>>>>> -funroll-loops -fpeel-loops -ftracer -finline-functions execution
>>>>> test
>>>>> FAIL: libgomp.fortran/pr71734-2.f90 -O3 -g execution test
>>>>>
>>>>> on AVX machines and
>>>>>
>>>>> FAIL: libgomp.fortran/simd3.f90 -O3 -fomit-frame-pointer
>>>>> -funroll-loops -fpeel-loops -ftracer -finline-functions execution
>>>>> test
>>>>> FAIL: libgomp.fortran/simd3.f90 -O3 -g execution test
>>>>> FAIL: libgomp.fortran/simd4.f90 -O3 -fomit-frame-pointer
>>>>> -funroll-loops -fpeel-loops -ftracer -finline-functions execution
>>>>> test
>>>>> FAIL: libgomp.fortran/simd4.f90 -O3 -g execution test
>>>>>
>>>>> on non-AVX machines?
>>>>>
>>>>> --
>>>>> H.J.
