`MPI_KEYVAL_INVALID` is not a constant

[blue42u]

MPItrampoline defines MPI_KEYVAL_INVALID publicly as an extern const int, making it impossible to use as a static initializer in C (not C++). Reproducer:

$ cat repro.c
#include <mpi.h>
static int key = MPI_KEYVAL_INVALID;
$ .../mpicc -c repro.c
repro.c:2:18: error: initializer element is not constant
    2 | static int foo = MPI_KEYVAL_INVALID;
      |                  ^~~~~~~~~~~~~~~~~~

This error is in conflict with the MPI standard, which explicitly states that MPI_KEYVAL_INVALID can be used for static initialization (MPI 4.0 § 7.7.2):

The special key value MPI_KEYVAL_INVALID is never returned by MPI_COMM_CREATE_KEYVAL. Therefore, it can be used for static initialization of key values.

The standard also includes example code using MPI_KEYVAL_INVALID this way (MPI 4.0 § 7.7.6):

/* key for this module’s stuff: */
static int gop_key = MPI_KEYVAL_INVALID;

The above language and example have persisted verbatim in the MPI standard since MPI 2.0.

[eschnett]

You are correct: This is unfortunately a design flaw in MPItrampoline. It is not easy to remedy this (but there is an effort underway to define an MPI ABI as part of the MPI standard.)

In the mean time I can offer a work-around that may or may not acceptable to you: Leave the static variable uninitialized, and define a "constructor" function that is automatically run at start-up. This is not part of the C standard, but rather the ELF standard that defines object files on Unix (Linux, BSD, macOS, etc.). This would look like this:

static int gop_key;
static void __attribute__((__constructor__)) gop_key_init() {
  gop_key = MPI_KEYVAL_INVALID;
}

(Building with C++ would also remedy this.)

[blue42u]

(An MPI ABI would make my life so much easier... it cannot happen soon enough!)

I don't personally mind this workaround, but I do not want to patch every MPI program I build to avoid this quirk. IMHO MPItrampoline should really just be standards-compliant here by whatever means necessary.

AFAICT from the MPI 4.0 standard, the value of MPI_KEYVAL_INVALID is rather immaterial. Most of the functions that take a comm_keyval input include this language:

The call is erroneous if there is no key with value keyval. [...] In particular MPI_KEYVAL_INVALID is an erroneous key value.

The exceptions to this rule are MPI_COMM_DELETE_ATTR and MPI_COMM_DELETE_KEYVAL, which do not explicitly state whether passing an arbitrary/nonexistent keyval is erroneous or not. The only place MPI_KEYVAL_INVALID appears in the ABI is with this language:

[MPI_COMM_DELETE_KEYVAL] sets the value of keyval to MPI_KEYVAL_INVALID.

So, would it be possible for MPItrampoline to #define an arbitrary value for MPI_KEYVAL_INVALID? The way I see it, it's either the app's fault for passing an arbitrary/nonexistent keyval (if defined to be erroneous), or the wrapped MPI implementation's fault for not handling that case (if defined to be not erroneous).

[eschnett]

The longer-term plan is to have a translation layer for these constants. This way, MPItrampoline can define the constants, and MPIwrapper will need to translate them. In most cases this will be very quick. However, this is a non-trivial implementation effort and I don't have the time to start this at the moment.

In my experience, most packages do not need work-around like this (otherwise I would have found this problem much earlier). That statement is, of course, biased towards the set of packages that I'm using in my own work...

[cmhamel]

This is also giving me issues when trying to build PnetCDF against mpitrampoline. It's mainly one function where mpi constants (not really constants with mpitrampoline) are in a switch block, thus leading to a compiler error. I could patch it, but that seems like not the right thing to do to me.

[eschnett]

According to the MPI standard this would be the right thing. However, given current practice I now think the MPI standard should be updated to adapt to current practice (or should make it very clear that such usage is not allowed).

[blue42u]

IMHO the standard will never lift these compile-time constants to link-time constants, they generally try to preserve backwards-compatibility for applications across MPI versions. What seems more likely (based on https://github.com/mpiwg-abi/abi-issues/issues/1) is that the MPI standard will #define the values for constants as part of the standardized ABI (MPI 4.1-ish).

[eschnett]

I wanted to suggest that the standard should do just that, namely define them to be proper compile-time constants instead of link-time constants since that is what many applications expect and what most MPI implementations support.