-
Notifications
You must be signed in to change notification settings - Fork 16
Add test for MPI 4.0 MPI_Win_shared_query #36
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
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Joseph Schuchart <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds a test suite for the new MPI 4.0 feature that allows MPI_Win_shared_query to be used with windows created by MPI_Win_create and MPI_Win_allocate, in addition to the existing support for MPI_Win_allocate_shared.
- Implements comprehensive test coverage for all three window creation methods
- Validates that
MPI_Win_shared_queryreturns appropriate results without failing - Includes proper error handling and memory verification
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| one-sided/test_mpi_win_shared_query.c | Main test implementation covering all three MPI window creation methods |
| one-sided/run.sh | Simple shell script to execute the test with 2 MPI processes |
| one-sided/Makefile | Build configuration for compiling the test executable |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| printf("%s: rank %d: Expected zero size but got %zu\n", test_name, rank, queried_size); | ||
| } | ||
| ret = MPI_ERR_NO_MEM; | ||
| break; | ||
| } | ||
|
|
||
| if (queried_size != shared_size) { | ||
| printf("%s: rank %d: Queried size mismatch: expected %d, got %zu\n", test_name, rank, shared_size, queried_size); |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format specifier %zu is used for MPI_Aint queried_size, but MPI_Aint is not guaranteed to be the same type as size_t. Use %ld with a cast to long or check the MPI implementation's recommended format specifier for MPI_Aint.
| printf("%s: rank %d: Expected zero size but got %zu\n", test_name, rank, queried_size); | |
| } | |
| ret = MPI_ERR_NO_MEM; | |
| break; | |
| } | |
| if (queried_size != shared_size) { | |
| printf("%s: rank %d: Queried size mismatch: expected %d, got %zu\n", test_name, rank, shared_size, queried_size); | |
| printf("%s: rank %d: Expected zero size but got %ld\n", test_name, rank, (long)queried_size); | |
| } | |
| ret = MPI_ERR_NO_MEM; | |
| break; | |
| } | |
| if (queried_size != shared_size) { | |
| printf("%s: rank %d: Queried size mismatch: expected %d, got %ld\n", test_name, rank, shared_size, (long)queried_size); |
| printf("%s: rank %d: Expected zero size but got %zu\n", test_name, rank, queried_size); | ||
| } | ||
| ret = MPI_ERR_NO_MEM; | ||
| break; | ||
| } | ||
|
|
||
| if (queried_size != shared_size) { | ||
| printf("%s: rank %d: Queried size mismatch: expected %d, got %zu\n", test_name, rank, shared_size, queried_size); |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The format specifier %zu is used for MPI_Aint queried_size, but MPI_Aint is not guaranteed to be the same type as size_t. Use %ld with a cast to long or check the MPI implementation's recommended format specifier for MPI_Aint.
| printf("%s: rank %d: Expected zero size but got %zu\n", test_name, rank, queried_size); | |
| } | |
| ret = MPI_ERR_NO_MEM; | |
| break; | |
| } | |
| if (queried_size != shared_size) { | |
| printf("%s: rank %d: Queried size mismatch: expected %d, got %zu\n", test_name, rank, shared_size, queried_size); | |
| printf("%s: rank %d: Expected zero size but got %ld\n", test_name, rank, (long)queried_size); | |
| } | |
| ret = MPI_ERR_NO_MEM; | |
| break; | |
| } | |
| if (queried_size != shared_size) { | |
| printf("%s: rank %d: Queried size mismatch: expected %d, got %ld\n", test_name, rank, shared_size, (long)queried_size); |
| } while (false); | ||
| } | ||
|
|
||
| int *ret_vals = (int *)malloc(size * sizeof(int)); |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing null check after malloc. If malloc fails and returns NULL, the subsequent MPI_Gather call will cause undefined behavior.
| int *ret_vals = (int *)malloc(size * sizeof(int)); | |
| int *ret_vals = (int *)malloc(size * sizeof(int)); | |
| if (ret_vals == NULL) { | |
| printf("%s: rank %d: malloc for ret_vals failed\n", test_name, rank); | |
| ret = MPI_ERR_NO_MEM; | |
| // Use MPI_Gather with a NULL recvbuf only on non-root, or skip entirely | |
| // To avoid undefined behavior, we return early | |
| return; | |
| } |
|
|
||
| // Test with MPI_Win_create | ||
| MPI_Win win_create; | ||
| int* local_mem = (int*)malloc(sizeof(int) * win_size); |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing null check after malloc. If malloc fails and returns NULL, the subsequent MPI_Win_create call may cause undefined behavior.
| int* local_mem = (int*)malloc(sizeof(int) * win_size); | |
| int* local_mem = (int*)malloc(sizeof(int) * win_size); | |
| if (local_mem == NULL) { | |
| fprintf(stderr, "Error: malloc failed for local_mem in MPI_Win_create test\n"); | |
| MPI_Abort(MPI_COMM_WORLD, EXIT_FAILURE); | |
| } |
| } | ||
|
|
||
| int *ret_vals = (int *)malloc(size * sizeof(int)); | ||
| MPI_Gather(&ret, 1, MPI_INT, ret_vals, 1, MPI_INT, 0, comm); |
Copilot
AI
Sep 19, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Memory leak: ret_vals is allocated but never freed. Add free(ret_vals) after the MPI_Gather operation and result processing.
Test for changes introduced in open-mpi/ompi#13330. Since MPI 4.0,
MPI_Win_shared_queryallows querying shared memory in windows created usingMPI_Win_createandMPI_Win_allocate. This test checks that the call toMPI_Win_shared_querydoes not fail. The test itself does not fail if shared memory was not provided since that is not required.