openshmem-org / specification

OpenSHMEM Application Programming Interface
http://www.openshmem.org
51 stars 41 forks source link

Undefined behavior in peer-to-peer synchronization routines #413

Closed jamesaross closed 4 years ago

jamesaross commented 4 years ago

It seems to me that every peer-to-peer synchronization routine that includes a status array defines behavior for element values of 0 or 1, but not other values. Wouldn't it be better to define the behavior in terms of 0 and non-zero? This should be a simple change, repeated 12 times.

Relevant routines (plus C11 Generics):

void shmem_TYPENAME_wait_until_all(TYPE *ivars, size_t nelems, const int *status, int cmp, TYPE cmp_value);
size_t shmem_TYPENAME_wait_until_any(TYPE *ivars, size_t nelems, const int *status, int cmp, TYPE cmp_value);
size_t shmem_TYPENAME_wait_until_some(TYPE *ivars, size_t nelems, size_t *indices, const int *status, int cmp, TYPE cmp_value);
void shmem_TYPENAME_wait_until_all_vector(TYPE *ivars, size_t nelems, const int *status, int cmp, TYPE *cmp_values);
size_t shmem_TYPENAME_wait_until_any_vector(TYPE *ivars, size_t nelems, const int *status, int cmp, TYPE *cmp_values);
size_t shmem_TYPENAME_wait_until_some_vector(TYPE *ivars, size_t nelems, size_t *indices, const int *status, int cmp, TYPE *cmp_values);
int shmem_TYPENAME_test_all(TYPE *ivars, size_t nelems, const int *status, int cmp, TYPE cmp_value);
size_t shmem_TYPENAME_test_any(TYPE *ivars, size_t nelems, const int *status, int cmp, TYPE cmp_value);
size_t shmem_TYPENAME_test_some(TYPE *ivars, size_t nelems, size_t *indices, const int *status, int cmp, TYPE cmp_value);
int shmem_TYPENAME_test_all_vector(TYPE *ivars, size_t nelems, const int *status, int cmp, TYPE *cmp_values);
size_t shmem_TYPENAME_test_any_vector(TYPE *ivars, size_t nelems, const int *status, int cmp, TYPE *cmp_values);
size_t shmem_TYPENAME_test_some_vector(TYPE *ivars, size_t nelems, size_t *indices, const int *status, int cmp, TYPE *cmp_values);
jdinan commented 4 years ago

@davidozog Interested to hear your thoughts?

davidozog commented 4 years ago

@jdinan It seems like a good point to me, thanks for noticing that @jamesaross. We could state that all values other than 0 and 1 are invalid; but if it's an acceptable change at this point, I wouldn't mind going through and changing "1" to "nonzero" to avoid undefined behavior altogether. Could this be a doc edit?

naveen-rn commented 4 years ago

Can we close this issue? https://github.com/naveen-rn/specification/pull/16/files Fixes this issue.

jamesaross commented 4 years ago

Thank you, @davidozog and @naveen-rn.