PSyclone icon indicating copy to clipboard operation
PSyclone copied to clipboard

Meta-issue tracking LFRic module-inline and intrinsics-inlining issues

Open sergisiso opened this issue 1 year ago • 5 comments

Updated March 2024, the current stats are:

Inline transformations successful 502 Inline transformations failed 139:

  • [ ] 90 because another, different, subroutine with the same name already exists (could be that routine eq is lacking)
  • [ ] 30 TODO #928
  • [ ] 11 Kernel calls another local-scoped subroutine
  • [ ] 7 Failed to find a kernel implementation with an interface that matches

Inline MATMUL intrinsic successful 64 Inline MATMUL intrinsic failed 73

  • [ ] 59 is not the sole operation on the rhs of an assignment.
  • [ ] 9 result and operands of MATMUL IntrinsicCall are not plain references
  • [ ] 2 Must have full type information for result and operands

sergisiso avatar Aug 03 '22 13:08 sergisiso

I started #1844 to fix these issues. I could deal with the duplicated declarations when the same kernel is in multiple invokes in the same file and a more subtle issues I found when psy.gen is called multiple times.

But I need your opinion how to solve issues with missing symbols inside the inlined routines. The problem is when the kernel code uses a globally imported variable. e.g.:

module dg_inc_matrix_vector_kernel_mod
     use constants_mod,           only : i_def, r_single, r_double
     ....
contains
    subroutine dg_inc_matrix_vector_code_r_single(cell, ...)
       ...
       real(kind=r_single), dimension(undf2),              intent(in)    :: x
       ...

When this routine is inlined the r_single is undeclared in the _psy.f90 file. There is a rule/check that no global variables are used inside inlined kernels but this is just checking the code and not the declarations kinds. I could:

  1. Always import r_single and r_double in the psy module. This will solve it for my case but will potentially fail for some other in the future.
  2. Check the imported symbols and bring them to the psy module. But I am not sure if having no global symbols in kernel code is a PSyKAl rule that makes ensuring the kernel is thread_safe easier to verify. Although kind symbols are safe. So should I made and exception for them?
  3. Make the PSyclone check fail for this kernels because of the global import. Then they will need to be written as:
module dg_inc_matrix_vector_kernel_mod
     
     ....
contains
    subroutine dg_inc_matrix_vector_code_r_single(cell, ...)
       use constants_mod,           only : i_def, r_single, r_double
       ...
       real(kind=r_single), dimension(undf2),              intent(in)    :: x
       ...

this will require updating a few LFRic kernel code.

@rupertford @arporter @TeranIvy what do you think?

sergisiso avatar Aug 17 '22 11:08 sergisiso

I think we should allow/make an exception for kind parameters. In #924 I copy any imports over to the call site, even if they are in module scope.

arporter avatar Aug 17 '22 20:08 arporter

I think we should allow/make an exception for kind parameters. In https://github.com/stfc/PSyclone/issues/924 I copy any imports over to the call site, even if they are in module scope.

Ok, I will copy to psy the symbols used in the kind parameter.

sergisiso avatar Aug 18 '22 08:08 sergisiso

To close this issue by moduleinlining the majority of LFric kernels we need:

  • [ ] (Issue #1824) Failed to retrieve with get_kernel_schedule - can be the metadata validation that is failing (291 kernel calls)
  • [ ] Improve routine equality check (19 kernel calls)
  • [ ] Allow to module-inline routines with globals. We just need to maintain the reference to the shared global variable and not replicate the variable (43 kernel calls)
  • [ ] Needs interface renaming (156 kernel calls)

Note that multiple kernel calls have more than one listed problem, so just fixing one issue does not guarantee successful inlining.

sergisiso avatar Nov 04 '22 13:11 sergisiso

Remaining issues after merging #1968 gravitywave_not_inlined.txt gungho_not_inlined.txt

sergisiso avatar Dec 07 '22 13:12 sergisiso

I updated the issue title and description with the current inlining stats for gungho_model LFRic miniapp

sergisiso avatar Mar 20 '24 11:03 sergisiso