PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

Inlining in GOcean

Open hiker opened this issue 1 year ago • 2 comments

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
  3. 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.

hiker avatar Sep 03 '24 01:09 hiker

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.

LonelyCat124 avatar Sep 03 '24 08:09 LonelyCat124

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.

arporter avatar Sep 03 '24 08:09 arporter