Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
7 tasks
sergisiso opened this issue Aug 3, 2022 · 7 comments
Open
7 tasks
Labels
bug LFRic Issue relates to the LFRic domain

Comments

@sergisiso
Copy link
Collaborator

sergisiso commented Aug 3, 2022

Updated March 2024, the current stats are:

Inline transformations successful 502
Inline transformations failed 139:

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 sergisiso added bug LFRic Issue relates to the LFRic domain labels Aug 3, 2022
sergisiso added a commit that referenced this issue Aug 17, 2022
@sergisiso sergisiso changed the title KernelInline does not consider name clashes from different Invokes ModuleInline does not consider name clashes from different Invokes Aug 17, 2022
@sergisiso
Copy link
Collaborator Author

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?

@arporter
Copy link
Member

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.

@sergisiso
Copy link
Collaborator Author

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.

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

rupertford added a commit that referenced this issue Sep 8, 2022
(towards #1823) Fix LFRic ModuleInline issues with repeated kernels
@sergisiso sergisiso changed the title ModuleInline does not consider name clashes from different Invokes ModuleInline issues in LFRic Sep 21, 2022
sergisiso added a commit that referenced this issue Sep 22, 2022
sergisiso added a commit that referenced this issue Sep 30, 2022
sergisiso added a commit that referenced this issue Oct 5, 2022
sergisiso added a commit that referenced this issue Oct 7, 2022
arporter added a commit that referenced this issue Oct 19, 2022
(towards #1823) refactor KernelModuleInlineTrans
@sergisiso
Copy link
Collaborator Author

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

@sergisiso sergisiso changed the title ModuleInline issues in LFRic Meta-issue tracking LFRic module-inline and intrinsics-inlining issues Mar 20, 2024
@sergisiso
Copy link
Collaborator Author

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

@arporter
Copy link
Member

As of October 2024, Sergi has produced a nice Sankey diagram summarising the situation:
image

@arporter
Copy link
Member

In my work on #2716, I have (belatedly) realised that the Kern.module_inline setter sets this flag for all Kernels with the same name in a given InvokeSchedule. There is a TODO there that refers to this issue and talks about giving the transformation more control over this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug LFRic Issue relates to the LFRic domain
Projects
None yet
Development

No branches or pull requests

2 participants