SHMEM_HEAP_TYPE constant is undefined, which causes compile error. Ch… #17
Conversation
…anged to read it as a system env var.
…mem_broadcast() to shmem_broadcast64(). Note that shmem_broadcast64 tests already exist; this has been reported as an issue
… calls. Also changed undefinded _SHMEM_ALLTOALL_SYNC_SIZE constant to SHMEM_ALLTOALL_SYNC_SIZE.
| @@ -49,7 +49,7 @@ static int memheap_type(void) | |||
| if (memheap_type != MEMHEAP_ALLOC_UNKNOWN) | |||
| return memheap_type; | |||
|
|
|||
| p = getenv(SHMEM_HEAP_TYPE); | |||
| p = getenv("SHMEM_HEAP_TYPE"); | |||
There was a problem hiding this comment.
SHMEM_HEAP_TYPE constant is part of spec, cannot be just string
see here:
https://github.com/open-mpi/ompi/blob/master/oshmem/include/shmem.h.in#L76
There was a problem hiding this comment.
I see it in the implementation (code), but i dont see it defined (in the specs): https://github.com/openshmem-org/specification,
OpenSHMEM-1.3.pdf
Is this constant unique to that flavor of openShmem?
There was a problem hiding this comment.
This looks like a specification compliance bug in the OSHMEM implementation. The SHMEM_HEAP_TYPE constant is not part of OpenSHMEM. Added this to the open ticket filed with OMPI (open-mpi/ompi#4098).
There was a problem hiding this comment.
I think this change is incorrect, although benign. SHMEM_HEAP_TYPE is a string constant in the OMPI header file. I think the right fix is to replace this with SHMEM_SYMMETRIC_HEAP_ALLOCATOR, which is the value of the SHMEM_HEAP_TYPE constant.
There was a problem hiding this comment.
yep, seems you are right
in ompi shmem.h it is defined as #define SHMEM_HEAP_TYPE "SHMEM_SYMMETRIC_HEAP_ALLOCATOR"
@ireed - can you please submit PR and use SPEC constant directly, thanks
|
@miked-mellanox Anything we can do to help with this PR? Thanks! |
…anged to read it as a system env var.