pass MPI_Comm by value #15159
pass MPI_Comm by value #15159
Conversation
|
More precisely, all MPI data types need to be (i) PODs -- that's just C -- and (ii) the object itself does not own resources, and so can be trivially copied. This does not imply that the data type is small and can be cheaply copied, but in practice the only two ways the second requirement can usefully be implemented is to store most of the information in tables that the MPI implementation owns and manages, and the MPI data types only store either pointers or indices into these tables. I don't have a particular opinion about whether we should pass references or values. The patch is ok with me (but I'm not ethusiastic about it either); I'll let others chime in. |
|
You probably want a changelog entry. |
|
Can you add the changelog entry? |
|
updated. |
doc/news/changes/minor/20230508tjhei
Outdated
| @@ -0,0 +1,5 @@ | |||
| Changed: MPI communicators (of type `MPI_Comm`) are not treated as | |||
doc/news/changes/minor/20230508tjhei
Outdated
| @@ -0,0 +1,5 @@ | |||
| Changed: MPI communicators (of type `MPI_Comm`) are not treated as | |||
| trivially copyable POD types throughout the library. This means that | |||
| we store and pass them by value and by reference. | |||
There was a problem hiding this comment.
and here
| we store and pass them by value and by reference. | |
| we store and pass them by value instead of by reference. |
?
There was a problem hiding this comment.
somebody was clearly half asleep when he wrote these two sentences in the changelog entry. :-)
Based on the recent discussion to return MPI_Comm by value from respective functions (see #14599 #15086 ), I think it makes sense to treat MPI_Comm as in int-like structure (instead of a class).
This is consistent with how the MPI standard is defined (int the API MPI_Comm is passed by value).
OpenMPI defines MPI_Comm to be a pointer: https://github.com/open-mpi/ompi/blob/5d9186207e63625091c292fe6425f3178ce4245d/ompi/include/mpi.h.in#L447
MPICH has it defined as an int:
https://github.com/pmodels/mpich/blob/eb36ab149dacf1870834b1c227f4e516eeec248d/src/include/mpi.h.in#L279