Skip to content
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

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

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

bwpriest
Copy link
Member

…sed on some platforms.

@bwpriest
Copy link
Member Author

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

@bwpriest
Copy link
Member Author

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

@steiltre
Copy link
Collaborator

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
Copy link
Member

KIwabuchi commented Dec 21, 2023

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?

@steiltre steiltre changed the base branch from develop to v0.7-dev August 8, 2024 20:34
@steiltre steiltre changed the base branch from v0.7-dev to develop August 8, 2024 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants