PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

[PSyData] read/write access computation misses accesses in declaration

Open hiker opened this issue 1 year ago • 4 comments

The UM physics contain for example:

real ::  k_blend_tq(pdims%i_start:pdims%i_end,pdims%j_start:pdims%j_end), &
                                   ! IN Theta level for blending height.

The kernel extraction misses the accesses to pdims, therefore the arrays in the driver will not be declared of the correct size, causing undefined behaviour of the driver.

hiker avatar Jan 17 '24 23:01 hiker

If this uses the reference_access method, this could be the same issue as #2271

sergisiso avatar Jan 18 '24 09:01 sergisiso

If this uses the reference_access method, this could be the same issue as #2271

I had a look, and I am not sure about the full scope of #2271, but I think I can provide a solid base to be used: if I add a reference_access method to the symbol table, which (initially, since this is all I need) returns all variables 'read':

  1. array dimension declarations, and
  2. Initial values of variables. I know these are technically not reads, since they are executed at link time by initialising the static memory. But I need them for the driver, and we can sort out what exactly is useful otherwise in #2271 and provide options to select what the user wants later.

Oh, can you think of any other constructs contained in the symbol table that I should look at and am just forgetting?

For #2271 we can then just use symbol_table.reference_accesses in the ScopingNode with hopefully only minor improvements.

hiker avatar Jan 19 '24 11:01 hiker

if I add a reference_access method to the symbol table

I was expecting it to be all implemented in ScopedNode, since Node has the reference_access method that needs to be overloaded. I think the logic about that is "referenced" is in the tree while what "exists" is in the symbol_table, although in this case is similar. Is there any reason you need it to be in the symbol table?

(initially, since this is all I need) returns all variables 'read'. [...] I know these are technically not reads, since they are executed at link time by initialising the static memory

I was also wondering about this, but I am fine with this being READ first and refined in a later PR if we need.

sergisiso avatar Jan 19 '24 12:01 sergisiso

if I add a reference_access method to the symbol table

I was expecting it to be all implemented in ScopedNode, since Node has the reference_access method that needs to be overloaded. I think the logic about that is "referenced" is in the tree while what "exists" is in the symbol_table, although in this case is similar. Is there any reason you need it to be in the symbol table?

I thought it would be easier for you :) I implement it (for development) in Routine (and move it later ;) ). There are quite a few options (e.g. Symboltable could have the loop ofer all symbols, and we call a new reference_accesses in the symbol. Or, we can do it all in Routine/ScopingNode ... ). I couldn't decide what's best, but am happy not to move this into symbol table. I don't know if we will ever have a case of having a symbol table, but not a ScopingNode :)

Having it all in Routine/ScopingNode just means that we have some details about the DataTypeSymbol and SymbolTable in the Routine (well, all following an API, so that's fine ... we need the list of all symbols, and the shape and initial value of a symbol).

Actually, the more I think about it, I believe you are right, ScopingNode is the way to go.

(initially, since this is all I need) returns all variables 'read'. [...] I know these are technically not reads, since they are executed at link time by initialising the static memory

I was also wondering about this, but I am fine with this being READ first and refined in a later PR if we need.

Great!

hiker avatar Jan 19 '24 12:01 hiker