Enable inlining of a subroutine with an array subrange in an argument #484
Conversation
|
Documentation for this branch can be viewed at https://sites.ecmwf.int/docs/loki/484/index.html |
awnawab
changed the title
awnawab
force-pushed
the
naan-inline-array-arg
branch
from
0bbdfb3 to
2945b63
Compare
awnawab
force-pushed
the
naan-inline-array-arg
branch
from
2945b63 to
a341e11
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #484 +/- ##
=======================================
Coverage 96.17% 96.17%
=======================================
Files 224 224
Lines 40386 40371 -15
=======================================
- Hits 38842 38828 -14
+ Misses 1544 1543 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| real(kind=8) :: tmp(m) | ||
| integer :: i | ||
|
|
||
| tmp(1:m) = x | ||
| do i=1, n | ||
| a(i) = b(i) + sum(tmp) | ||
| end do | ||
| do i=1,4 | ||
| c(i) = 0. |
There was a problem hiding this comment.
Could we add an assignment d(i) = 1. here too, to check that this index is shifted too?
| use BNDS_module, only: n, m | ||
| use another_module, only: x | ||
|
|
||
| real(kind=8), intent(inout) :: a(n), b(n) | ||
| real(kind=8), intent(out) :: c(4), d(4) |
There was a problem hiding this comment.
What happens to real, intent(inout) :: e(0:3) in the two cases below? Could we add a test for this too, just to cover our bases?
There was a problem hiding this comment.
That was indeed a corner case I'd missed, thanks!
| new_dimensions[indices[index]] = dim | ||
| # if the argument contains an array range, we must map the bounds accordingly | ||
| if isinstance(val.dimensions[index], sym.Range) and (lower := val.dimensions[index].lower): | ||
| if isinstance(dim, sym.Range): |
There was a problem hiding this comment.
[no action] Note to self, there's a similar index-shifting problem in the associate resolver. We could possibly re-use a shared utility for this if we were to externalise this? This is beyond this PR tho.
awnawab
force-pushed
the
naan-inline-array-arg
branch
from
c8f3482 to
8ab97d7
Compare
awnawab
force-pushed
the
naan-inline-array-arg
branch
from
8ab97d7 to
98ca42b
Compare
|
Please wait to merge this, I think I've missed the edge case where we are passing a subrange of a dimension and there are different lbounds for the argument and dummy. I'll confirm today and push a fix + test if necessary. |
A small fix to the inlining trafo that enables inlining of calls where we pass a subrange of an array argument, i.e.: