Open MPI logo

Open MPI Development Mailing List Archives

  |   Home   |   Support   |   FAQ   |   all Development mailing list

Subject: Re: [OMPI devel] MPI_Mprobe
From: Eugene Loh (eugene.loh_at_[hidden])
Date: 2012-07-31 11:17:26


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.