On 8/8/12 11:28 PM, "Eugene Loh" <eugene....@oracle.com> wrote:
>On 8/7/2012 5:45 AM, Jeff Squyres wrote:
>> So the issue is when (for example) Fortran MPI_Recv says "hey, C ints
>>are the same as Fortran INEGERs, so I don't need a temporary MPI_Status
>>buffer; I'll just use the INTEGER array that I was given, and pass it to
>>the back-end C MPI_Recv() routine." Then C MPI_Recv() tries to write to
>>the size_t variable, and it might be poorly aligned. Kaboom.
>Right. Kind of. Read on...
>>> 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.
>> Hmm. I'm not excited about this -- the whole point is that if we don't
>>need to do an extra copy, let's not do it.
>>
>> Is there a better way to fix this?
>>
>> Off the top of my head -- for example, could we change some of those
>>compile-time checks to run-time checks, and add in an alignment check?
>>E.g.:
>>
>> ----
>> #if OMPI_SIZEOF_FORTRAN_INTEGER == SIZEOF_INT
>> c_status = (MPI_Status *) status;
>> #else
>> c_status =&c_status2;
>> #endif
>> -----
>>
>> to
>>
>> ----
>> /* The constant checks will be resolved at compile time; assume
>> alignment_is_good() is an inline macro checking for "good"
>>alignment
>> on platforms where alignment(int) != alignment(size_t) */
>> if (OMPI_SIZEOF_FORTRAN_INTEGER == SIZEOF_INT&&
>>alignment_is_good(status)) {
>> c_status = (MPI_Status *) status;
>> } else {
>> c_status =&c_status2;
>> }
>> -----
>>
>> Would something like that work?
>>
>> I'm thinking that the benefit here is that we only penalize platforms
>>(with an extra "if" statement) where alignment(int) != alignment(size_t).
>I did quite a bit more poking around. It appears this issue is already
>"well known." That is, due to this issue, we're not allowed to assume
>that a status that the user passed to us (ompi/mpi/c layer) has proper
>alignment since it might have come from Fortran. So, we should use
>OMPI_STATUS_SET and OMPI_STATUS_SET_COUNT instead of doing direct status
>assignments. Not only does ompi/request/request.h define these macros,
>but it also gives a nice description of the issue and points us to trac
>2526.
>
>It seems to me there are a number of places where we do direct status
>assignments. I found a couple of places in ompi/mca/pml/ob1/*.c and
>quite a few more in ompi/mpi/c/*.c. If I'm sufficiently inspired
>tomorrow, I might look around to see if I can identify other places to
>look. I can also confirm this leads to failures -- not only the mprobe
>stuff I reported but even vanilla MPI_Recv if you tweak the conditions
>just right. I'll try to get to this stuff tomorrow.
That's incorrect. Fortran statuses should never be passed to C
interfaces. If you look at testany_f.c, for example, a temporary status
is created and then passed to the C binding (although, in this case, it
would probably be more efficient to pass it directly to
ompi_request_testany(), but that's not important here). The part that is
important is that outside of the Fortran interfaces themselves, requests
are always C requests.
Brian
--
Brian W. Barrett
Dept. 1423: Scalable System Software
Sandia National Laboratories
