ch4/ofi: Convert CUDA device id to handle for fi_mr_regattr #7156
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
test:mpich/ch4/gpu |
hzhou
reviewed
Oct 9, 2024
src/mpl/include/mpl_gpu_cuda.h
Outdated
| @@ -22,4 +23,6 @@ typedef volatile int MPL_gpu_event_t; | |||
|
|
|||
| #define MPL_GPU_DEV_AFFINITY_ENV "CUDA_VISIBLE_DEVICES" | |||
|
|
|||
| #define MPL_gpu_device_id_to_handle(...) cuDeviceGet(__VA_ARGS__) | |||
There was a problem hiding this comment.
Rather than an MPL_gpu_ wrapper, maybe we can directly call cuDeviceGet? The usage (in fi_mr_regattr) is always inside the #ifdef MPL_HAVE_CUDA anyway, which makes it OK to use CUDA-specific calls. If we still want the MPL abstraction, I think that we can name MPL_cuda_ for CUDA-specific functions.
There was a problem hiding this comment.
Makes sense. This version just calls cuDeviceGet in ofi_impl.h.
The abstraction of device id as an integer is a good abstraction above MPL. The opaqueness of MPL_gpu_device_handle_t, on the other hand, makes it useless, since the upperlayer can't do anything with it. For ZE, simply expose the internal device id. For new GPU runtimes that does support integer device ids, we can similarly do a map as in mpl_gpu_ze.c.
Libfabric docs say that the value of the cuda field in the regattr struct is the device handle gotten from cuDeviceGet, not the ordinal. Fixes pmodels#7148.
|
test:mpich/ch4/gpu/ofi |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request Description
Libfabric docs say that the value of the cuda field in the regattr
struct is the device handle gotten from cuDeviceGet, not the
ordinal. Fixes #7148.
This PR picks [842f4cf] from #7049 in order to avoid confusion between device ids and handles.
Author Checklist
Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
Commits are self-contained and do not do two things at once.
Commit message is of the form:
module: short descriptionCommit message explains what's in the commit.
Whitespace checker. Warnings test. Additional tests via comments.
For non-Argonne authors, check contribution agreement.
If necessary, request an explicit comment from your companies PR approval manager.