OSHMEM: v1.3: adds shmem_fetch and shmem_set AMOs #2103

alex-mikheev · 2016-09-22T07:38:56Z

The commit adds atomic set and fetch functions as described in
oshmem 1.3 spec.

jsquyres · 2016-09-22T10:12:04Z

@alex-mikheev Please rebase this to the top of master to fix the Travis failure. Please also set a milestone and appropriate labels. Thanks!

igor-ivanov

I think that we should add shmemx_ functions

igor-ivanov · 2016-09-22T13:09:32Z

oshmem/include/pshmem.h

+OSHMEM_DECLSPEC void pshmem_longlong_set(long long*target, long long value, int pe);
+OSHMEM_DECLSPEC void pshmemx_int32_set(int32_t *target, int32_t value, int pe);
+OSHMEM_DECLSPEC void pshmemx_int64_set(int64_t *target, int64_t value, int pe);
+


@alex-mikheev could you consider following ordering as:
char,short,int,long,long long, float, double, long double to improve readability.

igor-ivanov · 2016-09-22T13:11:13Z

oshmem/include/pshmem.h

+OSHMEM_DECLSPEC void pshmem_int_set(int *target, int value, int pe);
+OSHMEM_DECLSPEC void pshmem_long_set(long *target, long value, int pe);
+OSHMEM_DECLSPEC void pshmem_longlong_set(long long*target, long long value, int pe);
+OSHMEM_DECLSPEC void pshmemx_int32_set(int32_t *target, int32_t value, int pe);


v1.3 does not have shmex calls for this set of function.
I think shmex_ should be removed.

igor-ivanov · 2016-09-22T13:12:24Z

oshmem/include/pshmem.h

+OSHMEM_DECLSPEC long long pshmem_longlong_fetch(const long long *target, int pe);
+OSHMEM_DECLSPEC double pshmem_double_fetch(const double *target, int pe);
+OSHMEM_DECLSPEC float pshmem_float_fetch(const float *target, int pe);
+OSHMEM_DECLSPEC int32_t pshmemx_int32_fetch(const int32_t *target, int pe);


v1.3 does not have shmex calls for this set of function.
I think shmex_ should be removed.

igor-ivanov · 2016-09-22T13:13:10Z

oshmem/include/pshmem.h

+OSHMEM_DECLSPEC long pshmem_long_fetch(const long *target, int pe);
+OSHMEM_DECLSPEC long long pshmem_longlong_fetch(const long long *target, int pe);
+OSHMEM_DECLSPEC double pshmem_double_fetch(const double *target, int pe);
+OSHMEM_DECLSPEC float pshmem_float_fetch(const float *target, int pe);


please change float and double order

igor-ivanov · 2016-09-22T13:13:39Z

oshmem/include/shmem.h.in

+OSHMEM_DECLSPEC void shmem_float_set(float *target, float value, int pe);
+OSHMEM_DECLSPEC void shmem_int_set(int *target, int value, int pe);
+OSHMEM_DECLSPEC void shmem_long_set(long *target, long value, int pe);
+OSHMEM_DECLSPEC void shmem_longlong_set(long long*target, long long value, int pe);


@alex-mikheev could you consider following ordering as:
char,short,int,long,long long, float, double, long double to improve readability.

igor-ivanov · 2016-09-22T13:23:09Z

oshmem/shmem/c/shmem_fetch.c

+/*
+ * These routines perform an atomic fetch operation.
+ * The fetch and add routines retrieve the value at address target on PE pe.
+ * The operation must be completed without the possibility of another process 


incorrect description

igor-ivanov · 2016-09-22T13:24:15Z

oshmem/shmem/c/shmem_fetch.c

+            (void*)target,                                          \
+            (void*)&out_value,                                      \
+            (const void*)&value,                                    \
+            size,                                                   \


is target updated with 0 in this call?

zero is added to the target

igor-ivanov · 2016-09-22T13:25:14Z

oshmem/shmem/c/shmem_fetch.c

+#pragma weak shmem_longlong_fetch = pshmem_longlong_fetch
+#pragma weak shmem_double_fetch = pshmem_double_fetch
+#pragma weak shmem_float_fetch = pshmem_float_fetch
+#pragma weak shmemx_int32_fetch = pshmemx_int32_fetch


do we need shmemx_?

igor-ivanov · 2016-09-22T13:58:02Z

oshmem/shmem/man/man3/Makefile.extra

+		shmem/man/man3/shmem_int_set.3 \
+		shmem/man/man3/shmem_longlong_set.3 \
+		shmem/man/man3/shmem_long_set.3 \
+\


@alex-mikheev fix alignment

igor-ivanov · 2016-09-22T14:04:20Z

oshmem/shmem/man/man3/Makefile.extra

+		shmem/man/man3/shmem_int_fetch.3 \
+		shmem/man/man3/shmem_long_fetch.3 \
+		shmem/man/man3/shmem_longlong_fetch.3 \
+\
        shmem/man/man3/shmem_int_finc.3 \


@alex-mikheev fix alignment

igor-ivanov · 2016-09-23T09:16:01Z

@alex-mikheev please ignore I think that we should add shmemx_ functions review general comment. Should be I think that we should not add shmemx_ functions

alex-mikheev · 2016-09-23T12:18:52Z

I added shmemx_ functions because we already have them for atomic swap. So it it reasonable to have such functions for the set and fetch. Or remove them all.

jladd-mlnx · 2016-09-23T14:49:33Z

👍 This looks good.

alex-mikheev · 2016-09-25T08:25:37Z

Actually we have shmemx_int(32|64) extensions for pretty much everything. If we want to remove them all it should be done in separate PR. Meanwhile it is reasonable that shmem_set/shmem_fetch also have such extenstion.

The commit adds atomic set and fetch functions as described in oshmem 1.3 spec.

igor-ivanov · 2016-09-26T07:54:21Z

@alex-mikheev I do not know a reason we added shmemx_ functions in previous code. Do you know if there are requests from someone? Personally I prefer to avoid having one w/o any needs.

alex-mikheev · 2016-09-26T08:00:39Z

i do not remember why it was added

jladd-mlnx · 2016-09-27T00:18:00Z

@igor-ivanov I'm going to merge this. If you guys decide to remove the shmemx_ functions, do it as a separate PR.

OSHMEM v1.3: c11 generics (DNM before #2103)

…etch_set

igor-ivanov reviewed Sep 22, 2016

View reviewed changes

jsquyres mentioned this pull request Sep 22, 2016

v2.1.0: OSHMEM v1.3 API updates #2109

Closed

alex-mikheev added the enhancement label Sep 22, 2016

alex-mikheev added this to the v2.1.0 milestone Sep 22, 2016

alex-mikheev self-assigned this Sep 22, 2016

OSHMEM: v1.3: adds shmem_fetch and shmem_set AMOs

3a03435

The commit adds atomic set and fetch functions as described in oshmem 1.3 spec.

alex-mikheev force-pushed the topic/oshmem_amo_fetch_set branch from 7f1fb96 to 3a03435 Compare September 25, 2016 09:05

alex-mikheev mentioned this pull request Sep 25, 2016

OSHMEM v1.3: c11 generics (DNM before #2103) #2122

Merged

jladd-mlnx merged commit 13e53b4 into open-mpi:master Sep 27, 2016

jladd-mlnx added a commit that referenced this pull request Sep 27, 2016

Merge pull request #2122 from alex-mikheev/topic/oshmem_c11_generics

bf6c887

OSHMEM v1.3: c11 generics (DNM before #2103)

clementFoyer pushed a commit to bosilca/ompi that referenced this pull request Nov 4, 2016

Merge pull request open-mpi#2103 from alex-mikheev/topic/oshmem_amo_f…

6ad2362

…etch_set

OSHMEM: v1.3: adds shmem_fetch and shmem_set AMOs #2103

OSHMEM: v1.3: adds shmem_fetch and shmem_set AMOs #2103

Uh oh!

Conversation

alex-mikheev commented Sep 22, 2016

Uh oh!

jsquyres commented Sep 22, 2016

Uh oh!

igor-ivanov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

igor-ivanov commented Sep 23, 2016

Uh oh!

alex-mikheev commented Sep 23, 2016

Uh oh!

jladd-mlnx commented Sep 23, 2016

Uh oh!

alex-mikheev commented Sep 25, 2016

Uh oh!

igor-ivanov commented Sep 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alex-mikheev commented Sep 26, 2016

Uh oh!

jladd-mlnx commented Sep 27, 2016

Uh oh!

Uh oh!

igor-ivanov commented Sep 26, 2016 •

edited

Loading