ch4/ofi: clean up MPIDI_OFI_AV_ADDR macros

The MPIDI_OFI_AV_ADDR macros are difficult to read. It is only used in
MPIDI_OFI_av_to_phys and addr_exchange.

Expand the macro in both places so it can use if/else and add
proper comments.

Also replace the redundant macro MPIDI_OFI_AV_ROOT_ADDR, which is the
same as MPIDI_OFI_AV_ADDR_ROOT.

Author: hzhou

src/mpid/ch4/netmod/ofi/init_addrxchg.c

@@ -81,7 +81,7 @@ int MPIDI_OFI_addr_exchange_root_ctx(void)
 
         for (int i = 0; i < num_nodes; i++) {
             MPIR_Assert(mapped_table[i] != FI_ADDR_NOTAVAIL);
-            MPIDI_OFI_AV_ROOT_ADDR(&MPIDIU_get_av(0, node_roots[i])) = mapped_table[i];
+            MPIDI_OFI_AV_ADDR_ROOT(&MPIDIU_get_av(0, node_roots[i])) = mapped_table[i];
         }
         MPL_free(mapped_table);
         /* Then, allgather all address names using init_comm */
@@ -97,7 +97,7 @@ int MPIDI_OFI_addr_exchange_root_ctx(void)
                 char *addrname = (char *) table + recv_bc_len * rank_map[i];
                 MPIDI_OFI_CALL(fi_av_insert(MPIDI_OFI_global.ctx[0].av,
                                             addrname, 1, &addr, 0ULL, NULL), avmap);
-                MPIDI_OFI_AV_ROOT_ADDR(&MPIDIU_get_av(0, i)) = addr;
+                MPIDI_OFI_AV_ADDR_ROOT(&MPIDIU_get_av(0, i)) = addr;
             }
         }
         mpi_errno = MPIDU_bc_table_destroy();
@@ -111,7 +111,7 @@ int MPIDI_OFI_addr_exchange_root_ctx(void)
 
         for (int i = 0; i < size; i++) {
             MPIR_Assert(mapped_table[i] != FI_ADDR_NOTAVAIL);
-            MPIDI_OFI_AV_ROOT_ADDR(&MPIDIU_get_av(0, i)) = mapped_table[i];
+            MPIDI_OFI_AV_ADDR_ROOT(&MPIDIU_get_av(0, i)) = mapped_table[i];
         }
         MPL_free(mapped_table);
         mpi_errno = MPIDU_bc_table_destroy();

src/mpid/ch4/netmod/ofi/ofi_impl.h

@@ -34,28 +34,17 @@ ATTRIBUTE((unused));
 #define MPIDI_OFI_COMM_TO_INDEX(comm,rank) \
     MPIDIU_comm_rank_to_pid(comm, rank, NULL, NULL)
 
-#define MPIDI_OFI_AV_ROOT_ADDR(av)      MPIDI_OFI_AV(av).root_dest
-
-/* NOTE: these macros are a mess to read. They will be cleaned up in a few commits. */
 #ifdef MPIDI_OFI_VNI_USE_DOMAIN
 #define MPIDI_OFI_AV_ADDR_ROOT(av) \
     MPIDI_OFI_AV(av).root_dest
-#define MPIDI_OFI_AV_ADDR_OFFSET(av, vci, nic) \
-    (MPIDI_OFI_AV(av).all_dest[(vci)*MPIDI_OFI_global.num_nics+(nic)] + MPIDI_OFI_AV(av).root_offset)
-#define MPIDI_OFI_AV_ADDR_NO_OFFSET(av, vci, nic) \
+#define MPIDI_OFI_AV_ADDR_NONROOT(av, vci, nic) \
     MPIDI_OFI_AV(av).all_dest[(vci)*MPIDI_OFI_global.num_nics+(nic)]
 #else /* scalable endpoints - all vci share the same addr */
 #define MPIDI_OFI_AV_ADDR_ROOT(av) \
     MPIDI_OFI_AV(av).root_dest
-#define MPIDI_OFI_AV_ADDR_OFFSET(av, vci, nic) \
-    (MPIDI_OFI_AV(av).all_dest[nic] + MPIDI_OFI_AV(av).root_offset)
-#define MPIDI_OFI_AV_ADDR_NO_OFFSET(av, vci, nic) \
+#define MPIDI_OFI_AV_ADDR_NONROOT(av, vci, nic) \
     MPIDI_OFI_AV(av).all_dest[nic]
 #endif
-#define MPIDI_OFI_AV_ADDR(av, local_vci, local_nic, vci, nic) \
-    ((local_vci==0 && local_nic==0) ? \
-        ((vci == 0 && nic == 0) ? MPIDI_OFI_AV_ADDR_ROOT(av) : MPIDI_OFI_AV_ADDR_OFFSET(av, vci, nic)) : \
-        MPIDI_OFI_AV_ADDR_NO_OFFSET(av, vci, nic))
 
 #define MPIDI_OFI_WIN(win)     ((win)->dev.netmod.ofi)
 
@@ -465,7 +454,20 @@ MPL_STATIC_INLINE_PREFIX fi_addr_t MPIDI_OFI_av_to_phys(MPIDI_av_entry_t * av,
                                                         int local_vci, int local_nic,
                                                         int vci, int nic)
 {
-    fi_addr_t dest = MPIDI_OFI_AV_ADDR(av, local_vci, local_nic, vci, nic);
+    fi_addr_t dest;
+    if (local_vci == 0 && local_nic == 0) {
+        if (vci == 0 && nic == 0) {
+            /* root_dest */
+            dest = MPIDI_OFI_AV_ADDR_ROOT(av);
+        } else {
+            /* remote endpoints share the same address except on local root endpoint which have an offset */
+            dest = MPIDI_OFI_AV_ADDR_NONROOT(av, vci, nic) + MPIDI_OFI_AV(av).root_offset;
+        }
+    } else {
+        /* all_dest[*] */
+        dest = MPIDI_OFI_AV_ADDR_NONROOT(av, vci, nic);
+    }
+
 #ifdef MPIDI_OFI_VNI_USE_DOMAIN
     if (MPIDI_OFI_ENABLE_SCALABLE_ENDPOINTS) {
         return fi_rx_addr(dest, 0, MPIDI_OFI_MAX_ENDPOINTS_BITS);

src/mpid/ch4/netmod/ofi/ofi_init.c

@@ -1437,7 +1437,7 @@ static int try_open_shared_av(struct fid_domain *domain, struct fid_av **p_av)
         /* directly references the mapped fi_addr_t array instead               */
         fi_addr_t *mapped_table = (fi_addr_t *) av_attr.map_addr;
         for (int i = 0; i < MPIR_Process.size; i++) {
-            MPIDI_OFI_AV_ROOT_ADDR(&MPIDIU_get_av(0, i)) = mapped_table[i];
+            MPIDI_OFI_AV_ADDR_ROOT(&MPIDIU_get_av(0, i)) = mapped_table[i];
             MPL_DBG_MSG_FMT(MPIDI_CH4_DBG_MAP, VERBOSE,
                             (MPL_DBG_FDEST, " grank mapped to: rank=%d, av=%p, dest=%" PRIu64,
                              i, (void *) &MPIDIU_get_av(0, i), mapped_table[i]));

src/mpid/ch4/netmod/ofi/ofi_spawn.c

@@ -172,7 +172,7 @@ int MPIDI_OFI_upids_to_lpids(int size, int *remote_upid_size, char *remote_upids
                 sz = MPIDI_OFI_global.addrnamelen;
                 MPIDI_av_entry_t *av = &MPIDIU_get_av(k, j);
                 MPIDI_OFI_VCI_CALL(fi_av_lookup(MPIDI_OFI_global.ctx[ctx_idx].av,
-                                                MPIDI_OFI_AV_ROOT_ADDR(av), &tbladdr, &sz), 0,
+                                                MPIDI_OFI_AV_ADDR_ROOT(av), &tbladdr, &sz), 0,
                                    avlookup);
                 if (sz == addrname_len && !memcmp(tbladdr, addrname, addrname_len)) {
                     remote_lpids[i] = MPIDIU_GPID_CREATE(k, j);
@@ -207,7 +207,7 @@ int MPIDI_OFI_upids_to_lpids(int size, int *remote_upid_size, char *remote_upids
             MPIDI_OFI_VCI_CALL(fi_av_insert(MPIDI_OFI_global.ctx[ctx_idx].av, addrname,
                                             1, &addr, 0ULL, NULL), 0, avmap);
             MPIR_Assert(addr != FI_ADDR_NOTAVAIL);
-            MPIDI_OFI_AV_ROOT_ADDR(&MPIDIU_get_av(avtid, i)) = addr;
+            MPIDI_OFI_AV_ADDR_ROOT(&MPIDIU_get_av(avtid, i)) = addr;
 
             int node_id;
             mpi_errno = MPIR_nodeid_lookup(hostname, &node_id);
@@ -261,7 +261,7 @@ int MPIDI_OFI_get_local_upids(MPIR_Comm * comm, int **local_upid_size, char **lo
         size_t sz = MPIDI_OFI_global.addrnamelen;;
         MPIDI_av_entry_t *av = MPIDIU_comm_rank_to_av(comm, i);
         MPIDI_OFI_VCI_CALL(fi_av_lookup(MPIDI_OFI_global.ctx[ctx_idx].av,
-                                        MPIDI_OFI_AV_ROOT_ADDR(av),
+                                        MPIDI_OFI_AV_ADDR_ROOT(av),
                                         temp_buf + idx, &sz), 0, avlookup);
         idx += (int) sz;
 

src/mpid/ch4/netmod/ofi/ofi_vci.c

@@ -337,7 +337,8 @@ static int addr_exchange_all_ctx(MPIR_Comm * comm, int *all_num_vcis)
                     }
                 }
                 MPIR_Assert(expect_addr != FI_ADDR_NOTAVAIL);
-                MPIDI_OFI_AV_ADDR_NO_OFFSET(av, vci, nic) = expect_addr;
+                /* all_dest[*] */
+                MPIDI_OFI_AV_ADDR_NONROOT(av, vci, nic) = expect_addr;
                 /* next */
                 expect_addr++;
             }