Implement offload instructions via macros #80
Conversation
|
NB: the hpc ci tests are failing because of a permissions issue for PRs filed from forks. Once #78 is merged I can rebase on top of that to fix the failing hpc-ci. I have tested offline that it works. |
| set( Python3_FIND_VIRTUALENV STANDARD ) | ||
| find_package( Python3 COMPONENTS Interpreter ) | ||
|
|
||
| execute_process( COMMAND ${Python3_EXECUTABLE} -m pip --disable-pip-version-check install fypp OUTPUT_QUIET ) |
There was a problem hiding this comment.
Small comment from something we've hit very recently: Bare-metal Python installations may come without any pip or only ancient versions available. But they are guaranteed to have a bootstrap mechanism for it that ensures that you have at least a pip version equivalent to the ensurepip version packaged with the installation. For this, add the following:
| execute_process( COMMAND ${Python3_EXECUTABLE} -m pip --disable-pip-version-check install fypp OUTPUT_QUIET ) | |
| execute_process( COMMAND ${Python3_EXECUTABLE} -m ensurepip --upgrade OUTPUT_QUIET ) | |
| execute_process( COMMAND ${Python3_EXECUTABLE} -m pip --disable-pip-version-check install fypp OUTPUT_QUIET ) |
There was a problem hiding this comment.
I didn't even know we could have pip-less python, thanks!
|
|
||
| __all__ = ['NvidiaOpenACC'] | ||
|
|
||
| class NvidiaOpenACC(): |
There was a problem hiding this comment.
The idea of categorising backends by compiler-specific programming model implementations is a really good one, it allows to easily encode the minor differences in the interpretation of standards between vendors.
I would suggest to name them according to compiler toolchain, though, renaming this one e.g. to NVHPCOpenACC. This avoids also any issues associated with putting protected brand names like Nvidia into code.
There was a problem hiding this comment.
Yes I agree, thanks!
awnawab
force-pushed
the
naan-offload-macros
branch
from
8bfd956 to
9a18e00
Compare
… FIELD_RANKSUFF_DATA_MODULE
awnawab
force-pushed
the
naan-offload-macros
branch
from
9a18e00 to
aac718d
Compare
awnawab
changed the title
|
This is now ready for review, please have a look at your earliest convenience 🙏 |
There was a problem hiding this comment.
I agree we need something like this, but the code which was already difficult to understand, is becoming cryptic.
Would it be possible to translate OpenACC directives such as :
!$acc kernels present (PX)
Into :
$:offload_macros.kernels (present=['PX'])
So that it keeps its meaning in the NVIDIA lingo. I know it looks unfair for other vendors, but we have to choose a convention anyway.
|
Hi @pmarguinaud, Of course, if you think that makes the code more intuitive I am very happy to adapt it accordingly. Thanks for the feedback, I'll implement your suggestion 👍 |
A draft PR that represents a proof of concept of the development I proposed in #73.
The offload instructions in the code are replaced with macros, which are defined in python modules. More of these python modules can be added to support multiple backends, whilst leaving the code relatively clean. So far I have only implemented the macros for the code in
FIELD_RANKSUFF_DATA_MODULEthat would be run if OpenACC is enabled but not CUDA.The preprocessor definitions need to still be generalised. I envisage the following two:
WITH_GPU_OFFLOADcorresponding to_OPENACCandWITH_HICcorresponding to_CUDA.I am eager to start work (very) soon on an OpenMP offload backend, but before I write more code I would really appreciate your feedback on this.