Skip to content

Commit 61089d7

Browse files
committed
osc/ucx: convert ATOMIC_SIZE_SUPPORTED from a macro to static inline function
Generally speaking static inline functions are the better choice. Open MPI generally leave the old macro usage in place but this macro is not ideal as written. (_remote_addr) & 0x3 is interpreted to be a cast by clang-format. While not right it does not have enough information in the limited scope to decide it definitely is not a cast. I will try to patch this case in llvm but for not it is better to just modernize this piece of code to avoid the issue altogether. Signed-off-by: Nathan Hjelm <[email protected]>
1 parent 2edb461 commit 61089d7

File tree

1 file changed

+11
-9
lines changed

1 file changed

+11
-9
lines changed

ompi/mca/osc/ucx/osc_ucx_comm.c

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,13 @@
2626
return OMPI_ERROR; \
2727
}
2828

29-
/* macro to check whether UCX supports atomic operation on the size the operands */
30-
#define ATOMIC_SIZE_SUPPORTED(_remote_addr, _size) \
31-
((sizeof(uint32_t) == (_size) && !((_remote_addr) & 0x3)) || \
32-
(sizeof(uint64_t) == (_size) && !((_remote_addr) & 0x7)))
29+
/* helper to check whether UCX supports atomic operation on the size the operands */
30+
static inline __opal_attribute_always_inline__ bool is_atomic_size_supported(uint64_t remote_addr,
31+
size_t size)
32+
{
33+
return ((sizeof(uint32_t) == size && !(remote_addr & 0x3)) ||
34+
(sizeof(uint64_t) == size && !(remote_addr & 0x7)));
35+
}
3336

3437
typedef struct ucx_iovec {
3538
void *addr;
@@ -400,9 +403,8 @@ bool use_atomic_op(
400403
ompi_datatype_type_size(origin_dt, &origin_dt_bytes);
401404
ompi_datatype_type_size(target_dt, &target_dt_bytes);
402405
/* UCX only supports 32 and 64-bit operands atm */
403-
if (ATOMIC_SIZE_SUPPORTED(remote_addr, origin_dt_bytes) &&
404-
origin_dt_bytes == target_dt_bytes &&
405-
origin_count == target_count) {
406+
if (is_atomic_size_supported(remote_addr, origin_dt_bytes) &&
407+
origin_dt_bytes == target_dt_bytes && origin_count == target_count) {
406408
return true;
407409
}
408410
}
@@ -794,7 +796,7 @@ int ompi_osc_ucx_compare_and_swap(const void *origin_addr, const void *compare_a
794796
}
795797

796798
ompi_datatype_type_size(dt, &dt_bytes);
797-
if (ATOMIC_SIZE_SUPPORTED(remote_addr, dt_bytes)) {
799+
if (is_atomic_size_supported(remote_addr, dt_bytes)) {
798800
// fast path using UCX atomic operations
799801
return do_atomic_compare_and_swap(origin_addr, compare_addr,
800802
result_addr, dt, target,
@@ -850,7 +852,7 @@ int ompi_osc_ucx_fetch_and_op(const void *origin_addr, void *result_addr,
850852
ompi_datatype_type_size(dt, &dt_bytes);
851853

852854
/* UCX atomics are only supported on 32 and 64 bit values */
853-
if (ATOMIC_SIZE_SUPPORTED(remote_addr, dt_bytes) &&
855+
if (is_atomic_size_supported(remote_addr, dt_bytes) &&
854856
(op == &ompi_mpi_op_no_op.op || op == &ompi_mpi_op_replace.op ||
855857
op == &ompi_mpi_op_sum.op)) {
856858
uint64_t value;

0 commit comments

Comments
 (0)