Skip to content

Commit e215eff

Browse files
committed
UCX osc: atomic fetch-and-op only on 32 and 64bit values
Signed-off-by: Joseph Schuchart <schuchart@hlrs.de>
1 parent 434c905 commit e215eff

File tree

1 file changed

+19
-10
lines changed

1 file changed

+19
-10
lines changed

ompi/mca/osc/ucx/osc_ucx_comm.c

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@
2424
return OMPI_ERROR; \
2525
}
2626

27+
/* macro to check whether UCX supports atomic operation on the size the operands */
28+
#define ATOMIC_SIZE_SUPPORTED(_remote_addr, _size) \
29+
((sizeof(uint32_t) == (_size) && !((_remote_addr) & 0x3)) || \
30+
(sizeof(uint64_t) == (_size) && !((_remote_addr) & 0x7)))
31+
2732
typedef struct ucx_iovec {
2833
void *addr;
2934
size_t len;
@@ -367,6 +372,7 @@ static inline
367372
bool use_atomic_op(
368373
ompi_osc_ucx_module_t *module,
369374
struct ompi_op_t *op,
375+
uint64_t remote_addr,
370376
struct ompi_datatype_t *origin_dt,
371377
struct ompi_datatype_t *target_dt,
372378
int origin_count,
@@ -384,9 +390,8 @@ bool use_atomic_op(
384390
ompi_datatype_type_size(origin_dt, &origin_dt_bytes);
385391
ompi_datatype_type_size(target_dt, &target_dt_bytes);
386392
/* UCX only supports 32 and 64-bit operands atm */
387-
if (sizeof(uint64_t) >= origin_dt_bytes &&
388-
sizeof(uint32_t) <= origin_dt_bytes &&
389-
origin_dt_bytes == target_dt_bytes &&
393+
if (ATOMIC_SIZE_SUPPORTED(remote_addr, origin_dt_bytes) &&
394+
origin_dt_bytes == target_dt_bytes &&
390395
origin_count == target_count) {
391396
return true;
392397
}
@@ -603,7 +608,7 @@ int accumulate_req(const void *origin_addr, int origin_count,
603608
}
604609

605610
/* rely on UCX network atomics if the user told us that it safe */
606-
if (use_atomic_op(module, op, origin_dt, target_dt, origin_count, target_count)) {
611+
if (use_atomic_op(module, op, target_disp, origin_dt, target_dt, origin_count, target_count)) {
607612
return do_atomic_op_intrinsic(module, op, target,
608613
origin_addr, origin_count, origin_dt,
609614
target_disp, NULL, ucx_req);
@@ -775,7 +780,7 @@ int ompi_osc_ucx_compare_and_swap(const void *origin_addr, const void *compare_a
775780
}
776781

777782
ompi_datatype_type_size(dt, &dt_bytes);
778-
if (4 == dt_bytes || 8 == dt_bytes) {
783+
if (ATOMIC_SIZE_SUPPORTED(remote_addr, dt_bytes)) {
779784
// fast path using UCX atomic operations
780785
return do_atomic_compare_and_swap(origin_addr, compare_addr,
781786
result_addr, dt, target,
@@ -818,6 +823,7 @@ int ompi_osc_ucx_fetch_and_op(const void *origin_addr, void *result_addr,
818823
struct ompi_datatype_t *dt, int target,
819824
ptrdiff_t target_disp, struct ompi_op_t *op,
820825
struct ompi_win_t *win) {
826+
size_t dt_bytes;
821827
ompi_osc_ucx_module_t *module = (ompi_osc_ucx_module_t*) win->w_osc_module;
822828
int ret = OMPI_SUCCESS;
823829

@@ -826,12 +832,15 @@ int ompi_osc_ucx_fetch_and_op(const void *origin_addr, void *result_addr,
826832
return ret;
827833
}
828834

829-
if (op == &ompi_mpi_op_no_op.op || op == &ompi_mpi_op_replace.op ||
830-
op == &ompi_mpi_op_sum.op) {
835+
uint64_t remote_addr = (module->addrs[target]) + target_disp * OSC_UCX_GET_DISP(module, target);
836+
ompi_datatype_type_size(dt, &dt_bytes);
837+
838+
/* UCX atomics are only supported on 32 and 64 bit values */
839+
if (ATOMIC_SIZE_SUPPORTED(remote_addr, dt_bytes) &&
840+
(op == &ompi_mpi_op_no_op.op || op == &ompi_mpi_op_replace.op ||
841+
op == &ompi_mpi_op_sum.op)) {
831842
uint64_t value;
832-
uint64_t remote_addr = (module->addrs[target]) + target_disp * OSC_UCX_GET_DISP(module, target);
833843
ucp_atomic_fetch_op_t opcode;
834-
size_t dt_bytes;
835844
bool lock_acquired = false;
836845

837846
if (!module->acc_single_intrinsic) {
@@ -894,7 +903,7 @@ int get_accumulate_req(const void *origin_addr, int origin_count,
894903
}
895904

896905
/* rely on UCX network atomics if the user told us that it safe */
897-
if (use_atomic_op(module, op, origin_dt, target_dt, origin_count, target_count)) {
906+
if (use_atomic_op(module, op, target_disp, origin_dt, target_dt, origin_count, target_count)) {
898907
return do_atomic_op_intrinsic(module, op, target,
899908
origin_addr, origin_count, origin_dt,
900909
target_disp, result_addr, ucx_req);

0 commit comments

Comments
 (0)