Utility transformation for creating standalone subroutines from contained subroutines #181

skarppinen · 2023-10-25T13:37:38Z

This PR adds a function named lift_contained_subroutines (for a lack of a better name) which processes the contained subroutines of a loki.Subroutine such that they are converted to "stand alone subroutines" that have no global dependencies. To do this:

all global bindings from the point of view of the contained subroutine(s) are introduced as imports or dummy arguments to the contained subroutine(s).
all calls to the contained subroutines in the parent are modified accordingly.

To understand the basic idea of what the function does, consider the next example.
The following subroutine (with a contained subroutine "inner" in the CONTAINS block)

subroutine outer()
    integer :: y
    integer :: o
    o = 0
    y = 1
    call inner(o)
    contains
    subroutine inner(o)
       integer, intent(inout) :: o
       integer :: x
       x = 4
       o = x + y ! Note, 'y' is "global" here!
    end subroutine inner
end subroutine outer

is transformed into a list of loki.Subroutines, where the transformed "parent" (i.e "outer") comes first:

subroutine outer()
    integer :: y
    integer :: o
    o = 0
    y = 1
    call inner(o, y) ! 'y' now passed as argument.
    contains
end subroutine outer

and the transformed children (i.e in this case only "inner") come next:

subroutine inner(o, y)
       integer, intent(inout) :: o
       integer, intent(inout) :: y
       integer :: x
       x = 4
       o = x + y ! Note, 'y' is no longer "global"
end subroutine inner

Naturally, multiple contained subroutines are supported as well.

Some remarks:

The current implementation always sets the intent of "resolved variables" (i.e 'y' in the above example) as "inout", unless the variable to be resolved has an explicitly specified intent in the parent routine ('outer').
If a variable is to be resolved that lacks a definition in the parent scope, an exception is thrown.
If a variable to be resolved is a member of a derived type (say 'a%b'), the whole derived type (i.e 'a') is introduced as a dummy argument to the contained subroutine.
If a variable to be resolved is defined via an import, the import is added to the contained subroutine.
If the definition of a variable to be resolved depends on imports or other variables, the necessary imports are added to the contained subroutine, and the variables are added as arguments (if need be). These cases occur for example, when the variable to be resolved (below 'x') is defined for example by:

USE parkind1, only: jprb, jpim
INTEGER(KIND=jpim) :: klon
REAL(KIND=jprb) :: x(klon) ! to resolve 'x', import 'jprb' and introduce 'klon' as argument.

resolve_associates is called for each contained subroutine.

The tests cover the above cases (excluding resolving associates) and some other things (see docstring for each test). As additional "field testing", I have tested this (as a replacement for inlining) in the ACRANEB2 dwarf, where as part of an SCC pipeline this transform handles all Fortran constructs found there and produces code that compiles and provides correct results on NVIDIA GPUs.

…still

…utines from contains but leave everything else

codecov · 2023-11-09T13:43:52Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (955a9da) 92.21% compared to head (fa1dd79) 92.24%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #181      +/-   ##
==========================================
+ Coverage   92.21%   92.24%   +0.02%     
==========================================
  Files          93       94       +1     
  Lines       16839    16903      +64     
==========================================
+ Hits        15528    15592      +64     
  Misses       1311     1311

Flag	Coverage Δ
lint_rules	`96.22% <ø> (ø)`
loki	`92.22% <100.00%> (+0.03%)`	⬆️
transformations	`91.44% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

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

reuterbal

First of all, thanks for this contribution and apologies for the long time I took to review this. This does look very useful and I'm keen to bring this in.

There are a few formal code checks that failed, such as copyright headers and pylint warnings, which I hope you could take care of.

I have also left a few comments already for things that I think could be improved. Generally, you have re-implemented frequently some functionality that is already provided via convenience API and I would encourage you to use that whenever possible, since it will make maintenance and compatibility of your utility easier in the long run by relying on concepts abstracted from IR implementation details. Also, by splitting the utility we would gain composable functionality and a cleaner control flow. I have marked this accordingly in the code.

Feel free to ask questions, otherwise please ping me once you think this is ready for another look, at which stage I'll also pay a bit more attention to the details of the symbol resolution of the lifted routine (which I have only skimmed at this stage).

Thanks again for this contribution, and as a final request: please add your name to AUTHORS.md as part of this PR ;-)