PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

Improve and use whole routine reference_accesses

Open sergisiso opened this issue 2 years ago • 3 comments

Some transformations have ad-hoc implementations to find all the symbols "used" in a routine, e.g. the two inline transformations (src/psyclone/domain/common/transformations/kernel_module_inline_trans.py line 184) and (src/psyclone/psyir/transformations/inline_trans.py line 678) and maybe others.

These are very error-prone and I wanted to centralise this logic in a utility, but then I realize that "reference_accesses" is probably already what we want, but there is no specialisation of it for routines/containers/scoping_nodes yet.

What refence access for a whole routine should check is, in addition to check the reference_access of the children:

  • ScopingNodes should report on all its declarations.
  • ScopingNodes should report on all its declaration initial values.
  • ScopingNodes should report on all its declaration shapes.
  • ScopingNodes should report on all its datatype declaration initial values.
  • Literal should report on its "literal.datatype.precision"
  • Caller should report on its caller.routine
  • Codeblocks should report on the symbols listed in cblock.get_symbol_names

Then we could replace the ad-hoc implementation on both inline transformations.

Other requirements for this are that:

  • There are situations like #2314 when we need smaller granularity than ScopingNode reference access of symbols touched in the symbol declaration. So maybe the symbol needs to provide this and ScopingNode just request it for the relevant symbols.
  • Some "references" are just touched at compile-time/static which is something different than a READ dependency. I see that the references_access options allow for a "COLLECT-ARRAY-SHAPE-READS" already, but maybe we need something more generic for static reads.

sergisiso avatar Aug 16 '23 10:08 sergisiso

Thanks for this one Sergi. I noticed today that ACCRegionDirective has signatures which should also be replaced somehow with reference_accesses: https://github.com/stfc/PSyclone/blob/433d31bdceb199754e2a260716e749ec7a4222f2/src/psyclone/psyir/nodes/acc_directives.py#L97-L128

arporter avatar Aug 16 '23 11:08 arporter

#2278 could also be considered a part of this.

arporter avatar Aug 16 '23 15:08 arporter

When @arporter 's #2366 is merged, there will be symbols with import interface and initial values.

This should be excluded from the reference_accesses of the routine, as they are in reallity in another module, e.g.:

module mymodule
   use other1
   use other2, only: other_symbol2
   integer, parameter :: a = other_symbol1 + other_symbol2
end module mymodule

module test
  subroutine my_sub
     use mymodule, only: a
  end subroutine my_sub
end module test

After resolving the imports of my_sub, that symbol table will have an 'a' datasymbol with initial_value references to other_symbol1 and other_symbol2. These are in fact not accessed during the execution of the routine as they are static and handled at compile time.

sergisiso avatar Oct 26 '23 13:10 sergisiso

This has improved a lot with #2825 because the reference_access now report most symbols. But there are a few TODOs remaining in order to close this issue. I believe most are regarding actually using the new functionality.

sergisiso avatar Jan 23 '25 12:01 sergisiso