Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#269 closed bug (fixed)

use of win_ptr->fence_cnt in MPIDI_Win_fence()

Reported by: David Gingold <david.gingold@…> Owned by:
Priority: major Milestone:
Component: mpich Keywords:
Cc:

Description

MPICH2 folks --

I'm trying to make sense of code that I cribbed from the ch3
implementation, and I think this bit in ch3u_rma_sync.c is likely wrong:

     if ((win_ptr->fence_cnt == 0) && ((assert & MPI_MODE_NOSUCCEED) !
= 1))

MPI_MODE_NOSUCCEED is 16384 so assert & MPI_MODE_NOSUCCEED is never 1.

But even with that fixed to !(assert & MPI_MODE_NOSUCCEED), I'm
puzzled by the logic around this.  Asserting MPI_MODE_NOSUCCEED
should only make the code path shorter, yet it seems to have the
opposite effect here.  What am I missing?

-dg

--
David Gingold
Principal Software Engineer
SiCortex
Three Clock Tower Place, Suite 210
Maynard MA 01754
(978) 897-0214 x224



Attachments (1)

part0001.html (2.7 KB) - added by David Gingold 5 years ago.
Added by email2trac

Download all attachments as: .zip

Change History (5)

Changed 5 years ago by David Gingold

Added by email2trac

comment:1 Changed 5 years ago by David Gingold

  • id set to 269

This message has 1 attachment(s)

comment:2 Changed 5 years ago by Rajeev Thakur

David,
      You are right as usual. I have changed it to the following now.

    if (win_ptr->fence_cnt == 0)
    {
        /* win_ptr->fence_cnt == 0 means either this is the very first
           call to fence or the preceding fence had the
           MPI_MODE_NOSUCCEED assert.

           If this fence has MPI_MODE_NOSUCCEED, do nothing and return.
           Otherwise just increment the fence count and return. */

        if (!(assert & MPI_MODE_NOSUCCEED)) win_ptr->fence_cnt = 1;
    }
    else
    {
        /* This is the second or later fence. Do all the preceding RMA ops.
*/
        ...

Does this makes sense?

Rajeev



>  MPICH2 folks --
>
>  I'm trying to make sense of code that I cribbed from the ch3
>  implementation, and I think this bit in ch3u_rma_sync.c is
> likely wrong:
>
>  if ((win_ptr->fence_cnt == 0) && ((assert & MPI_MODE_NOSUCCEED) != 1))
>
>  MPI_MODE_NOSUCCEED is 16384 so assert & MPI_MODE_NOSUCCEED
> is never 1.
>
>  But even with that fixed to !(assert & MPI_MODE_NOSUCCEED), I'm
>  puzzled by the logic around this.  Asserting MPI_MODE_NOSUCCEED
>  should only make the code path shorter, yet it seems to have the
>  opposite effect here.  What am I missing?

comment:3 Changed 5 years ago by thakur

  • Resolution set to fixed
  • Status changed from new to closed

comment:4 Changed 5 years ago by David Gingold


On Nov 6, 2008, at 1:22 PM, mpich2 wrote:
>
> Comment (by Rajeev Thakur):
>
>  David,
>        You are right as usual. I have changed it to the following now.
>
>      if (win_ptr->fence_cnt == 0)
>      {
>          /* win_ptr->fence_cnt == 0 means either this is the very
> first
>             call to fence or the preceding fence had the
>             MPI_MODE_NOSUCCEED assert.
>
>             If this fence has MPI_MODE_NOSUCCEED, do nothing and
> return.
>             Otherwise just increment the fence count and return. */
>
>          if (!(assert & MPI_MODE_NOSUCCEED)) win_ptr->fence_cnt = 1;
>      }
>      else
>      {
>          /* This is the second or later fence. Do all the preceding
> RMA
>  ops.
>  */
>          ...
>
>  Does this makes sense?

Yes, I think this is better.  Thanks for the fix.

-dg

Note: See TracTickets for help on using tickets.