Add GPU awareness to MPL using IGNORE_TKR directives#105
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces NVHPC-specific IGNORE_TKR-based directives to allow MPL MPI wrappers to accept device-resident buffers (for GPU-aware MPI), and refactors several MPL routines into Fortran submodules to host the implementations.
Changes:
- Add
IGNORE_DEVICE/IGNORE_CONTIG_DEVICEpreprocessor definitions (NVHPC:!DIR$ IGNORE_TKR(...), otherwise comments) and apply them to MPL buffer dummy arguments. - Refactor MPL send/recv/broadcast/allreduce/alltoallv/gatherv/scatterv/allgatherv implementations into new
*_implementation_mod.F90submodules, leaving interface-only parent modules. - Update buffer dummy argument shapes (notably to assumed-rank
(..)in several APIs) and adjust templates/calls accordingly.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/fiat/CMakeLists.txt | Adds compiler-dependent preprocessor macros for IGNORE_TKR directives. |
| src/fiat/mpl/internal_f08/mpl_send_implementation_mod.F90 | New submodule implementing MPL_SEND procedures. |
| src/fiat/mpl/internal_f08/mpl_recv_implementation_mod.F90 | New submodule implementing MPL_RECV procedures. |
| src/fiat/mpl/internal_f08/mpl_broadcast_implementation_mod.F90 | New submodule implementing MPL_BROADCAST procedures. |
| src/fiat/mpl/internal_f08/mpl_scatterv_mod.F90 | Converts to interface-only module procedures; adds IGNORE_DEVICE and assumed-rank dummies. |
| src/fiat/mpl/internal_f08/mpl_scatterv_implementation_mod.F90 | New submodule implementing MPL_SCATTERV procedures. |
| src/fiat/mpl/internal_f08/mpl_scatterv_realarray_tmpl.i90 | New shared template for REAL scatterv implementation. |
| src/fiat/mpl/internal_f08/mpl_scatterv_integerarray_tmpl.i90 | New shared template for INTEGER scatterv implementation. |
| src/fiat/mpl/internal_f08/mpl_gatherv_mod.F90 | Converts to interface-only module procedures; adds IGNORE_DEVICE and assumed-rank dummies. |
| src/fiat/mpl/internal_f08/mpl_gatherv_implementation_mod.F90 | New submodule implementing MPL_GATHERV procedures. |
| src/fiat/mpl/internal_f08/mpl_gatherv_realarray_tmpl.i90 | Adjusts MPI_GATHERV/IGATHERV call to pass PRECVBUF directly. |
| src/fiat/mpl/internal_f08/mpl_gatherv_integerarray_tmpl.i90 | New shared template for INTEGER gatherv implementation. |
| src/fiat/mpl/internal_f08/mpl_alltoallv_mod.F90 | Converts to interface-only module procedures; adds IGNORE_DEVICE and assumed-rank dummies. |
| src/fiat/mpl/internal_f08/mpl_alltoallv_implementation_mod.F90 | New submodule implementing MPL_ALLTOALLV procedures. |
| src/fiat/mpl/internal_f08/mpl_allreduce_mod.F90 | Converts to interface-only module procedures; adds IGNORE_CONTIG_DEVICE markers. |
| src/fiat/mpl/internal_f08/mpl_allreduce_implementation_mod.F90 | New submodule implementing MPL_ALLREDUCE procedures. |
| src/fiat/mpl/internal_f08/mpl_allgatherv_mod.F90 | Converts to interface-only module procedures; adds IGNORE_DEVICE and assumed-rank dummies. |
| src/fiat/mpl/internal_f08/mpl_allgatherv_implementation_mod.F90 | New submodule implementing MPL_ALLGATHERV procedures. |
| src/fiat/mpl/internal_f08/mpl_allgatherv_preamble.i90 | Changes recv displacement handling when KRECVDISPL is present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| IF(PRESENT(KRECVDISPL)) THEN | ||
| IRECVDISPL_PT => KRECVDISPL | ||
| !! IRECVDISPL_PT => KRECVDISPL | ||
| ALLOCATE(IRECVDISPL,SOURCE=KRECVDISPL) | ||
| IRECVDISPL_PT => IRECVDISPL |
There was a problem hiding this comment.
When KRECVDISPL is present, this preamble now allocates a local copy (IRECVDISPL) and points IRECVDISPL_PT at it. For non-blocking allgatherv, this local allocatable will be deallocated when the procedure returns, but MPI may still access the displacement array until the request completes, leading to use-after-free. Either keep pointing to the caller-provided KRECVDISPL in the non-blocking case, or register the allocated copy in YDDISPLS_LIST (and ensure it is freed only after request completion).
| IF(LMPLSTATS) THEN | ||
| CALL MPL_SENDSTATS(ISENDCOUNT,MPI_INTEGER%MPI_VAL) | ||
| CALL MPL_RECVSTATS(SUM(KRECVCOUNTS),MPI_INTEGER%MPI_VAL) | ||
| ENDIF |
There was a problem hiding this comment.
In MPL_ALLGATHERV_INT, KRECVCOUNTS is OPTIONAL (to support the scalar case), but stats currently call SUM(KRECVCOUNTS) unconditionally. This will be invalid when KRECVCOUNTS is not present; use SUM(IRECVCOUNTS) (the local array you already build) or guard with PRESENT(KRECVCOUNTS).
3faa385 to
06d8991
Compare
|
Code warnings thrown by intel compilers being investigated, as work-around for intel breaks NVHPC. |
|
In the develop branch there are a lot of missing USE statements for e.g. MPI_ALLGATHER and MPI_IALLGATHER because they are guarded by ifdef "USE_8_BYTE_WORDS". In that case the functions are used with an implicit declaration and resolved by the MPI_F77 library. This is why we also get those gfortran warnings about mismatched MPI interfaces! I think this PR fixes these issues from what I can tell, but I am not sure if this is done throughout and if we can somehow script this to be sure (or let a coding agent loose). I don't think we are compiling fiat with USE_8_BYTE_WORDS at all. Do we foresee this will ever be needed? |
01d00b8 to
cb8cbdd
Compare
* interface module with implementing submodule * assumed rank for array arguments being communicated * IGNORE_TKR directive applied in compiler-flexible way
* interface module with implementing submodule * assumed rank for array arguments being communicated * IGNORE_TKR directive applied in compiler-flexible way * TEMPORARY : separation of gatherv template into integer template and real template to avoid calling ASSOCIATE on assumed-rank array (not allowed)
* interface module with implementing submodule * assumed rank for array arguments being communicated * IGNORE_TKR directive applied in compiler-flexible way * adjust call to MPL_BROADCAST in MPL_ALLREDUCE_MOD
* interface module with implementing submodule * assumed rank for array arguments being communicated * IGNORE_TKR directive applied in compiler-flexible way * TEMPORARY : separation of scatterv template into integer template and real template to avoid calling ASSOCIATE on assumed-rank array (not allowed)
* interface module with implementing submodule * assumed rank for array arguments being communicated * IGNORE_TKR directive applied in compiler-flexible way * TEMPORARY(?) work around Nvidia compiler bug affecting a pointer to an optional argument in a module procedure
* interface module with implementing submodule * assumed rank for array arguments being communicated * IGNORE_TKR directive applied in compiler-flexible way
* interface module with implementing submodule * assumed rank for array arguments being communicated * IGNORE_TKR directive applied in compiler-flexible way * NB : for MPL_ALLREDUCE we have used IGNORE_TKR(cd) instead of (d) to work around compiler bug
* symbols provided by mpi_serial library
…ith Intel / IntelLLVM. * the warning is believed to be unjustified in this case, a reproducer will be reported to Intel
cb8cbdd to
77bae3e
Compare
* update ci to use nvhpc version 26.1 * switch to F77 MPL implementation for older compilers
Use IGNORE_TKR directive to ignore device residency status
of buffers, allowing use of GPU-aware MPI under MPL.