coll: make bcast ring unsigned-safe #13287
Merged
+47
−32
Conversation
This file contains hidden or 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
devreal
requested changes
Jun 2, 2025
hppritcha
force-pushed
the
add_some_unsigned_safe_code
branch
from
1861ca1 to
5686600
Compare
good idea. i kept a helper routine inside this file as part of this PR. |
|
@devreal please review again when you have a chance |
devreal
previously approved these changes
Jun 5, 2025
ompi/mca/coll/base/coll_base_bcast.c
Outdated
| @@ -850,10 +860,8 @@ int ompi_coll_base_bcast_intra_scatter_allgather( | |||
| * Allgather by recursive doubling | |||
| * Each process has the curr_count elems in the buf[vrank * scatter_count, ...] | |||
| */ | |||
| size_t rem_count = count - vrank * scatter_count; | |||
| size_t rem_count = (count > vrank * scatter_count) ? count - vrank * scatter_count : 0; | |||
There was a problem hiding this comment.
Can use rectify_diff here too:
Suggested change
| size_t rem_count = (count > vrank * scatter_count) ? count - vrank * scatter_count : 0; | |
| size_t rem_count = rectify_diff(count, (size_t)(vrank * scatter_count)); |
hppritcha
force-pushed
the
add_some_unsigned_safe_code
branch
from
5686600 to
9b2ff4a
Compare
|
@devreal recheck when you have a chance |
|
@dalcinl would you have suggestions on debugging what's going on with mpi4py? it looks like some kind of problem with generation of mpi4py internal docs? |
|
@dalcinl never mind we are just going to disable the test_doc.py in our CI. |
In the conversion to support big count, there are several places where signed int's were replaced by unsigned types (size_t). Unfortunately there were a few places where signedness was being used and these need to be refactored. To find these places the -Wtype-limit gnu compile option was used. This compile option is added to the --enable-picky compile option list as part of this PR. Signed-off-by: Howard Pritchard <[email protected]>
hppritcha
force-pushed
the
add_some_unsigned_safe_code
branch
from
9b2ff4a to
0f34c42
Compare
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.
In the conversion to support big count, there are several places where signed int's were replaced by unsigned types (size_t). Unfortunately there were a few places where signedness was being used and these need to be refactored.
To find these places the -Wtype-limit gnu compile option was used. This compile option is added to the --enable-picky compile option list as part of this PR.