Hi Open MPI developers,

I and my coworkers checked mpi-f90-interfaces.h against MPI 2.2 standard
and found many bugs in it. Attached patches fix them for trunk.
Though some of them are trivial, others are not so trivial.
So I'll explain them below.

1. incorrect parameter types

  Two trivial parameter type mismatches.
  Fixed in my mpi-f90-interface.type-mismatch.patch.

  MPI_Cart_map periods: integer -> logical
  MPI_Reduce_scatter recvcounts: missing "dimension(*)"

2. incorrect intent against MPI 2.2 standard

  This is a somewhat complex issue.
  First, I'll cite MPI 2.2 standard below.

  2.3 in MPI 2.2 standard says:
    There is one special case - if an argument is a handle to an opaque
    object (these terms are defined in Section 2.5.1), and the object is
    updated by the procedure call, then the argument is marked INOUT or OUT.
    It is marked this way even though the handle itself is not modified -
    we use the INOUT or OUT attribute to denote that what the handle
    references is updated. Thus, in C++, IN arguments are usually either
    references or pointers to const objects.

  2.3 in MPI 2.2 standard also says:
    MPI's use of IN, OUT and INOUT is intended to indicate to the user
    how an argument is to be used, but does not provide a rigorous
    classification that can be translated directly into all language
    bindings (e.g., INTENT in Fortran 90 bindings or const in C bindings).
    For instance, the "constant" MPI_BOTTOM can usually be passed to
    OUT buffer arguments. Similarly, MPI_STATUS_IGNORE can be passed as
    the OUT status argument.

  16.2.4 in MPI 2.2 standard says:
    Advice to implementors.
    The appropriate INTENT may be different from what is given in the
    MPI generic interface. Implementations must choose INTENT so that
    the function adheres to the MPI standard.

  Hmm. intent in mpi-f90-interfaces.h does not necessarily match
  IN/OUT/INOUT in MPI 2.2, especially regarding opaque objects.
  mpi-f90-interfaces.h seems to have consideration of opaque objects
  partially, which is handled in r9922 and r23098.

  I compared MPI 2.2 ("standard") and mpi-f90-interfaces.h ("header")
  and classified problematic parameters into four types.

  Type A. Trivial error.

  These are fixed in my mpi-f90-interface.intent-error-a.patch to match
  the standard.

         subroutine            |  parameter  | standard | header
  -----------------------------+-------------+----------+--------
  MPI_Bcast                    | buffer      | inout    | in
  MPI_Pack                     | outsize     | in       | out
  MPI_Comm_test_inter          | comm        | in       | inout
                               | flag        | out      | in
  MPI_Add_error_class          | errorclass  | out      | in
  MPI_Win_create               | win         | out      | in
  MPI_Get                      | origin_addr | out      | in

  Type B. Not match but not error (opaque object).

  Though these parameters in the header don't match the standard,
  it's OK because these are opaque object and should not be INOUT
  for Fortran. Some of these are fixed in r9922 and r23098.
  These should not be changed.

         subroutine            |  parameter  | standard | header
  -----------------------------+-------------+----------+--------
  MPI_Comm_set_attr            | comm        | inout    | in
  MPI_Win_set_attr             | win         | inout    | in
  MPI_Type_set_attr            | type        | inout    | in
  MPI_Comm_set_errhandler      | comm        | inout    | in
  MPI_Win_set_errhandler       | win         | inout    | in
  MPI_File_set_errhandler      | file        | inout    | in
  MPI_File_set_view            | fh          | inout    | in
  MPI_Attr_put                 | comm        | inout    | in
  MPI_Attr_delete              | comm        | inout    | in
  MPI_Errhandler_set           | comm        | inout    | in

  Type C. Not match and yet error (opaque object).

  This parameter is similar to Type B but it should be IN, not OUT.
  This is fixed in my mpi-f90-interface.intent-error-c.patch to match
  intent that it should be.

         subroutine            |  parameter  | standard | header | should be
  -----------------------------+-------------+----------+--------+-----------
  MPI_Info_delete              | info        | inout    | out    | in

  Type D. Match but error (opaque object).

  Though these parameters in the header match the standard,
  they should be IN for Fortran because these are opaque objects and
  their handles themselves are not changed.
  These are fixed in my mpi-f90-interface.intent-error-d.patch to match
  intent that they should be for Fortran.

         subroutine            |  parameter  | standard | header | should be
  -----------------------------+-------------+----------+--------+-----------
  MPI_Comm_delete_attr         | comm        | inout    | inout  | in
  MPI_Win_delete_attr          | win         | inout    | inout  | in
  MPI_Type_delete_attr         | type        | inout    | inout  | in
  MPI_Comm_set_name            | comm        | inout    | inout  | in
  MPI_Type_set_name            | type        | inout    | inout  | in
  MPI_Win_set_name             | win         | inout    | inout  | in
  MPI_Info_set                 | info        | inout    | inout  | in
  MPI_Grequest_complete        | request     | inout    | inout  | in
  MPI_File_set_size            | fh          | inout    | inout  | in
  MPI_File_preallocate         | fh          | inout    | inout  | in
  MPI_File_set_info            | fh          | inout    | inout  | in
  MPI_File_write_at            | fh          | inout    | inout  | in
  MPI_File_write_at_all        | fh          | inout    | inout  | in
  MPI_File_iwrite_at           | fh          | inout    | inout  | in
  MPI_File_read                | fh          | inout    | inout  | in
  MPI_File_read_all            | fh          | inout    | inout  | in
  MPI_File_write               | fh          | inout    | inout  | in
  MPI_File_write_all           | fh          | inout    | inout  | in
  MPI_File_iread               | fh          | inout    | inout  | in
  MPI_File_iwrite              | fh          | inout    | inout  | in
  MPI_File_seek                | fh          | inout    | inout  | in
  MPI_File_read_shared         | fh          | inout    | inout  | in
  MPI_File_write_shared        | fh          | inout    | inout  | in
  MPI_File_iread_shared        | fh          | inout    | inout  | in
  MPI_File_iwrite_shared       | fh          | inout    | inout  | in
  MPI_File_read_ordered        | fh          | inout    | inout  | in
  MPI_File_write_ordered       | fh          | inout    | inout  | in
  MPI_File_seek_shared         | fh          | inout    | inout  | in
  MPI_File_write_at_all_begin  | fh          | inout    | inout  | in
  MPI_File_write_at_all_end    | fh          | inout    | inout  | in
  MPI_File_read_all_begin      | fh          | inout    | inout  | in
  MPI_File_read_all_end        | fh          | inout    | inout  | in
  MPI_File_write_all_begin     | fh          | inout    | inout  | in
  MPI_File_write_all_end       | fh          | inout    | inout  | in
  MPI_File_read_ordered_begin  | fh          | inout    | inout  | in
  MPI_File_read_ordered_end    | fh          | inout    | inout  | in
  MPI_File_write_ordered_begin | fh          | inout    | inout  | in
  MPI_File_write_ordered_end   | fh          | inout    | inout  | in
  MPI_File_set_atomicity       | fh          | inout    | inout  | in
  MPI_File_sync                | fh          | inout    | inout  | in

3. incorrect intent for MPI_IN_PLACE

  5.2.1 in MPI 2.2 standard says:
    Advice to users.
    By allowing the "in place" option, the receive buffer in many of the
    collective calls becomes a send-and-receive buffer. For this reason,
    a Fortran binding that includes INTENT must mark these as INOUT,
    not OUT. Note that MPI_IN_PLACE is a special kind of value; it has
    the same restrictions on its use that MPI_BOTTOM has.

  My mpi-f90-interfaces.intent-inplace.patch adapt intent parameters
  to match intent for Fortran regarding MPI_IN_PLACE.
  Note that intent of MPI_Scatter(v) should not be changed because
  MPI_IN_PLACE can be specified for recvbuf instead of sendbuf and
  no data are modified in sendbuf.

         subroutine            | parameter | standard | header | should be
  -----------------------------+-----------+----------+--------+-----------
  MPI_Gather(v)                | recvbuf   | out      | out    | inout
  MPI_Scatter(v)               | sendbuf   | in       | in     | in
  MPI_Allgather(v)             | recvbuf   | out      | out    | inout
  MPI_Alltoall(v,w)            | recvbuf   | out      | out    | inout
  MPI_Reduce                   | recvbuf   | out      | out    | inout
  MPI_Allreduce                | recvbuf   | out      | out    | inout
  MPI_Reduce_scatter           | recvbuf   | out      | out    | inout
  MPI_Scan                     | recvbuf   | out      | out    | inout
  MPI_Exscan                   | recvbuf   | out      | out    | inout

  It seems that MPI_Reduce_scatter_block, which appears in MPI 2.2, is not
  implemented in trunk yet. So my patch doesn't include modification for it.

  If you merge this patch to v1.4 branch, don't merge modification of
  MPI_Alltoall(v,w) and MPI_Exscan. MPI 2.1 doesn't allow to use MPI_IN_PLACE
  for these functions yet.

  This modification is necessary for some compilers. For example, consider
  the following code.

    double, dimension(1) :: buf
    buf(1) = value
    call mpi_allreduce(mpi_in_place, buf, 1, mpi_double, mpi_sum, comm)

  If recvbuf is declared as "intent(out)" in mpi.mod, a compiler may
  remove substitution of value for optimization because it is considered
  not to be read in mpi_allreduce subroutine.

4. mismatched interface and bridge

  I compared ompi/mpi/f90/scripts/mpi-f90-interfaces.h ("interface") and
  ompi/mpi/f90/scripts/mpi_*_f90.f90.sh ("bridge") and found some mismatches
  between them. The interfaces (except MPI_Mrecv) seems to be modifed in
  r11057 but (I imagine) the bridges are forgotten to be modified at that
  time.
  These are fixed in my mpi-f90-interfaces.mismatched-bridge.patch to match
  the interface.

         subroutine            | parameter | interface | bridge
  -----------------------------+-----------+-----------+--------
  MPI_Recv                     | status    | out       | inout
  MPI_Sendrecv                 | status    | out       | inout
  MPI_Sendrecv_replace         | status    | out       | inout
  MPI_Mrecv                    | status    | out       | inout

I also attached a patch mpi-f90-interfaces.all-in-one.patch that includes
all 6 patches described above.

Regards,

Takahiro Kawashima,
MPI development team,
Fujitsu

Attachment: mpi-f90-interfaces.type-mismatch.patch
Description: Binary data

Attachment: mpi-f90-interfaces.intent-error-a.patch
Description: Binary data

Attachment: mpi-f90-interfaces.intent-error-c.patch
Description: Binary data

Attachment: mpi-f90-interfaces.intent-error-d.patch
Description: Binary data

Attachment: mpi-f90-interfaces.intent-inplace.patch
Description: Binary data

Attachment: mpi-f90-interfaces.mismatched-bridge.patch
Description: Binary data

Attachment: mpi-f90-interfaces.all-in-one.patch
Description: Binary data

Reply via email to