opal/asm: add opal_atomic_compare_exchange_strong functions #4475
Conversation
|
Tested on x86_64. PowerPC has been lightly tested but will be tested by CI. |
|
The IBM CI (GNU Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/14cad17efe7a777fb653261e36da9ee3 |
|
Why do you keep the deprecated opal_atomic_bool_cmpset_xx functions ? |
|
@bosilca Continuity. I though we should deprecate (and warn-- I need to add the attribute) for 4.0 and remove for 5.0. Its internal so I don't know if that is necessary. |
|
Indeed they are internal and in most cases inlined. So even for binary distributed modules there will be no portability issues. +1 to just remove them. |
|
Ok, I will do that now and update the PR. |
This commit adds a new set of compare-and-exchange functions. These functions have a signature similar to the functions found in C11. The old cmpset functions are now deprecated and defined in terms of the new compare-and-exchange functions. All asm backends have been updated. Signed-off-by: Nathan Hjelm <[email protected]>
Signed-off-by: Nathan Hjelm <[email protected]>
|
@bosilca Removed the old cmpset functions. Should be good to go after CI finishes. |
hjelmn
force-pushed
the
asm_cleanup
branch
3 times, most recently
from
1bd9734 to
712186e
Compare
|
The IBM CI (GNU Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/0902da0b2ba74947e60c2dd2f61e1bd3 |
|
The IBM CI (XL Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/62cab238c29b3e0028568ffb493da39a |
|
The IBM CI (PGI Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/5034a7ff503d6f68ea034ef44a36f8c7 |
|
The IBM CI (GNU Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/e28dfa582269fcdee8845d858400a995 |
|
The IBM CI (XL Compiler) build failed! Please review the log, linked below. Gist: https://gist.github.com/55785a274afb5d9c8de1028fc89e7e40 |
|
@bosilca Ok, think I got all the issues worked out. This PR adds the new compare-exchange functions and removes the old cmpset ones. |
This commit eliminates the old opal_atomic_bool_cmpset functions. They have been replaced by the opal_atomic_compare_exchange_strong functions. Signed-off-by: Nathan Hjelm <[email protected]>
| { \ | ||
| type oldval, newval; \ | ||
| do { \ | ||
| oldval = *addr; \ |
There was a problem hiding this comment.
shouldn't this be outside the do loop ? (the oldval will be updated by the compare_exchange anyway).
There was a problem hiding this comment.
Ah yes. I made that change in other places but missed this one. Will be irrelevant in the next PR though. I will be replacing the underlying op-and-fetch arithmetic atomics with fetch-and-op. This is already fixed in that branch. I will post that PR once this one is merged unless you would like me to include the fetch-and-op change in this PR.
|
@bosilca I am going to open a PR that has both the compare-exchange changes and the atomic arithmetic changes. We can either merge the one and the other one will only handle the arithmetic or close this one and use the other PR. |
|
See #4552 |
|
I also added some tests to atomic_math.c to check each form (fetch-and-op and op-and-fetch) of the atomic operations. |
|
On master already. Closing. |
This commit adds a new set of compare-and-exchange functions. These
functions have a signature similar to the functions found in C11. The
old cmpset functions are now deprecated and defined in terms of the
new compare-and-exchange functions. All asm backends have been
updated.
Signed-off-by: Nathan Hjelm [email protected]