#2723 Add some support for dependency analysis involving array ranges.

[hiker]

Fixes #2723. I am not sure if more support is needed or would be useful (e.g. I could check if two ranges have the same stride, but accesses different elements, ...).

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.89%. Comparing base (06e0257) to head (4aeae63).
Report is 3 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2880   +/-   ##
=======================================
  Coverage   99.89%   99.89%           
=======================================
  Files         359      359           
  Lines       50997    51011   +14     
=======================================
+ Hits        50942    50956   +14     
  Misses         55       55           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[hiker]

CI was green. Note that I have some typos in the issue number - I mixed up 2878 and 2723, so e.g. the CI shows 2878, not 2723 :(

[sergisiso]

Thanks to tackling this @hiker, the implementation, docstring and tests all look great. The only comment I have is that when looking for TODOs in the same file I found a reference to #2168 (L401) which I am wondering if it's the same issue and can now also be fixed/closed. But I will approve regardless. Let me know what you think.

[hiker]

Thanks to tackling this @hiker, the implementation, docstring and tests all look great. The only comment I have is that when looking for TODOs in the same file I found a reference to #2168 (L401) which I am wondering if it's the same issue and can now also be fixed/closed. But I will approve regardless. Let me know what you think.

I had a look. That issue is concerned about array-ranges that involve the loop variable that is to be parallelised, e.g.:

do i=1, n
   a(2*i:2*i+1)  = 0
enddo

(stupid example, but you get the idea). The moment the loop variable is involved, this new range comparison is not called at all. I looked if we could simply call this new function here:

Now we could do a similar test to the rest of 'one index that depends on the looppart of the dependency analysis: if there is an overlap between the array ranges for two loop iterationsi1andi2the loop cannot be parallelised. Following the current implementation, that is done by computingd=i2-i1, the distance between the loop iterations for which the indices would overlap. If this has only 0 as solution for d`, the loops can be parallelised, because there exist no other loop iteration that accesses the same range.

That leads to a systems of inequalities (I hope that's the right word), which simplifies to:
-0.5 <=d <= 0.5
So, no integer solution except 0 -> the loop can be parallelised. But you can see that this is not at all trivial, and does require more work than what we now have (atm any array range with the loop index will just be rejected).

So, #2168 is not fixed here. But if you agree that array ranges depending on loop indices do not happen in any realistic code we need to handle (or will in general be not parallelisable anyway, so the current behaviour of rejecting it is good enough), we can close #2168, and replace the #TODO with just a comment for 'the future in case we need to look at it again'?

[sergisiso]

@hiker I am ok with keeping the issue, I just wanted to check if it was something easy to solve with the new functionality but I agree is not. This was my only comment, this is ready to merge.