ADI/comm: exchange context_id in MPID_Intercomm_exchange

hzhou · hzhou · commit 4b4239b3e5df · 2025-03-31T10:49:47.000-05:00
We can easily exchange the context_id along with the rest of the remote
info rather than do it in a separate step.

We can determine is_low_group by comparing world namespace and
world_rank entirely in the MPIR layer, thus no longer need it in
MPID_Intercomm_exchange.

Rename MPID_Intercomm_exchange_map to MPID_Intercomm_exchange to better
reflect that it is not just exchanging maps.
diff --git a/src/mpi/comm/comm_impl.c b/src/mpi/comm/comm_impl.c
@@ -952,88 +952,84 @@ int MPIR_Comm_set_info_impl(MPIR_Comm * comm_ptr, MPIR_Info * info_ptr)
     goto fn_exit;
 }
 
+/* arbitrarily determine which group is the low_group by comparing
+ * world namespaces and world ranks */
+static int determine_low_group(MPIR_Lpid remote_lpid, bool * is_low_group_out)
+{
+    int mpi_errno = MPI_SUCCESS;
+
+    int my_world_idx = 0;
+    int my_world_rank = MPIR_Process.rank;
+    int remote_world_idx = MPIR_LPID_WORLD_INDEX(remote_lpid);
+    int remote_world_rank = MPIR_LPID_WORLD_RANK(remote_lpid);
+
+    if (my_world_idx == remote_world_idx) {
+        /* same world, just compare world ranks */
+        MPIR_Assert(my_world_idx == 0);
+        *is_low_group_out = (my_world_rank < remote_world_rank);
+    } else {
+        /* different world, compare namespace */
+        int cmp_result = strncmp(MPIR_Worlds[my_world_idx].namespace,
+                                 MPIR_Worlds[remote_world_idx].namespace,
+                                 MPIR_NAMESPACE_MAX);
+        MPIR_Assert(cmp_result != 0);
+        if (cmp_result < 0)
+            *is_low_group_out = false;
+        else
+            *is_low_group_out = true;
+    }
+
+    return mpi_errno;
+}
+
 int MPIR_Intercomm_create_impl(MPIR_Comm * local_comm_ptr, int local_leader,
                                MPIR_Comm * peer_comm_ptr, int remote_leader, int tag,
                                MPIR_Comm ** new_intercomm_ptr)
 {
     int mpi_errno = MPI_SUCCESS;
-    int final_context_id, recvcontext_id;
     int remote_size = 0;
     MPIR_Lpid *remote_lpids = NULL;
-    int comm_info[3];
-    int is_low_group = 0;
     MPIR_Session *session_ptr = local_comm_ptr->session_ptr;
 
     MPIR_FUNC_ENTER;
 
-    /* Shift tag into the tagged coll space */
-    tag |= MPIR_TAG_COLL_BIT;
-
-    mpi_errno = MPID_Intercomm_exchange_map(local_comm_ptr, local_leader,
-                                            peer_comm_ptr, remote_leader,
-                                            &remote_size, &remote_lpids, &is_low_group);
-    MPIR_ERR_CHECK(mpi_errno);
-
     /*
      * Create the contexts.  Each group will have a context for sending
      * to the other group. All processes must be involved.  Because
      * we know that the local and remote groups are disjoint, this
      * step will complete
      */
-    MPL_DBG_MSG_FMT(MPIR_DBG_COMM, VERBOSE,
-                    (MPL_DBG_FDEST, "About to get contextid (local_size=%d) on rank %d",
-                     local_comm_ptr->local_size, local_comm_ptr->rank));
     /* In the multi-threaded case, MPIR_Get_contextid_sparse assumes that the
      * calling routine already holds the single critical section */
     /* TODO: Make sure this is tag-safe */
+    int recvcontext_id;
     mpi_errno = MPIR_Get_contextid_sparse(local_comm_ptr, &recvcontext_id, FALSE);
     MPIR_ERR_CHECK(mpi_errno);
     MPIR_Assert(recvcontext_id != 0);
-    MPL_DBG_MSG_FMT(MPIR_DBG_COMM, VERBOSE, (MPL_DBG_FDEST, "Got contextid=%d", recvcontext_id));
-
-    /* Leaders can now swap context ids and then broadcast the value
-     * to the local group of processes */
-    if (local_comm_ptr->rank == local_leader) {
-        int remote_context_id;
 
-        mpi_errno =
-            MPIC_Sendrecv(&recvcontext_id, 1, MPIR_CONTEXT_ID_T_DATATYPE, remote_leader, tag,
-                          &remote_context_id, 1, MPIR_CONTEXT_ID_T_DATATYPE, remote_leader, tag,
-                          peer_comm_ptr, MPI_STATUS_IGNORE, MPIR_ERR_NONE);
-        MPIR_ERR_CHECK(mpi_errno);
-
-        final_context_id = remote_context_id;
+    /* Shift tag into the tagged coll space */
+    tag |= MPIR_TAG_COLL_BIT;
 
-        /* Now, send all of our local processes the remote_lpids,
-         * along with the final context id */
-        comm_info[0] = final_context_id;
-        MPL_DBG_MSG(MPIR_DBG_COMM, VERBOSE, "About to bcast on local_comm");
-        mpi_errno = MPIR_Bcast(comm_info, 1, MPIR_INT_INTERNAL, local_leader,
-                               local_comm_ptr, MPIR_ERR_NONE);
-        MPIR_ERR_CHECK(mpi_errno);
-        MPL_DBG_MSG_D(MPIR_DBG_COMM, VERBOSE, "end of bcast on local_comm of size %d",
-                      local_comm_ptr->local_size);
-    } else {
-        /* we're the other processes */
-        MPL_DBG_MSG(MPIR_DBG_COMM, VERBOSE, "About to receive bcast on local_comm");
-        mpi_errno = MPIR_Bcast(comm_info, 1, MPIR_INT_INTERNAL, local_leader,
-                               local_comm_ptr, MPIR_ERR_NONE);
-        MPIR_ERR_CHECK(mpi_errno);
+    int remote_context_id;
+    mpi_errno = MPID_Intercomm_exchange(local_comm_ptr, local_leader,
+                                        peer_comm_ptr, remote_leader, tag,
+                                        recvcontext_id, &remote_context_id,
+                                        &remote_size, &remote_lpids);
+    MPIR_ERR_CHECK(mpi_errno);
 
-        /* Extract the context and group sign information */
-        final_context_id = comm_info[0];
-    }
+    bool is_low_group;
+    mpi_errno = determine_low_group(remote_lpids[0], &is_low_group);
+    MPIR_ERR_CHECK(mpi_errno);
 
     /* At last, we now have the information that we need to build the
      * intercommunicator */
 
     /* All processes in the local_comm now build the communicator */
 
     mpi_errno = MPIR_Comm_create(new_intercomm_ptr);
-    if (mpi_errno)
-        goto fn_fail;
+    MPIR_ERR_CHECK(mpi_errno);
 
-    (*new_intercomm_ptr)->context_id = final_context_id;
+    (*new_intercomm_ptr)->context_id = remote_context_id;
     (*new_intercomm_ptr)->recvcontext_id = recvcontext_id;
     (*new_intercomm_ptr)->remote_size = remote_size;
     (*new_intercomm_ptr)->local_size = local_comm_ptr->local_size;
@@ -1055,6 +1051,7 @@ int MPIR_Intercomm_create_impl(MPIR_Comm * local_comm_ptr, int local_leader,
     /* construct remote_group */
     mpi_errno = MPIR_Group_create_map(remote_size, MPI_UNDEFINED, session_ptr, remote_lpids,
                                       &(*new_intercomm_ptr)->remote_group);
+    MPIR_ERR_CHECK(mpi_errno);
 
     MPIR_Comm_set_session_ptr(*new_intercomm_ptr, session_ptr);
 
diff --git a/src/mpid/ch3/include/mpidpost.h b/src/mpid/ch3/include/mpidpost.h
@@ -186,10 +186,10 @@ int MPIDI_GPID_ToLpidArray( int size, MPIDI_Gpid gpid[], MPIR_Lpid lpid[] );
 int MPIDI_PG_ForwardPGInfo( MPIR_Comm *peer_ptr, MPIR_Comm *comm_ptr,
                             int nPGids, const MPIDI_Gpid gpids[],
                             int root );
-int MPID_Intercomm_exchange_map( MPIR_Comm *local_comm_ptr, int local_leader,
-                                 MPIR_Comm *peer_comm_ptr, int remote_leader,
-                                 int *remote_size, MPIR_Lpid **remote_lpids,
-                                 int *is_low_group);
+int MPID_Intercomm_exchange(MPIR_Comm *local_comm_ptr, int local_leader,
+                            MPIR_Comm *peer_comm_ptr, int remote_leader,
+                            int tag, int context_id, int *remote_context_id,
+                            int *remote_size, MPIR_Lpid **remote_lpids);
 int MPID_Create_intercomm_from_lpids( MPIR_Comm *newcomm_ptr,
                                       int size, const MPIR_Lpid lpids[] );
 int MPID_Comm_get_lpid(MPIR_Comm *comm_ptr, int idx, MPIR_Lpid *lpid_ptr, bool is_remote);
diff --git a/src/mpid/ch3/src/mpid_vc.c b/src/mpid/ch3/src/mpid_vc.c
@@ -440,24 +440,20 @@ static int check_disjoint_lpids(MPIR_Lpid lpids1[], int n1, MPIR_Lpid lpids2[],
 #endif /* HAVE_ERROR_CHECKING */
 
 /*@
-  MPID_Intercomm_exchange_map - Exchange address mapping for intercomm creation.
+  MPID_Intercomm_exchange - Exchange remote info for intercomm creation.
  @*/
-int MPID_Intercomm_exchange_map(MPIR_Comm *local_comm_ptr, int local_leader,
-                                MPIR_Comm *peer_comm_ptr, int remote_leader,
-                                int *remote_size, MPIR_Lpid **remote_lpids,
-                                int *is_low_group)
+int MPID_Intercomm_exchange(MPIR_Comm *local_comm_ptr, int local_leader,
+                            MPIR_Comm *peer_comm_ptr, int remote_leader, int tag,
+                            int context_id, int *remote_context_id,
+                            int *remote_size, MPIR_Lpid **remote_lpids)
 {
     int mpi_errno = MPI_SUCCESS;
     int singlePG;
     int local_size;
     MPIR_Lpid *local_lpids=0;
     MPIDI_Gpid *local_gpids=NULL, *remote_gpids=NULL;
-    int comm_info[2];
-    int cts_tag;
     MPIR_CHKLMEM_DECL();
 
-    cts_tag = 0 | MPIR_TAG_COLL_BIT;
-
     if (local_comm_ptr->rank == local_leader) {
 
         /* First, exchange the group information.  If we were certain
@@ -471,13 +467,17 @@ int MPID_Intercomm_exchange_map(MPIR_Comm *local_comm_ptr, int local_leader,
         /* printf( "About to sendrecv in intercomm_create\n" );fflush(stdout);*/
         MPL_DBG_MSG_FMT(MPIDI_CH3_DBG_OTHER,VERBOSE,(MPL_DBG_FDEST,"rank %d sendrecv to rank %d", peer_comm_ptr->rank,
                                        remote_leader));
-        mpi_errno = MPIC_Sendrecv( &local_size,  1, MPIR_INT_INTERNAL,
-                                      remote_leader, cts_tag,
-                                      remote_size, 1, MPIR_INT_INTERNAL,
-                                      remote_leader, cts_tag,
-                                      peer_comm_ptr, MPI_STATUS_IGNORE, MPIR_ERR_NONE );
+        int local_ints[2] = {local_size, context_id};
+        int remote_ints[2];
+        mpi_errno = MPIC_Sendrecv(local_ints, 2, MPIR_INT_INTERNAL,
+                                  remote_leader, tag,
+                                  remote_ints, 2, MPIR_INT_INTERNAL,
+                                  remote_leader, tag,
+                                  peer_comm_ptr, MPI_STATUS_IGNORE, MPIR_ERR_NONE );
         MPIR_ERR_CHECK(mpi_errno);
 
+        *remote_size = remote_ints[0];
+        *remote_context_id = remote_ints[1];
         MPL_DBG_MSG_FMT(MPIDI_CH3_DBG_OTHER,VERBOSE,(MPL_DBG_FDEST, "local size = %d, remote size = %d", local_size,
                                        *remote_size ));
         /* With this information, we can now send and receive the
@@ -492,9 +492,9 @@ int MPID_Intercomm_exchange_map(MPIR_Comm *local_comm_ptr, int local_leader,
 
         /* Exchange the lpid arrays */
         mpi_errno = MPIC_Sendrecv( local_gpids, local_size*sizeof(MPIDI_Gpid), MPIR_BYTE_INTERNAL,
-                                      remote_leader, cts_tag,
+                                      remote_leader, tag,
                                       remote_gpids, (*remote_size)*sizeof(MPIDI_Gpid), MPIR_BYTE_INTERNAL,
-                                      remote_leader, cts_tag, peer_comm_ptr,
+                                      remote_leader, tag, peer_comm_ptr,
                                       MPI_STATUS_IGNORE, MPIR_ERR_NONE );
         MPIR_ERR_CHECK(mpi_errno);
 
@@ -520,22 +520,18 @@ int MPID_Intercomm_exchange_map(MPIR_Comm *local_comm_ptr, int local_leader,
         }
 #       endif /* HAVE_ERROR_CHECKING */
 
-        /* Make an arbitrary decision about which group of process is
-           the low group.  The LEADERS do this by comparing the
-           local process ids of the 0th member of the two groups */
-        (*is_low_group) = local_lpids[0] < (*remote_lpids)[0];
-
         /* At this point, we're done with the local lpids; they'll
            be freed with the other local memory on exit */
 
     } /* End of the first phase of the leader communication */
     /* Leaders can now swap context ids and then broadcast the value
        to the local group of processes */
+    int comm_info[3];
     if (local_comm_ptr->rank == local_leader) {
         /* Now, send all of our local processes the remote_lpids,
            along with the final context id */
         comm_info[0] = *remote_size;
-        comm_info[1] = *is_low_group;
+        comm_info[1] = *remote_context_id;
         MPL_DBG_MSG(MPIDI_CH3_DBG_OTHER,VERBOSE,"About to bcast on local_comm");
         mpi_errno = MPIR_Bcast( comm_info, 2, MPIR_INT_INTERNAL, local_leader, local_comm_ptr, MPIR_ERR_NONE );
         MPIR_ERR_CHECK(mpi_errno);
@@ -559,7 +555,7 @@ int MPID_Intercomm_exchange_map(MPIR_Comm *local_comm_ptr, int local_leader,
         MPIR_ERR_CHECK(mpi_errno);
 
         /* Extract the context and group sign information */
-        *is_low_group     = comm_info[1];
+        *remote_context_id = comm_info[1];
     }
 
     /* Finish up by giving the device the opportunity to update
diff --git a/src/mpid/ch4/include/mpidch4.h b/src/mpid/ch4/include/mpidch4.h
@@ -167,7 +167,10 @@ int MPID_Type_commit_hook(MPIR_Datatype *);
 int MPID_Type_free_hook(MPIR_Datatype *);
 int MPID_Op_commit_hook(MPIR_Op *);
 int MPID_Op_free_hook(MPIR_Op *);
-int MPID_Intercomm_exchange_map(MPIR_Comm *, int, MPIR_Comm *, int, int *, MPIR_Lpid **, int *);
+int MPID_Intercomm_exchange(MPIR_Comm * local_comm, int local_leader,
+                            MPIR_Comm * peer_comm, int remote_leader, int tag,
+                            int context_id, int *remote_context_id_out,
+                            int *remote_size_out, MPIR_Lpid ** remote_lpids_out);
 int MPID_Create_intercomm_from_lpids(MPIR_Comm *, int, const MPIR_Lpid[]);
 int MPID_Comm_commit_pre_hook(MPIR_Comm *);
 int MPID_Comm_free_hook(MPIR_Comm *);
diff --git a/src/mpid/ch4/src/ch4_comm.c b/src/mpid/ch4/src/ch4_comm.c
diff --git a/src/mpid/ch4/src/ch4_impl.h b/src/mpid/ch4/src/ch4_impl.h
