search

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
104 stars 28 forks source link

Inlining in GOcean #2705

Open hiker opened 3 weeks ago

hiker commented 3 weeks ago

The full inlining does not yet work with GOcean based code. Here the issues I have identified so far: 1) GOcean imports field mod without listing r2d_field, therefore the field type is not known, and validation of inlining aborts. It might be that just importing r2d_field explicitly (and whatever else is needed) explicitly would fix this. 2) field_r2d is declared as an unresolved type in gocean1p0.py: https://github.com/stfc/PSyclone/blob/92fbf609e448f750dd2b420db7ac29d12a66c0c4/src/psyclone/gocean1p0.py#L1498-L1500 2) There are two import statements added inGOPsy: the constructor adds it to the PSy-layer symbol table (https://github.com/stfc/PSyclone/blob/92fbf609e448f750dd2b420db7ac29d12a66c0c4/src/psyclone/gocean1p0.py#L100-L102), and the gen code (https://github.com/stfc/PSyclone/blob/92fbf609e448f750dd2b420db7ac29d12a66c0c4/src/psyclone/gocean1p0.py#L123) adds this as well. I believe atm the gen code is responsible for the use statement that turns up in the output psy-layer file, but the symbol table is used to avoid name clashes.

When I tried adding a proper declaration to the module symbol table, the code mentioned in item 2 would create a new symbol avoiding the name clash with r2d_field. I realise now that this was likely because I created the symbol without a tag :(

I also tried to disable the validation, and while them inlining worked, the backend visitor crashed because it hit the undefined symbol r2d_field.

I could get the inlining to work after some hacky changes to gocean.py - adding the r2d_field symbol and fixing up the renaming (due to the tag I missed I believe), so there doesn't appear to be anything mayor missing.

LonelyCat124 commented 3 weeks ago

Yeah I hit the same thing with the tasking work (where i just disabled most validation) - my suggestion for this from discussion with @arporter was #2415 and use the domain knowledge to handle the r2d_field imports and just know what they are.

arporter commented 3 weeks ago

Oh yes, I'd forgotten we'd been here before! I suspect that the type of r2d_field extends another type and we don't support that although I think @JulienRemy was looking at this.