-
Notifications
You must be signed in to change notification settings - Fork 28
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
(Closes #2716) Add support for module-inlining calls to polymorphic kernels/routines #2732
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2732 +/- ##
========================================
Coverage 99.86% 99.86%
========================================
Files 354 354
Lines 49010 49112 +102
========================================
+ Hits 48946 49048 +102
Misses 64 64 ☔ View full report in Codecov by Sentry. |
As well as addressing the coverage, I need to extend the LFRic integration tests to use this new functionality. |
Added the KernelModuleInlineTrans back into the LFRic transformation script and get a crash when trying to add the interface symbol into the symbol table. |
If you merge with master, I added multiple prints in the output, so you can grep and count the number of each cases that we could not get the kernel_schedule before due to polymorphic kernels, which was 331 |
The OMP offload LFRic integration test failed with an ICE:
It's seems odd that there's a EDIT: L85 of that file corresponds to the end of the (module-inlined?) |
Do the build manually for OpenACC (which doesn't give an ICE) and things are looking better: |
I need to figure out why we get an inlined routine without an offload directive added to it. The build log says:
|
Part of the problem was that the optimisation script wasn't checking that it hadn't already transformed a given kernel for a given invoke. Fixing that removes the 'failed to inline' messages but we still end up with an inlined kernel that doesn't have a directive added to it. |
The problem was that, having successfully module-inlined the kernel routine, we proceed to apply the annotation transformation to the original Kern and that does not update the Routine that has been inlined. It probably should? For now, I can search for the newly inlined routine and apply the transformation to that and we get the expected code. |
Now get a compilation failure:
This seems to be because we have successfully inlined this interface and its routines but subsequent PSy-layer subroutines are still importing it from a module. In turn, this is because I now check whether or not we've already transformed a kernel of a given name. Essentially, we need multiple LFRicKern objects to all point to the same bit of PSyIR. |
I think I made that change because, without copying, a routine gets removed from its original Container but that breaks any Interface that refers to it. This becomes a problem in the (unlikely) event that a routine is called directly as well as via an interface. |
If we have an Algorithm layer that contains two
This shouldn't happen. However, we also want to mark all kernels of the same name as being module-inlined and pointing to a single implementation. Since |
Thinking about this a bit more, I belatedly realise that if a given PSy layer routine calls the same kernel more than once then either all of them must be module inlined or none of them (unless we attempt to rename the inlined version and that gets complicated). I think the reason that LFRic is still not working is that we fail to inline a second instance of the same kernel (because the first instance has been transformed) but then we do proceed to transform it. Therefore, |
I've realised that The solution might therefore be as simple as not attempting to transform a Kernel that is already marked as module inlined. |
The trouble with the LFRic example is that we have two, separate |
No description provided.