open-mpi / ompi

Open MPI main development repository
https://www.open-mpi.org
Other
2.18k stars 861 forks source link

shmem.h uses C++ <complex> type for C++, but standard uses _Complex from <complex.h> #6728

Open sam6258 opened 5 years ago

sam6258 commented 5 years ago

Background information

What version of Open MPI are you using? (e.g., v1.10.3, v2.1.0, git branch name and hash, etc.)

master, v4.0.x, v3.1.x, v3.0.x. Didn't check older versions.

Describe how Open MPI was installed (e.g., from a source/distribution tarball, from a git clone, from an operating system distribution package, etc.)

Built from source

Please describe the system on which you are running


Details of the problem

In the Sandia openshmem tests, there is a C++ test that tries to use a call to the complex reduction APIs (like shmem_complexf_sum_to_all() )using the _Complex type from <complex.h>. Here is the failure:

> oshc++ -o cxx_test_shmem_complex cxx_test_shmem_complex.cpp
cxx_test_shmem_complex.cpp:88:3: error: no matching function for call to 'shmem_complexf_sum_to_all'
  TEST_COMPLEX(float,f,sum);
  ^~~~~~~~~~~~~~~~~~~~~~~~~
cxx_test_shmem_complex.cpp:62:5: note: expanded from macro 'TEST_COMPLEX'
    shmem_complex##LETTER##_##OP##_to_all(TYPE##_dest,TYPE##_src,10, \
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
<scratch space>:63:1: note: expanded from here
shmem_complexf_sum_to_all
^~~~~~~~~~~~~~~~~~~~~~~~~
/gpfs0/Builds/Urial_dev9/exports/include/shmem.h:1769:22: note: candidate function not viable: no known conversion from '_Complex float [10]' to 'std::complex<float> *' for 1st argument
OSHMEM_DECLSPEC void shmem_complexf_sum_to_all(OSHMEM_COMPLEX_TYPE(float) *target, const OSHMEM_COMPLEX_TYPE(float) *source, int nreduce, int PE_start, int logPE_stride, int PE_size, OSHMEM_COMPLEX_TYPE(float) *pWrk, long *pSync);
                     ^
cxx_test_shmem_complex.cpp:89:3: error: no matching function for call to 'shmem_complexf_prod_to_all'
  TEST_COMPLEX(float,f,prod);

The problem is that the shmem.h header expects the C++ <complex> type when C++ is used:

#if defined(c_plusplus) || defined(__cplusplus)
#    include <complex>
#    define OSHMEM_COMPLEX_TYPE(type)    std::complex<type>
#else
#    include <complex.h>
#    define OSHMEM_COMPLEX_TYPE(type)    type complex
#endif

When I change the header to:

#    include <complex.h>
#    define OSHMEM_COMPLEX_TYPE(type)    type complex

The compile will work.

The standard, though, specifies that the <complex.h> complex type be used for both C & C++ - (http://openshmem.org/site/sites/default/site_files/OpenSHMEM-1.4.pdf - p. 80) :

9.8.7.4 SUM C/C++: Performs a sum reduction across a set of PEs. void shmem_complexd_sum_to_all(double _Complex dest, const double _Complex source, int nreduce, int PE_start, int logPE_stride, int PE_size, double _Complex pWrk, long pSync); void shmem_complexf_sum_to_all(float _Complex dest, const float _Complex source, int nreduce, int PE_start, int logPE_stride, int PE_size, float _Complex pWrk, long pSync);

While it is understandable that a user of C++ would want to use the C++ complex types, the specification only has the _Complex type. There are some things you can do to allow for both. I started going down this route, but there are a couple of catchas, like both headers use the complex keyword, and it is #defined in complex.h. The easy fix is to just use the _Complex from <complex.h> type.

Thoughts on this?

hppritcha commented 3 years ago

@jladd-mlnx are there any plans to fix this? I thought c++ support for OpenSHMEM was going away at some point anyway.

hppritcha commented 3 years ago

removing old target labels as this issue will not be addressed in those releases.