-
Notifications
You must be signed in to change notification settings - Fork 4
MPI: Notified-OSC-SM Operations #6
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
Conversation
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: 08f38c3: Notify for Rput variants
acf1108: Put notify and get notify
1f1438e: put notify and get notify
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: f37ec8f: Delete .cache/clangd/index directory
08f38c3: Notify for Rput variants
acf1108: Put notify and get notify
1f1438e: put notify and get notify
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: 8fc7ada: Git Ignore update
f37ec8f: Delete .cache/clangd/index directory
08f38c3: Notify for Rput variants
acf1108: Put notify and get notify
1f1438e: put notify and get notify
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: d17854f: Remove unecessary changes pushed:
8fc7ada: Git Ignore update
f37ec8f: Delete .cache/clangd/index directory
08f38c3: Notify for Rput variants
acf1108: Put notify and get notify
1f1438e: put notify and get notify
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
d17854f to
8fc7ada
Compare
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: 8fc7ada: Git Ignore update
f37ec8f: Delete .cache/clangd/index directory
08f38c3: Notify for Rput variants
acf1108: Put notify and get notify
1f1438e: put notify and get notify
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
devreal
left a comment
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.
Thanks, good job! Some comments inline.
Also, you have committed more than necessary. Please remove compile_commands.json and remove the changes to .gitignore and the 3rd party libraries.
| ret = ompi_datatype_sndrcv((void *)origin_addr, origin_count, origin_dt, | ||
| remote_address, target_count, target_dt); | ||
|
|
||
| opal_atomic_rmb(); |
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.
This is the wrong barrier for put: we want to make sure that all writes are visible at the target prior to incrementing the notification counter. opal_atomic_wmb() (i.e., a write-barrier) is the right way to go.
Same applies to rput_with_notify
| void *remote_address; | ||
|
|
||
| OPAL_OUTPUT_VERBOSE((50, ompi_osc_base_framework.framework_output, | ||
| "rget: 0x%lx, %zu, %s, %d, %d, %zu, %s, 0x%lx", |
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.
Small fix:
| "rget: 0x%lx, %zu, %s, %d, %d, %zu, %s, 0x%lx", | |
| "rget_notify: 0x%lx, %zu, %s, %d, %d, %zu, %s, 0x%lx", |
And please add the notification counter to the debug output
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: 8fc7ada: Git Ignore update
f37ec8f: Delete .cache/clangd/index directory
08f38c3: Notify for Rput variants
acf1108: Put notify and get notify
1f1438e: put notify and get notify
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
|
Hello! The Git Commit Checker CI bot found a few problems with this PR: d9f5d2b: Cleaning up the PR
8fc7ada: Git Ignore update
f37ec8f: Delete .cache/clangd/index directory
08f38c3: Notify for Rput variants
acf1108: Put notify and get notify
1f1438e: put notify and get notify
Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks! |
|
Closing this, please open a new PR from a fresh branch. |
No description provided.