On 7/31/2012 5:15 AM, Jeff Squyres wrote:
On Jul 31, 2012, at 2:58 AM, Eugene Loh wrote:
The main issue is this.  If I go to ompi/mpi/fortran/mpif-h, I see six files (*recv_f and 
*probe_f) that take status arguments.  Normally, we do some conversion between Fortran 
and C status arguments.  These files test if OMPI_SIZEOF_FORTRAN_INTEGER==SIZEOF_INT, 
however, and bypass the conversion if the two types of integers are the same size.  The 
problem with this is that while the structures may be the same size, the C status has a 
size_t in it.  So, while the Fortran INTEGER array can start on any 4-byte alignment, the 
C status can end up with a 64-bit pointer on a 4-byte alignment.  That's not pleasant in 
general and can incur some serious hand-slapping on SPARC.  Specifically, SPARC/-m64 
errors out on *probe and *recv with MPI_PROC_NULL sources.  Would it be all right if I 
removed these "shorts cuts"?
Ew.  Yes.  You're right.

What specifically do you propose?  I don't remember offhand if the status 
conversion macros are the same as the regular int/INTEGER conversion macros -- 
we want to keep the no-op behavior for the regular int/INTEGER conversion 
macros and only handle the conversion of MPI_Status separately, I think.  
Specifically: for MPI_Status, we can probably still have those shortcuts for 
the int/INTEGERs, but treat the copying to the size_t separately.
I'm embarrassingly unfamiliar with the code. My impression is that internally we deal with C status structures and so our requirements for Fortran status are:
*)  enough bytes to hold whatever is in a C status
*) several words are addressable via the indices MPI_SOURCE, MPI_TAG, and MPI_ERROR So, I think what we do today is sufficient in most respects. Copying between Fortran and C integer-by-integer is fine. It might be a little nonsensical for an 8-byte size_t component to be handled as two 4-byte words, but if we do so only for copying and otherwise only use that component from the C side, things should be fine.

The only problem is if we try to use the Fortran array in-place. It's big enough, but its alignment might be wrong.

So, specifically, what I propose is getting rid of the short-cuts that try to use Fortran statuses in-place if Fortran INTEGERs are as big as C ints. I can make the changes. Sanity checks on all that are welcome.
Thanks for fixing the ibm MPROBE tests, btw.  Further proof that I must have 
been clinically insane when I added all those tests.  :-(
Insane, no, but you might copy out long-hand 100x:
for(i=0;i<N;i++) {
translates to
DO I=0,N-1
Related issue: do we need to (conditionally) add padding for the size_t in the 
Fortran array?
I guess so, but once again am unsure of myself. If I look in ompi/config/ompi_setup_mpi_fortran.m4, we compute the size of 4 C ints and a size_t in units of Fortran INTEGERs. I'm guessing that usually works for us today since any padding is at the very end and doesn't need to be copied. If someone reorganized MPI_Status, however, there could be internal padding and we would start losing parts of the status. So, it might make the code a little more robust if the padding were accounted for. I'm not keen on making such a change myself.

Meanwhile, the config code errors out if the size turns out not to be an even multiple of Fortran INTEGER size. I don't get this. I would think one could just round up to the next multiple. I'm worried my understanding of what's going on is faulty.
Here are two more smaller issues.  I'm pretty sure about them and can make the 
appropriate changes, but if someone wants to give feedback...

1)  If I look at, say, the v1.7 MPI_Mprobe man page, it says:

     A  matching  probe  with  MPI_PROC_NULL  as  source  returns
     message  =  MPI_MESSAGE_NULL...

In contrast, if I look at ibm/pt2pt/mprobe_mpifh.f90, it's checking the message to be 
MPI_MESSAGE_NO_PROC.  Further, if I look at the source code, mprobe.c seems to set the message to 
"no proc".  So, I take it the man page is wrong?  It should say "message = 
MPI_MESSAGE_NO_PROC"?
Oh, yes -- I think the man page is wrong.  The issue here is that the original 
MPI-3 proposal said to return MESSAGE_NULL, but this turns out to be ambiguous. 
 So we amended the original MPI-3 proposal with the new constant 
MPI_MESSAGE_NO_PROC.  So I think we fixed the implementation, but accidentally 
left the man page saying MESSAGE_NULL.

If you care, here's the specifics:

https://svn.mpi-forum.org/trac/mpi-forum-web/ticket/38
https://svn.mpi-forum.org/trac/mpi-forum-web/ticket/328
2)  Next, looking further at mprobe.c, it looks like this:

int MPI_Mprobe(int source, int tag, MPI_Comm comm,
               MPI_Message *message, MPI_Status *status)
{
    if (MPI_PROC_NULL == source) {
        if (MPI_STATUS_IGNORE != status) {
            *status = ompi_request_empty.req_status;
            *message =&ompi_message_no_proc.message;
        }
        return MPI_SUCCESS;
    }
    ......
}

This means that if source==MPI_PROC_NULL and status==MPI_STATUS_IGNORE, the 
message does not get set.  The assignment to *message should be moved outside 
the status check, right?
Agreed.  Good catch.

Do the IBM MPROBE tests check for this condition?
IBM?  Huh?  Okay, I know what you're talking about.  :^)

No.  The tests don't exercise MPI_STATUS_IGNORE.
If not, we should probably extend them to do so.
I suppose so. Horse having left the barn, we had better close the doors. I can add to the tests, albeit making some judgment calls about how carried away to get with this task. Clearly, that test suite is not very aggressive about exercising all code paths.

Reply via email to