stfc / PSyclone

Domain-specific compiler and code transformation system for Finite Difference/Volume/Element Earth-system models in Fortran
BSD 3-Clause "New" or "Revised" License
107 stars 28 forks source link

[GOcean] Single Iteration Loops #129

Open hiker opened 6 years ago

hiker commented 6 years ago

I am just adding several new iteration spaces for the halos in the gocean API (in a kind of private branch since I assume the halo-north/west/east/south only spaces I need are not of general interest). While implementing them I noticed that the Loop creation code in psyGen() does only partially support eliminating loops with one iteration only:

if self._start == "1" and self._stop == "1":  # no need for a loop
    for child in self.children:
        child.gen_code(parent)

This code only works for a loop that have the value 1, but not e.g. for a loop:

do i=x_max, x_max

and also it does not create a variable and assign it the correct value, e.g. atm it would create:

! No assignment to j (unless a value from a previous loop)
DO i=1,istop+1
    CALL swlon_east_code(i, j, ua%data, ...)    ! j is undefined
END DO 

In my branch I've fixed this code so it can now create code like:

j = y_max
DO i=1,istop+1
    CALL swlon_east_code(i, j, ua%data, ...)
END DO 

Would that be of interest to 'general' PSyclone? I am happy to make this a separate pull request, otherwise just close it ;)

hiker commented 6 years ago

For the record, the following patch achieves that: https://github.com/hiker/PSyclone/commit/5e31dc90bfb68d8f8255399a20cf07b72671a99c Am happy to turn this into a pull request (which means to also update tests and add some comments)

arporter commented 6 years ago

Hi @hiker, if we're generating (or could generate) incorrect code then we do need to fix it so please put in a PR for that.

re. the N/S/E/W-only iteration spaces I suspect that they probably are generally useful. Maybe something to discuss with @rupertford when you see him next week?

hiker commented 6 years ago

OK, will do.

Re 1d iteration spaces: yes, I will discuss with @rupertford (they are working fine already for me) , but tbh, #109 (making it easier to add iteration spaces) would be a better solution - though more difficult to implement.

arporter commented 5 years ago

Hi @hiker. I'm just doing some Issue cleaning and found this one. Is the necessary functionality now on master (and can I close this)?

hiker commented 5 years ago

I don't have time atm to chase the details, but I believe psyGen's Loop class still contains:

 def gen_code(self, parent):
        '''Generate the fortran Loop and any associated code '''
       ...
        if self._start == "1" and self._stop == "1":  # no need for a loop
            for child in self.children:
                child.gen_code(parent)

Look at the patch I refer above. Afaik it could just be merged in, but needs test cases, and I won't have time for the next 2.5 weeks to do that :( But for now please do not close - the code is still buggy (a "do i=1,1" loop would not be created, so i is not initialised, but would be used when calling the actual kernel).