-
Notifications
You must be signed in to change notification settings - Fork 901
Compilation fails for powerpc with --enable-mpi-cxx #2055
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.
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
Comments
@nysal - can you or someone on your team look at this? |
This isn't PowerPC specific. I used C99 in a header exposed to c++. I need to figure out a way to protect internal headers from c++ until the bindings are removed. will look at this today. |
@hjelmn and I chatted on the call today -- he has an interesting proposal to fix this: require a C++11 compiler for the C++ MPI bindings. My gut reaction is "we can't do that!", but Nathan raises the valid point that the MPI C++ bindings have been removed from the MPI 3.0 spec. So raising the bar a little to compile the MPI C++ bindings may not be horrible. Need to think about this a little bit... |
Yeah, if we do C++11 I think it is ok to include any of our internal headers to C++. My understanding is many of the C99 features we use in Open MPI are available in C++11. |
I can't get g++ 6.1 to accept C99 subobject naming. Guess I was wrong that it is supported. Working on another workaround now. |
This commit removes the #include of errhandler/errhandler.h (which includes opal_object.h). The inlude was only needed to get the extern declaration of ompi_mpi_errors_throw_exceptions. Since we have the ompi_predefined_errhandler_t in mpi.h we can declare the extern in mpicxx.c without needing the C errhandler header. This should fix open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]>
This commit adds some glue code to support the C++ bindings and updates the bindings to use the new glue code. This protects our internal headers (which are C99) from C++. This is done as a quick workaround to compilation errors when the legacy C++ bindings are requested. Fixes open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]>
This commit adds some glue code to support the C++ bindings and updates the bindings to use the new glue code. This protects our internal headers (which are C99) from C++. This is done as a quick workaround to compilation errors when the legacy C++ bindings are requested. Fixes open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]>
This commit adds some glue code to support the C++ bindings and updates the bindings to use the new glue code. This protects our internal headers (which are C99) from C++. This is done as a quick workaround to compilation errors when the legacy C++ bindings are requested. Fixes open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]>
This commit adds some glue code to support the C++ bindings and updates the bindings to use the new glue code. This protects our internal headers (which are C99) from C++. This is done as a quick workaround to compilation errors when the legacy C++ bindings are requested. Fixes open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]>
This commit adds some glue code to support the C++ bindings and updates the bindings to use the new glue code. This protects our internal headers (which are C99) from C++. This is done as a quick workaround to compilation errors when the legacy C++ bindings are requested. Fixes open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]> (cherry picked from commit c6464ca)
This commit adds some glue code to support the C++ bindings and updates the bindings to use the new glue code. This protects our internal headers (which are C99) from C++. This is done as a quick workaround to compilation errors when the legacy C++ bindings are requested. Fixes open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]> (cherry picked from commit c6464ca)
This commit adds some glue code to support the C++ bindings and updates the bindings to use the new glue code. This protects our internal headers (which are C99) from C++. This is done as a quick workaround to compilation errors when the legacy C++ bindings are requested. Fixes open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]> (cherry picked from commit c6464ca)
This commit adds some glue code to support the C++ bindings and updates the bindings to use the new glue code. This protects our internal headers (which are C99) from C++. This is done as a quick workaround to compilation errors when the legacy C++ bindings are requested. Fixes open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]> (cherry picked from commit c6464ca)
Just tried to build master (it passes) and the v2.0.x branch (it fails the same way as described here) on ppc64 Fedora 25. Has been the backport from master complete? |
Still the same problem with the opal_atomics(), I'm currently bisecting the 2.0.x branch to find the problematic change. |
and this is the result of my bisecting |
and for the record also the error message
|
Yup. We use c99 internally and the c++ was exposed to it. Some compilers do ok but others do not. We fixed it by isolating the legacy c++ bindings from the c99 on master and now 2.x and 2.0.x. |
@hjelmn He's saying it's not fixed. |
Respectively it's fixed in master, but it is not in the 2.0.x branch, haven't tried 2.x branch yet. |
The difference between master and v2.0.x branch is that the problematic |
Looks like we have an erroneous include in intercepts.cc still. The powerpc header is not used by default in master because we made (a perhaps poor) decision to use the builtin compiler atomics by default. The powerpc/atomic.h header is legal C99 which is not supported by C++. |
FYI, the C++ bindings in MPI were removed in MPI-3. We will do what is needed to keep them compiling in 2.x but they may not survive to 3.0. |
Fixes open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]>
Try patching with the commit in #2317 and see if that does it. |
This commit adds some glue code to support the C++ bindings and updates the bindings to use the new glue code. This protects our internal headers (which are C99) from C++. This is done as a quick workaround to compilation errors when the legacy C++ bindings are requested. Fixes open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]>
This commit adds some glue code to support the C++ bindings and updates the bindings to use the new glue code. This protects our internal headers (which are C99) from C++. This is done as a quick workaround to compilation errors when the legacy C++ bindings are requested. Fixes open-mpi#2055 Signed-off-by: Nathan Hjelm <[email protected]>
Per comment on #2317 there is still more work to do (checking against the v2.1.0 tarball with that patch) |
@jjhursey any updates on this issue? |
I can confirm that this is fixed in
We can close this issue, and I'll followup with the results of the release branch testing. |
I verified correct compilation with |
I can confirm I got a successful build with --enable-mpi-cxx on ppc64 from the v2.0.x branch, thanks to all. |
Hi,
Enabling --enable-mpi-cxx leads to:
This fails because in opal/include/opal/sys/atomic.h:
and opal_atomic_swap_32 is conditionally only included if opal_atomic_swap_32 is set
The text was updated successfully, but these errors were encountered: