Skip to content
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.

Sign up for GitHub

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

Signal operation type for put with signal #929

Merged
merged 10 commits into from
Feb 24, 2020
Merged

Signal operation type for put with signal #929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1b471e5
Signal operation type for put with signal
wrrobin Feb 6, 2020
be9e259
Added changes to portals4 put-signal implementation
wrrobin Feb 6, 2020
67912f5
Correction for instance with len = 0
wrrobin Feb 6, 2020
42a61cf
Correction in API call for instance with len = 0
wrrobin Feb 6, 2020
217c430
Trial hack for the fix related to on-node transport of signal atomic
wrrobin Feb 12, 2020
bada86b
Bug fix for missing function declaration and incorrect macro
wrrobin Feb 12, 2020
32bf17d
Update in configure script to enable shr atomics when only xpmem is e…
wrrobin Feb 12, 2020
3af033e
Bug fix for data race in put_signal_nbi test
wrrobin Feb 12, 2020
e24e99a
Set the enable-shr-atomics to auto by default rather than disabled
wrrobin Feb 12, 2020
432835a
Moved signal op constants to shmemx.h; incorporated git comments
wrrobin Feb 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ AS_IF([test "$enable_ofi_fence" = "yes"],

AC_ARG_ENABLE([shr-atomics],
[AC_HELP_STRING([--enable-shr-atomics],
[Enable shared memory atomic operations. (default: disabled)])])
[Enable shared memory atomic operations. (default: auto)])])
AS_IF([test "$enable_shr_atomics" = "yes"],
[AC_DEFINE([USE_SHR_ATOMICS], [1], [If defined, the shared memory layer will perform processor atomics.])])

Expand Down Expand Up @@ -414,6 +414,10 @@ AS_IF([test "$transport_xpmem" = "yes" -o "$transport_cma" = "yes"],
AC_DEFINE([ENABLE_HARD_POLLING], [1], [Enable hard polling])
])

if test "$enable_shr_atomics" != "no" -a "$transport_xpmem" = "yes" -a "$transport_ofi" = "no" -a "$transport_portals4" = "no" ; then
AC_DEFINE([USE_SHR_ATOMICS], [1], [If defined, the shared memory layer will perform processor atomics.])
fi
Copy link
Member

Choose a reason for hiding this comment

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

We have always treated the above as an invalid build; the configure script should emit a warning any time there isn't a transport selected. That having been said, this seems like a good change. You might want to update the description of the --enable-shr-atomics to say "default: auto" rather than "default: disabled".

A minor fix here, the check for "$enable_shr_atomics" = "no" is not quite right (see Example 1.1 here). It's counterintuitive, but I think this check should be "$enable_shr_atomics" != "no" i.e. "not disabled".


AC_ARG_ENABLE([pmi-simple], [AC_HELP_STRING([--enable-pmi-simple],
[Use MPICH simple PMI-1 library for process management])])
AC_ARG_ENABLE([pmi-mpi], [AC_HELP_STRING([--enable-pmi-mpi],
Expand Down
3 changes: 3 additions & 0 deletions mpp/shmemx-def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,9 @@ typedef struct {

#define SHMEMX_TEAM_NUM_CONTEXTS (1l<<0)

#define SHMEMX_SIGNAL_SET 0
#define SHMEMX_SIGNAL_ADD 1

#ifdef __cplusplus
}
#endif
Expand Down
16 changes: 8 additions & 8 deletions mpp/shmemx.h4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,24 +190,24 @@ SHMEM_CXX_DEFINE_FOR_BITWISE_AMO(`SHMEM_CXX_ATOMIC_FETCH_XOR_NBI')
/* Blocking put with signal */
define(`SHMEM_CXX_PUT_SIGNAL',
`static inline void shmemx_put_signal($2* dest, const $2* source,
size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe) {
shmemx_$1_put_signal(dest, source, nelems, sig_addr, signal, pe);
size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe) {
shmemx_$1_put_signal(dest, source, nelems, sig_addr, signal, sig_op, pe);
}
static inline void shmemx_put_signal(shmem_ctx_t ctx, $2* dest, const $2* source,
size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe) {
shmemx_ctx_$1_put_signal(ctx, dest, source, nelems, sig_addr, signal, pe);
size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe) {
shmemx_ctx_$1_put_signal(ctx, dest, source, nelems, sig_addr, signal, sig_op, pe);
}')dnl
SHMEM_CXX_DEFINE_FOR_RMA(`SHMEM_CXX_PUT_SIGNAL')

/* Non-blocking put with signal */
define(`SHMEM_CXX_PUT_SIGNAL_NBI',
`static inline void shmemx_put_signal_nbi($2* dest, const $2* source,
size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe) {
shmemx_$1_put_signal_nbi(dest, source, nelems, sig_addr, signal, pe);
size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe) {
shmemx_$1_put_signal_nbi(dest, source, nelems, sig_addr, signal, sig_op, pe);
}
static inline void shmemx_put_signal_nbi(shmem_ctx_t ctx, $2* dest, const $2* source,
size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe) {
shmemx_ctx_$1_put_signal_nbi(ctx, dest, source, nelems, sig_addr, signal, pe);
size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe) {
shmemx_ctx_$1_put_signal_nbi(ctx, dest, source, nelems, sig_addr, signal, sig_op, pe);
}')dnl
SHMEM_CXX_DEFINE_FOR_RMA(`SHMEM_CXX_PUT_SIGNAL_NBI')

Expand Down
16 changes: 8 additions & 8 deletions mpp/shmemx_c_func.h4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,39 +155,39 @@ SHMEM_DECLARE_FOR_BITWISE_AMO(`SHMEM_C_CTX_FETCH_OR_NBI')

/* Blocking put with signal */
define(`SHMEM_C_PUT_SIGNAL',
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_$1_put_signal($2 *target, const $2 *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe)')dnl
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_$1_put_signal($2 *target, const $2 *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe)')dnl
SHMEM_DECLARE_FOR_RMA(`SHMEM_C_PUT_SIGNAL')

define(`SHMEM_C_CTX_PUT_SIGNAL',
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_ctx_$1_put_signal(shmem_ctx_t ctx, $2 *target, const $2 *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe)')dnl
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_ctx_$1_put_signal(shmem_ctx_t ctx, $2 *target, const $2 *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe)')dnl
SHMEM_DECLARE_FOR_RMA(`SHMEM_C_CTX_PUT_SIGNAL')

define(`SHMEM_C_PUT_N_SIGNAL',
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_put$1_signal(void* target, const void *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe)')dnl
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_put$1_signal(void* target, const void *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe)')dnl
SHMEM_DECLARE_FOR_SIZES(`SHMEM_C_PUT_N_SIGNAL')
SHMEM_C_PUT_N_SIGNAL(mem,1);

define(`SHMEM_C_CTX_PUT_N_SIGNAL',
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_ctx_put$1_signal(shmem_ctx_t ctx, void* target, const void *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe)')dnl
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_ctx_put$1_signal(shmem_ctx_t ctx, void* target, const void *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe)')dnl
SHMEM_DECLARE_FOR_SIZES(`SHMEM_C_CTX_PUT_N_SIGNAL')
SHMEM_C_CTX_PUT_N_SIGNAL(mem,1);

/* Non-blocking put with signal */
define(`SHMEM_C_PUT_SIGNAL_NBI',
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_$1_put_signal_nbi($2 *target, const $2 *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe)')dnl
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_$1_put_signal_nbi($2 *target, const $2 *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe)')dnl
SHMEM_DECLARE_FOR_RMA(`SHMEM_C_PUT_SIGNAL_NBI')

define(`SHMEM_C_CTX_PUT_SIGNAL_NBI',
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_ctx_$1_put_signal_nbi(shmem_ctx_t ctx, $2 *target, const $2 *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe)')dnl
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_ctx_$1_put_signal_nbi(shmem_ctx_t ctx, $2 *target, const $2 *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe)')dnl
SHMEM_DECLARE_FOR_RMA(`SHMEM_C_CTX_PUT_SIGNAL_NBI')

define(`SHMEM_C_PUT_N_SIGNAL_NBI',
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_put$1_signal_nbi(void* target, const void *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe)')dnl
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_put$1_signal_nbi(void* target, const void *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe)')dnl
SHMEM_DECLARE_FOR_SIZES(`SHMEM_C_PUT_N_SIGNAL_NBI')
SHMEM_C_PUT_N_SIGNAL_NBI(mem,1);

define(`SHMEM_C_CTX_PUT_N_SIGNAL_NBI',
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_ctx_put$1_signal_nbi(shmem_ctx_t ctx, void* target, const void *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int pe)')dnl
`SHMEM_FUNCTION_ATTRIBUTES void SHPRE()shmemx_ctx_put$1_signal_nbi(shmem_ctx_t ctx, void* target, const void *source, size_t nelems, uint64_t *sig_addr, uint64_t signal, int sig_op, int pe)')dnl
SHMEM_DECLARE_FOR_SIZES(`SHMEM_C_CTX_PUT_N_SIGNAL_NBI')
SHMEM_C_CTX_PUT_N_SIGNAL_NBI(mem,1);

Expand Down
40 changes: 30 additions & 10 deletions src/data_c.c4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,66 +473,86 @@ SHMEM_PROF_DEF_CTX_PUT_N_SIGNAL_NBI(`mem')
void SHMEM_FUNCTION_ATTRIBUTES \
SHMEMX_FUNC_PROTOTYPE(STYPE##_put_signal, TYPE *target, \
const TYPE *source, size_t nelems, \
uint64_t *sig_addr, uint64_t signal, int pe) \
uint64_t *sig_addr, uint64_t signal, \
int sig_op, int pe) \
long completion = 0; \
SHMEM_ERR_CHECK_INITIALIZED(); \
SHMEM_ERR_CHECK_PE(pe); \
SHMEM_ERR_CHECK_SYMMETRIC(target, sizeof(TYPE) * nelems); \
SHMEM_ERR_CHECK_NULL(source, nelems); \
SHMEM_ERR_CHECK_SIG_OP(sig_op); \
shmem_internal_put_nb(ctx, target, source, \
sizeof(TYPE) * nelems, pe, \
&completion); \
shmem_internal_put_wait(ctx, &completion); \
shmem_internal_fence(ctx); \
shmem_internal_put_scalar(ctx, sig_addr, &signal, \
sizeof(uint64_t), pe); \
if (sig_op == SHMEMX_SIGNAL_ADD) \
shmem_internal_atomic(ctx, sig_addr, &signal, sizeof(uint64_t), \
pe, SHM_INTERNAL_SUM, \
SHM_INTERNAL_UINT64); \
else \
shmem_internal_atomic_set(ctx, sig_addr, &signal, \
sizeof(uint64_t), pe, \
SHM_INTERNAL_UINT64); \
}


#define SHMEM_DEF_PUT_N_SIGNAL(NAME,SIZE) \
void SHMEM_FUNCTION_ATTRIBUTES \
SHMEMX_FUNC_PROTOTYPE(put##NAME##_signal, void *target, \
const void *source, size_t nelems, \
uint64_t *sig_addr, uint64_t signal, int pe) \
uint64_t *sig_addr, uint64_t signal, \
int sig_op, int pe) \
long completion = 0; \
SHMEM_ERR_CHECK_INITIALIZED(); \
SHMEM_ERR_CHECK_PE(pe); \
SHMEM_ERR_CHECK_SYMMETRIC(target, (SIZE) * nelems); \
SHMEM_ERR_CHECK_NULL(source, nelems); \
SHMEM_ERR_CHECK_SIG_OP(sig_op); \
shmem_internal_put_nb(ctx, target, source, (SIZE) * nelems, \
pe, &completion); \
shmem_internal_put_wait(ctx, &completion); \
shmem_internal_fence(ctx); \
shmem_internal_put_scalar(ctx, sig_addr, &signal, \
sizeof(uint64_t), pe); \
if (sig_op == SHMEMX_SIGNAL_ADD) \
shmem_internal_atomic(ctx, sig_addr, &signal, sizeof(uint64_t), \
pe, SHM_INTERNAL_SUM, \
SHM_INTERNAL_UINT64); \
else \
shmem_internal_atomic_set(ctx, sig_addr, &signal, \
sizeof(uint64_t), pe, \
SHM_INTERNAL_UINT64); \
}

#define SHMEM_DEF_PUT_SIGNAL_NBI(STYPE,TYPE) \
void SHMEM_FUNCTION_ATTRIBUTES \
SHMEMX_FUNC_PROTOTYPE(STYPE##_put_signal_nbi, TYPE *target, \
const TYPE *source, size_t nelems, \
uint64_t *sig_addr, uint64_t signal, int pe) \
uint64_t *sig_addr, uint64_t signal, \
int sig_op, int pe) \
SHMEM_ERR_CHECK_INITIALIZED(); \
SHMEM_ERR_CHECK_PE(pe); \
SHMEM_ERR_CHECK_SYMMETRIC(target, sizeof(TYPE) * nelems); \
SHMEM_ERR_CHECK_NULL(source, nelems); \
SHMEM_ERR_CHECK_SIG_OP(sig_op); \
shmem_internal_put_signal_nbi(ctx, target, source, \
sizeof(TYPE) * nelems, sig_addr, \
signal, pe); \
signal, sig_op, pe); \
}


#define SHMEM_DEF_PUT_N_SIGNAL_NBI(NAME,SIZE) \
void SHMEM_FUNCTION_ATTRIBUTES \
SHMEMX_FUNC_PROTOTYPE(put##NAME##_signal_nbi, void *target, \
const void *source, size_t nelems, \
uint64_t *sig_addr, uint64_t signal, int pe) \
uint64_t *sig_addr, uint64_t signal, \
int sig_op, int pe) \
SHMEM_ERR_CHECK_INITIALIZED(); \
SHMEM_ERR_CHECK_PE(pe); \
SHMEM_ERR_CHECK_SYMMETRIC(target, (SIZE) * nelems); \
SHMEM_ERR_CHECK_NULL(source, nelems); \
SHMEM_ERR_CHECK_SIG_OP(sig_op); \
shmem_internal_put_signal_nbi(ctx, target, source, (SIZE) * nelems, \
sig_addr, signal, pe); \
sig_addr, signal, sig_op, pe); \
}


Expand Down
13 changes: 9 additions & 4 deletions src/shmem_comm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,17 +61,22 @@ shmem_internal_put_nb(shmem_ctx_t ctx, void *target, const void *source, size_t
static inline
void
shmem_internal_put_signal_nbi(shmem_ctx_t ctx, void *target, const void *source, size_t len,
uint64_t *sig_addr, uint64_t signal, int pe)
uint64_t *sig_addr, uint64_t signal, int sig_op, int pe)
{
if (len == 0) {
shmem_internal_put_scalar(ctx, sig_addr, &signal, sizeof(uint64_t), pe);
if (sig_op == SHMEMX_SIGNAL_ADD)
shmem_transport_atomic((shmem_transport_ctx_t *) ctx, sig_addr, &signal, sizeof(uint64_t),
pe, SHM_INTERNAL_SUM, SHM_INTERNAL_UINT64);
else
shmem_transport_atomic_set((shmem_transport_ctx_t *) ctx, sig_addr, &signal,
sizeof(uint64_t), pe, SHM_INTERNAL_UINT64);
return;
}

if (shmem_shr_transport_use_write(ctx, target, source, len, pe)) {
shmem_shr_transport_put_signal(ctx, target, source, len, sig_addr, signal, pe);
shmem_shr_transport_put_signal(ctx, target, source, len, sig_addr, signal, sig_op, pe);
} else {
shmem_transport_put_signal_nbi((shmem_transport_ctx_t *) ctx, target, source, len, sig_addr, signal, pe);
shmem_transport_put_signal_nbi((shmem_transport_ctx_t *) ctx, target, source, len, sig_addr, signal, sig_op, pe);
}
}

Expand Down
15 changes: 15 additions & 0 deletions src/shmem_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,20 @@ extern unsigned int shmem_internal_rand_seed;
} \
} while (0)

#define SHMEM_ERR_CHECK_SIG_OP(op) \
do { \
switch(op) { \
case SHMEMX_SIGNAL_SET: \
case SHMEMX_SIGNAL_ADD: \
break; \
default: \
fprintf(stderr, "ERROR: %s(): Argument \"%s\", " \
"invalid atomic operation for signal (%d)\n", \
__func__, #op, (int) (op)); \
jdinan marked this conversation as resolved.
Show resolved Hide resolved
shmem_runtime_abort(100, PACKAGE_NAME " exited in error"); \
} \
} while (0)

#else
#define SHMEM_ERR_CHECK_INITIALIZED()
#define SHMEM_ERR_CHECK_POSITIVE(arg)
Expand All @@ -314,6 +328,7 @@ extern unsigned int shmem_internal_rand_seed;
#define SHMEM_ERR_CHECK_SYMMETRIC_HEAP(ptr)
#define SHMEM_ERR_CHECK_NULL(ptr, nelems)
#define SHMEM_ERR_CHECK_CMP_OP(op)
#define SHMEM_ERR_CHECK_SIG_OP(op) \

#endif /* ENABLE_ERROR_CHECKING */

Expand Down
Loading