stfc / PSyclone

Domain-specific compiler and code transformation system for Finite Difference/Volume/Element Earth-system models in Fortran
BSD 3-Clause "New" or "Revised" License
107 stars 29 forks source link

Do we need support to remove public/private in kernel extraction driver? #2580

Open hiker opened 6 months ago

hiker commented 6 months ago

The original code to remove protected/private variables in inlined modules was very fragile. It relied only on regex to replace the terms, meaning if a code should contain a symbol called 'private' it would be incorrectly removed.

The main question is, do we need to support this in the first place? The proper solution to this issue is that in case of compilation for kernel-extraction any source code (at least the one used directly or indirectly by a driver) should have all protected/private attributes removed, to allow the extraction to write the symbol, and the driver to initialise these variables from the kernel data file.

A specific-solution for the driver only could be done, but that only makes sense for a program that uses only(!)protected attributes (not private), and does not (cannot??) want to remove these attributes in case of using the driver extraction (if a program uses private, the extraction won't work).

I will be adding a script to fparser that can be used to remove attributes.

Now there might be another option we could consider, but I think it might not be worth the effort (given that it is only useful for codes that use protected, but not private):

We could add support to fparser (or PSyclone) to run an optional script on any parsed file before using the fparser tree in PSyclone. This feature could then be used to remove private/protected attributes. BUT, that of course does only affect the driver (and any source code that is processed & modified by PSyclone.

So, imho we can just close this issue, but @arporter, let me know if you see anything useful in adding this feature.

arporter commented 6 months ago

If you're happy to close it @hiker then so am I as it's primarily the kernel extraction that needs it. The only thing I don't understand is how you're proposing to do it as the problem is still there?

hiker commented 6 months ago

I'll be providing the script to remove protected/private in fparser (as example I'd guess). Then it's up to the build system.

While I don't think it should be integrated into the FAB (too limited use case to make it part of a somewhat generic build tool), I will include it as an example in the LFRic build system (which will be using on FAB, i.e. it will introduce a separate step where it changes all files).

Let's not think about the cost of parsing all files with fparser once more (remove attributes, then psyclone, then the FAB analysis step). We are looking at options to share fparser trees. The main problem is that all these steps are done in child processes, so we need to bring them back to the master process to be cached (module manager). And we need to check if forking indeed allows the child processes to use the cached data.