Merge pull request #11666 from bosilca/topic/fix_reordering_in_han_gather

Fix reordering of received data in the gather

Author: bosilca

ompi/mca/coll/han/coll_han_gather.c

@@ -189,32 +189,11 @@ mca_coll_han_gather_intra(const void *sbuf, int scount,
 
     ompi_request_wait(&temp_request, MPI_STATUS_IGNORE);
 
-    /* Suppose, the expected message is 0 1 2 3 4 5 6 7 but the processes are
-     * mapped on 2 nodes, for example |0 2 4 6| |1 3 5 7|. The messages from
-     * low gather will be 0 2 4 6 and 1 3 5 7.
-     * So the upper gather result is 0 2 4 6 1 3 5 7 which must be reordered.
-     * The 3rd element (4) must be recopied at the 4th place. In general, the
-     * i-th element must be recopied at the place given by the i-th entry of the
-     * topology, which is topo[i*topolevel +1]
-     */
     /* reorder rbuf based on rank */
     if (w_rank == root && !han_module->is_mapbycore) {
-        ptrdiff_t rextent;
-        ompi_datatype_type_extent(rdtype, &rextent);
-        for (int i = 0 ; i < w_size ; i++) {
-            OPAL_OUTPUT_VERBOSE((30, mca_coll_han_component.han_output,
-                                 "[%d]: Han Gather copy from %d to %d\n",
-                                 w_rank,
-                                 i * 2 + 1,
-                                 topo[i * 2 + 1]));
-            ptrdiff_t block_size = rextent * (ptrdiff_t)rcount;
-            ptrdiff_t src_shift = block_size * i;
-            ptrdiff_t dest_shift = block_size * (ptrdiff_t)topo[i * 2 + 1];
-            ompi_datatype_copy_content_same_ddt(rdtype,
-                                                (ptrdiff_t)rcount,
-                                                reorder_rbuf + src_shift,
-                                                (char *)rbuf + dest_shift);
-        }
+        ompi_coll_han_reorder_gather(reorder_buf,
+                                     rbuf, rcount, rdtype,
+                                     comm, topo);
         free(reorder_buf);
     }
 
@@ -227,15 +206,8 @@ int mca_coll_han_gather_lg_task(void *task_args)
     mca_coll_han_gather_args_t *t = (mca_coll_han_gather_args_t *) task_args;
     OPAL_OUTPUT_VERBOSE((30, mca_coll_han_component.han_output, "[%d] Han Gather:  lg\n",
                          t->w_rank));
-    ompi_datatype_t *dtype;
-    size_t count;
-    if (t->w_rank == t->root) {
-        dtype = t->rdtype;
-        count = t->rcount;
-    } else {
-        dtype = t->sdtype;
-        count = t->scount;
-    }
+    ompi_datatype_t* dtype = (t->w_rank == t->root) ? t->rdtype : t->sdtype;
+    size_t count = (t->w_rank == t->root) ? t->rcount : t->scount;
 
     /* If the process is one of the node leader */
     char *tmp_buf = NULL;
@@ -299,15 +271,8 @@ int mca_coll_han_gather_ug_task(void *task_args)
         OPAL_OUTPUT_VERBOSE((30, mca_coll_han_component.han_output,
                              "[%d] Han Gather:  ug noop\n", t->w_rank));
     } else {
-        ompi_datatype_t *dtype;
-        size_t count;
-        if (t->w_rank == t->root) {
-            dtype = t->rdtype;
-            count = t->rcount;
-        } else {
-            dtype = t->sdtype;
-            count = t->scount;
-        }
+        ompi_datatype_t* dtype = (t->w_rank == t->root) ? t->rdtype : t->sdtype;
+        size_t count = (t->w_rank == t->root) ? t->rcount : t->scount;
 
 
         int low_size = ompi_comm_size(t->low_comm);
@@ -375,17 +340,9 @@ mca_coll_han_gather_intra_simple(const void *sbuf, int scount,
 
     ompi_communicator_t *low_comm = han_module->sub_comm[INTRA_NODE];
     ompi_communicator_t *up_comm = han_module->sub_comm[INTER_NODE];
-    ompi_datatype_t *dtype;
-    size_t count;
-
-    if (w_rank == root) {
-        dtype = rdtype;
-        count = rcount;
-    } else {
-        dtype = sdtype;
-        count = scount;
-    }
 
+    ompi_datatype_t* dtype = (w_rank == root) ? rdtype : sdtype;
+    size_t count = (w_rank == root) ? rcount : scount;
 
     /* Get the 'virtual ranks' mapping corresponding to the communicators */
     int *vranks = han_module->cached_vranks;
@@ -403,10 +360,10 @@ mca_coll_han_gather_intra_simple(const void *sbuf, int scount,
     char *reorder_buf = NULL;  // allocated memory
     char *reorder_buf_start = NULL; // start of the data
     if (w_rank == root) {
-	if (MPI_IN_PLACE == sbuf) {
+        if (MPI_IN_PLACE == sbuf) {
             ptrdiff_t rextent;
             ompi_datatype_type_extent(rdtype, &rextent);
-            sbuf = rbuf + rextent * (ptrdiff_t)rcount * w_rank;
+            sbuf = (char*)rbuf + rextent * (ptrdiff_t)rcount * w_rank;
         }
         if (han_module->is_mapbycore) {
             reorder_buf_start = (char *)rbuf;
@@ -507,13 +464,11 @@ ompi_coll_han_reorder_gather(const void *sbuf,
     int w_size = ompi_comm_size(comm);
     ptrdiff_t rextent;
     ompi_datatype_type_extent(dtype, &rextent);
+    const ptrdiff_t block_size = rextent * (ptrdiff_t)count;
     for ( i = 0; i < w_size; i++ ) {
         OPAL_OUTPUT_VERBOSE((30, mca_coll_han_component.han_output,
-                             "[%d]: Future reorder from %d to %d\n",
-                             w_rank,
-                             i * topolevel + 1,
-                             topo[i * topolevel + 1]));
-        ptrdiff_t block_size = rextent * (ptrdiff_t)count;
+                             "[%d]: HAN Gather reorder from %d to %d\n",
+                             w_rank, i, topo[i * topolevel + 1]));
         ptrdiff_t src_shift = block_size * i;
         ptrdiff_t dest_shift = block_size * (ptrdiff_t)topo[i * topolevel + 1];
         ompi_datatype_copy_content_same_ddt(dtype,