mpi_f08: fix Fortran-8-byte-INTEGER vs. C-4-byte-int issue

[jsquyres]

It is important to have the mpi_f08 Type(MPI_Status) be the same
length (in bytes) as the mpif.h status (which is an array of
MPI_STATUS_SIZE INTEGERs). The reason is because MPI_Status_ctof()
basically does the following:

MPI_Fint *f_status = ...;
int *s = (int*) &c_status;
for i=0..sizeof(MPI_Status)/sizeof(int)
   f_status[i] = c_status[i];

Meaning: the Fortran status needs to be able to hold as many INTEGERs
are there are C int's that can fit in sizeof(MPI_Status) bytes.

This is because a Fortran INTEGER may be larger than a C int (e.g.,
Fortran 8 bytes vs. C 4 bytes). Hence, the assignment on the Fortran
side will take sizeof(INTEGER) bytes for each sizeof(int) bytes in the
C MPI_Status.

This commit pads out the mpi_f08 Type(MPI_Status) with enough INTEGERs
to make it the same size as an array of MPI_TYPE_SIZE INTEGERs.
Hence, MPI_Status_ctof() will work properly, regardless of whether it
is assinging to an mpi_f08 Type(MPI_Status) or an mpif.h array of
MPI_STATUS_SIZE INTEGERs.

Thanks to @ahaichen for reporting the issue.

Signed-off-by: Jeff Squyres [email protected]

Refs #7918

[ggouaillardet]

@jsquyres on top of my head

• have you checked this new definition of type(MPI_Status) is valid w.r.t. the MPI standard?
• this PR will break the ABI, right?
• my first intent was to preserve the current definition of type(MPI_Status) (and do not break the ABI), and update the use-mpi-f08 subroutines to something like this

subroutine MPI_Recv_f08(buf,count,datatype,source,tag,comm,status,ierror)
   use :: mpi_f08_types, only : MPI_Datatype, MPI_Comm, MPI_Status
   use :: ompi_mpifh_bindings, only : empi_recv_f
   implicit none
   OMPI_FORTRAN_IGNORE_TKR_TYPE :: buf
   INTEGER, INTENT(IN) :: count, source, tag
   TYPE(MPI_Datatype), INTENT(IN) :: datatype
   TYPE(MPI_Comm), INTENT(IN) :: comm
   TYPE(MPI_Status), INTENT(OUT) :: status
   INTEGER :: f_status(MPI_STATUS_SIZE)
   INTEGER, OPTIONAL, INTENT(OUT) :: ierror
   integer :: c_ierror

   call ompi_recv_f(buf,count,datatype%MPI_VAL,source,tag,comm%MPI_VAL,f_status,c_ierror)
   if (c_ierror.EQ.MPI_SUCCESS .AND. status.NE.MPI_STATUS_IGNORE) call PMPI_Status_ftof08(f_status, status, c_ierror)
   if (present(ierror)) ierror = c_ierror

end subroutine MPI_Recv_f08

note we could/should use macros to skip the PMPI_Status_ftof08() invocation when the Fortran and Fortran2008 statuses have the same size.

[jsquyres]

@jsquyres on top of my head

• have you checked this new definition of type(MPI_Status) is valid w.r.t. the MPI standard?

Yes, it is. The 2 members that I removed were private members, anyway. I think we added them here in OMPI just as a matter of course (i.e., parity with the C struct). But this doesn't take into account the "Integers larger than ints" case.

• this PR will break the ABI, right?

Yes. ☹️

Perhaps we should do this simple solution on master only, and do something like your proposal on the v4.x branches...?

• my first intent was to preserve the current definition of type(MPI_Status) (and do not break the ABI), and update the use-mpi-f08 subroutines to something like this

subroutine MPI_Recv_f08(buf,count,datatype,source,tag,comm,status,ierror)
   use :: mpi_f08_types, only : MPI_Datatype, MPI_Comm, MPI_Status
   use :: ompi_mpifh_bindings, only : empi_recv_f
   implicit none
   OMPI_FORTRAN_IGNORE_TKR_TYPE :: buf
   INTEGER, INTENT(IN) :: count, source, tag
   TYPE(MPI_Datatype), INTENT(IN) :: datatype
   TYPE(MPI_Comm), INTENT(IN) :: comm
   TYPE(MPI_Status), INTENT(OUT) :: status
   INTEGER :: f_status(MPI_STATUS_SIZE)
   INTEGER, OPTIONAL, INTENT(OUT) :: ierror
   integer :: c_ierror

   call ompi_recv_f(buf,count,datatype%MPI_VAL,source,tag,comm%MPI_VAL,f_status,c_ierror)
   if (c_ierror.EQ.MPI_SUCCESS .AND. status.NE.MPI_STATUS_IGNORE) call PMPI_Status_ftof08(f_status, status, c_ierror)
   if (present(ierror)) ierror = c_ierror

end subroutine MPI_Recv_f08

Ugh, yes, this is probably a good solution for the release branches. This will touch a LOT of code, though (i.e., everywhere we use a status).

...alternatively, we could state that this is known not to work on the v4.x release. We could add a configure check for INTEGER=8 bytes + int=4 bytes and refuse to build (i.e., abort configure). If this has never worked in the v4.x series, then this is technically not a regression -- we would just be making the failure occur sooner (i.e., during configure).

note we could/should use macros to skip the PMPI_Status_ftof08() invocation when the Fortran and Fortran2008 statuses have the same size.

If we actually fix this in v4.x, 👍

[jsquyres]

bot:aws:retest

[ggouaillardet]

that looks fair to me

• merge this PR in master
• do not build the use mpi_f08 bindings in the release branches if the INTEGER size is larger than 32 bits.

[jsquyres]

I updated the commit message to indicate that this is an ABI break.

@ggouaillardet Can you review so that I can merge?

(we keep getting weird SSL failures in AWS CI... trying again...)

bot:aws:retest

[jsquyres]

bot:aws:retest