libMesh / TIMPI

Templated Interface to MPI
GNU Lesser General Public License v2.1
12 stars 11 forks source link

Compile error with NVCC + OpenMPI #122

Closed NamjaeChoi closed 1 year ago

NamjaeChoi commented 1 year ago

I'm seeing the following error in request.h when compiling a code with NVCC and OpenMPI:

/home/choin/hoodoo/install/moose/libmesh/installed/include/timpi/request.h(112): error: expression must have a constant value

It seems that the definition of MPI_REQUEST_NULL in OpenMPI is not considered compile-time constant by NVCC, which is:

OMPI_DECLSPEC extern struct ompi_predefined_request_t ompi_request_null;
#define OMPI_PREDEFINED_GLOBAL(type, global) (static_cast<type> (static_cast<void *> (&(global))))
#define MPI_REQUEST_NULL OMPI_PREDEFINED_GLOBAL(MPI_Request, ompi_request_null)

We might need a better way of defining null_request that is compatible across different MPI variants.

roystgnr commented 1 year ago

We might have overreached by declaring null_request as constexpr. Could you try just using static const and see if NVidia is any happier?

I did some work on nvc++/libMesh compatibility late last year, but while still working on serial builds I got thrown by a unit test that their optimizer miscompiled (they'd been pretty fast about fixing other issues I reported, but not this one) and I haven't looked at it again in months. Their GPGPU-via-C++17 work is really exciting and I'd love to find out that we could use it.

NamjaeChoi commented 1 year ago

Thanks for a quick reply, but that leads to another failure: /home/choin/hoodoo/install/moose/libmesh/installed/include/timpi/request.h:112:24: error: ‘constexpr’ needed for in-class initialization of static data member ‘ompi_request_t* const TIMPI::Request::null_request’ of non-integral type [-fpermissive] 112 | static const request null_request = MPI_REQUEST_NULL; | ^~~~~~~~~~~~

roystgnr commented 1 year ago

Oh, of course it does. :-P I should have known I picked constexpr for some reason better than "I got suddenly and weirdly specifically concerned with microoptimizations". We're going to have to move the initialization to request.C.

NamjaeChoi commented 1 year ago

Is the following what you suggest?

diff --git a/src/parallel/include/timpi/request.h b/src/parallel/include/timpi/request.h
index 3b555b9..9afc672 100644
--- a/src/parallel/include/timpi/request.h
+++ b/src/parallel/include/timpi/request.h
@@ -108,11 +108,7 @@ public:
   // made from \p this have been cleaned up.
   void add_post_wait_work(PostWaitWork * work);

-#ifdef TIMPI_HAVE_MPI
-  static constexpr request null_request = MPI_REQUEST_NULL;
-#else
-  static constexpr request null_request = 0;
-#endif
+  static const request null_request;

 private:
   request _request;
diff --git a/src/parallel/src/request.C b/src/parallel/src/request.C
index 5494d72..77a97d7 100644
--- a/src/parallel/src/request.C
+++ b/src/parallel/src/request.C
@@ -37,6 +37,12 @@
 namespace TIMPI
 {

+#ifdef TIMPI_HAVE_MPI
+const request Request::null_request = MPI_REQUEST_NULL;
+#else
+const request Request::null_request = 0;
+#endif
+
 // ------------------------------------------------------------
 // Request member functions
 Request::Request () :