Skip to content

OSHMEM/AMO: added int/uint/32/64 atomics calls - v4.0 #6110

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

Merged

Conversation

hoopoepg
Copy link
Contributor

  • added int/uint/32/64 atomics calls
  • added SHMEM_SYNC_SIZE macro

backport from #6096

Signed-off-by: Sergey Oblomov sergeyo@mellanox.com
(cherry picked from commit 4c071da)

- added int/uint/32/64 atomics calls
- added SHMEM_SYNC_SIZE macro

Signed-off-by: Sergey Oblomov <sergeyo@mellanox.com>
(cherry picked from commit 4c071da)
@hoopoepg hoopoepg changed the title OSHMEM/AMO: added int/uint/32/64 atomics calls OSHMEM/AMO: added int/uint/32/64 atomics calls - v4.0 Nov 26, 2018
@hoopoepg hoopoepg added this to the v4.0.1 milestone Nov 26, 2018
@gpaulsen
Copy link
Member

Lets discuss on Nov 27 web-ex call.

@xinzhao3
Copy link
Contributor

We confirm that this PR will not break the backward compatibility.

@gpaulsen gpaulsen requested a review from xinzhao3 November 30, 2018 19:59
@gpaulsen
Copy link
Member

@xinzhao3 can you please review? If not please assign someone else.

@xinzhao3
Copy link
Contributor

reviewing it now

OSHMEM_DECLSPEC void pshmem_int32_atomic_and(int32_t *target, int32_t value, int pe);
OSHMEM_DECLSPEC void pshmem_int64_atomic_and(int64_t *target, int64_t value, int pe);
OSHMEM_DECLSPEC void pshmem_uint32_atomic_and(uint32_t *target, uint32_t value, int pe);
OSHMEM_DECLSPEC void pshmem_uint64_atomic_and(uint64_t *target, uint64_t value, int pe);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add int32/64, uint32/64 to C11 macro?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no - it failed to compile due to datatype conflicts

OSHMEM_DECLSPEC void pshmem_int32_atomic_or(int32_t *target, int32_t value, int pe);
OSHMEM_DECLSPEC void pshmem_int64_atomic_or(int64_t *target, int64_t value, int pe);
OSHMEM_DECLSPEC void pshmem_uint32_atomic_or(uint32_t *target, uint32_t value, int pe);
OSHMEM_DECLSPEC void pshmem_uint64_atomic_or(uint64_t *target, uint64_t value, int pe);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add int32/64, uint32/64 to C11 macro?

OSHMEM_DECLSPEC void pshmem_int32_atomic_xor(int32_t *target, int32_t value, int pe);
OSHMEM_DECLSPEC void pshmem_int64_atomic_xor(int64_t *target, int64_t value, int pe);
OSHMEM_DECLSPEC void pshmem_uint32_atomic_xor(uint32_t *target, uint32_t value, int pe);
OSHMEM_DECLSPEC void pshmem_uint64_atomic_xor(uint64_t *target, uint64_t value, int pe);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add int32/64, uint32/64 to C11 macro?

OSHMEM_DECLSPEC void pshmem_uint32_wait_until(volatile uint32_t *addr, int cmp, uint32_t value);
OSHMEM_DECLSPEC void pshmem_uint64_wait_until(volatile uint64_t *addr, int cmp, uint64_t value);
OSHMEM_DECLSPEC void pshmem_size_wait_until(volatile size_t *addr, int cmp, size_t value);
OSHMEM_DECLSPEC void pshmem_ptrdiff_wait_until(volatile ptrdiff_t *addr, int cmp, ptrdiff_t value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add int32/64, uint32/64, size, ptrdiff to C11 macro?

OSHMEM_DECLSPEC int pshmem_uint32_test(volatile uint32_t *addr, int cmp, uint32_t value);
OSHMEM_DECLSPEC int pshmem_uint64_test(volatile uint64_t *addr, int cmp, uint64_t value);
OSHMEM_DECLSPEC int pshmem_size_test(volatile size_t *addr, int cmp, size_t value);
OSHMEM_DECLSPEC int pshmem_ptrdiff_test(volatile ptrdiff_t *addr, int cmp, ptrdiff_t value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add int32/64, uint32/64, size, ptrdiff to C11 macro?

OSHMEM_DECLSPEC int32_t shmem_ctx_int32_atomic_fetch_xor(shmem_ctx_t ctx, int32_t *target, int32_t value, int pe);
OSHMEM_DECLSPEC int64_t shmem_ctx_int64_atomic_fetch_xor(shmem_ctx_t ctx, int64_t *target, int64_t value, int pe);
OSHMEM_DECLSPEC uint32_t shmem_ctx_uint32_atomic_fetch_xor(shmem_ctx_t ctx, uint32_t *target, uint32_t value, int pe);
OSHMEM_DECLSPEC uint64_t shmem_ctx_uint64_atomic_fetch_xor(shmem_ctx_t ctx, uint64_t *target, uint64_t value, int pe);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add int32/64, uint32/64 to C11 macro?

OSHMEM_DECLSPEC int32_t shmem_int32_atomic_fetch_xor(int32_t *target, int32_t value, int pe);
OSHMEM_DECLSPEC int64_t shmem_int64_atomic_fetch_xor(int64_t *target, int64_t value, int pe);
OSHMEM_DECLSPEC uint32_t shmem_uint32_atomic_fetch_xor(uint32_t *target, uint32_t value, int pe);
OSHMEM_DECLSPEC uint64_t shmem_uint64_atomic_fetch_xor(uint64_t *target, uint64_t value, int pe);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add int32/64, uint32/64 to C11 macro?

OSHMEM_DECLSPEC int32_t shmem_ctx_int32_atomic_fetch_or(shmem_ctx_t ctx, int32_t *target, int32_t value, int pe);
OSHMEM_DECLSPEC int64_t shmem_ctx_int64_atomic_fetch_or(shmem_ctx_t ctx, int64_t *target, int64_t value, int pe);
OSHMEM_DECLSPEC uint32_t shmem_ctx_uint32_atomic_fetch_or(shmem_ctx_t ctx, uint32_t *target, uint32_t value, int pe);
OSHMEM_DECLSPEC uint64_t shmem_ctx_uint64_atomic_fetch_or(shmem_ctx_t ctx, uint64_t *target, uint64_t value, int pe);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add int32/64, uint32/64 to C11 macro?

OSHMEM_DECLSPEC int32_t shmem_ctx_int32_atomic_fetch_and(shmem_ctx_t ctx, int32_t *target, int32_t value, int pe);
OSHMEM_DECLSPEC int64_t shmem_ctx_int64_atomic_fetch_and(shmem_ctx_t ctx, int64_t *target, int64_t value, int pe);
OSHMEM_DECLSPEC uint32_t shmem_ctx_uint32_atomic_fetch_and(shmem_ctx_t ctx, uint32_t *target, uint32_t value, int pe);
OSHMEM_DECLSPEC uint64_t shmem_ctx_uint64_atomic_fetch_and(shmem_ctx_t ctx, uint64_t *target, uint64_t value, int pe);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should add int32/64, uint32/64 to C11 macro?

@@ -63,6 +63,7 @@
#pragma weak shmem_ctx_int_g = pshmem_ctx_int_g
#pragma weak shmem_ctx_long_g = pshmem_ctx_long_g
#pragma weak shmem_ctx_longlong_g = pshmem_ctx_longlong_g
#pragma weak shmem_ctx_schar_g = pshmem_ctx_schar_g
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to add int8, int16, int32, int64, uint8, uint16, uint32, uint64, size, ptrdiff here, also same with shmem_p

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, will add in next PR (will add into master first)

@hoopoepg
Copy link
Contributor Author

hoopoepg commented Dec 6, 2018

@xinzhao3 anything else to fix?
we will add shmem_g/p API in next PR (for now we are forcing this PR to allow tests pass)

@hppritcha
Copy link
Member

@xinzhao3 do you and @hoopoepg have agreement here? Is this PR ready to merge?

@hoopoepg
Copy link
Contributor Author

hoopoepg commented Dec 10, 2018

@hppritcha I think yes, API requested by @xinzhao3 is added in #6168, no more changes are required

@hppritcha
Copy link
Member

@xinzhao3 could you switch you review to ✅ ?

@xinzhao3
Copy link
Contributor

@hppritcha @hoopoepg I am reviewing this again now, will response soon today.

@hppritcha hppritcha merged commit 552834b into open-mpi:v4.0.x Dec 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants