LLNL / ygm

Other
31 stars 22 forks source link

explicitly defining an MPI_Datatype for size_t because gcc gets confu… #155

Open bwpriest opened 1 year ago

bwpriest commented 1 year ago

…sed on some platforms.

bwpriest commented 1 year ago

I see. This fix is not portable either. I guess I need to wrap it with precompiler guards or something.

bwpriest commented 1 year ago

Looks like the test that failed did so because the Arrow fetch barfed, @steiltre. I think that this is now ready to merge.

steiltre commented 1 year ago

The mpi_typeof<uint64_t> should be added back in. It explicitly names a type that is 64 bits. The others are platform-dependent and won't necessarily match with MPI_UINT64_t. MPI has MPI_UNSIGNED_LONG and MPI_UNSIGNED_LONG_LONG to match with whatever sizes the unsigned long int and unsigned long long int are on a given system.

A list of these types in MPI can be found here.

KIwabuchi commented 11 months ago

To solve this issue, how about utilizing if-constexpr? I tried to solve the issue following the template specialization technique, but I failed... So, I implemented a if-constexpr version. I was able to build the same if-constexpr code on Linux as well as MacOS, where GCC distinguishes size_t from uint64_t.

Here is an example mpi_typeof function that uses if-constexpr:

template <typename T>
inline constexpr MPI_Datatype mpi_typeof(T) {
  if constexpr (std::is_same_v<T, char>) {
    return MPI_CHAR;
  } else if constexpr (std::is_same_v<T, bool>) {
    return MPI_CXX_BOOL;
  } else if constexpr (std::is_same_v<T, int8_t>) {
    return MPI_INT8_T;
  } else if constexpr (std::is_same_v<T, int16_t>) {
    return MPI_INT16_T;
  } else if constexpr (std::is_same_v<T, int32_t>) {
    return MPI_INT32_T;
  } else if constexpr (std::is_same_v<T, int64_t>) {
    return MPI_INT64_T;
  } else if constexpr (std::is_same_v<T, uint8_t>) {
    return MPI_UINT8_T;
  } else if constexpr (std::is_same_v<T, uint16_t>) {
    return MPI_UINT16_T;
  } else if constexpr (std::is_same_v<T, uint32_t>) {
    return MPI_UINT32_T;
  } else if constexpr (std::is_same_v<T, uint64_t> ||
                       std::is_same_v<T, std::size_t>) {
    return MPI_UINT64_T;
  } else if constexpr (std::is_same_v<T, float>) {
    return MPI_FLOAT;
  } else if constexpr (std::is_same_v<T, double>) {
    return MPI_DOUBLE;
  } else if constexpr (std::is_same_v<T, long double>) {
    return MPI_LONG_DOUBLE;
  } else {
    // Thanks to 'constexpr', the following static_assert does not assert
    // as long as 'T' is one of the supported types above.
    static_assert(always_false<>, "Unkown MPI Type");
  }

  return 0;
}

What do you think?