Merge pull request #3646 from ggouaillardet/spacc-fix-coverity-warnings

coll/spacc: misc fixes

ompi/mca/coll/spacc/coll_spacc.h

@@ -17,8 +17,9 @@
 BEGIN_C_DECLS
 
 /* Globally exported variables */
-extern int ompi_coll_spacc_stream;
+extern int mca_coll_spacc_stream;
-extern int ompi_coll_spacc_priority;
+extern int mca_coll_spacc_priority;
+extern int mca_coll_spacc_verbose;
 
 /* API functions */
 

ompi/mca/coll/spacc/coll_spacc_allreduce.c

@@ -87,18 +87,19 @@ int mca_coll_spacc_allreduce_intra_redscat_allgather(
     int comm_size = ompi_comm_size(comm);
     int rank = ompi_comm_rank(comm);
 
-    OPAL_OUTPUT((ompi_coll_spacc_stream,
+    opal_output_verbose(30, mca_coll_spacc_stream,
-                "coll:spacc:allreduce_intra_redscat_allgather: rank %d/%d",
+                        "coll:spacc:allreduce_intra_redscat_allgather: rank %d/%d",
-                rank, comm_size));
+                        rank, comm_size);
 
     /* Find nearest power-of-two less than or equal to comm_size */
     int nsteps = opal_hibit(comm_size, comm->c_cube_dim + 1);   /* ilog2(comm_size) */
+    assert(nsteps >= 0);
     int nprocs_pof2 = 1 << nsteps;                              /* flp2(comm_size) */
 
     if (count < nprocs_pof2 || !ompi_op_is_commute(op)) {
-        OPAL_OUTPUT((ompi_coll_spacc_stream,
+        opal_output_verbose(20, mca_coll_spacc_stream, 
-                    "coll:spacc:allreduce_intra_redscat_allgather: rank %d/%d count %d switching to base allreduce",
+                            "coll:spacc:allreduce_intra_redscat_allgather: rank %d/%d count %d switching to base allreduce",
-                    rank, comm_size, count));
+                            rank, comm_size, count);
         return ompi_coll_base_allreduce_intra_basic_linear(sbuf, rbuf, count, dtype,
                                                            op, comm, module);
     }
@@ -275,27 +276,27 @@ int mca_coll_spacc_allreduce_intra_redscat_allgather(
                            rcount[step], dtype);
 
             /* Move the current window to the received message */
-            rindex[step + 1] = rindex[step];
+            if (step + 1 < nsteps) {
-            sindex[step + 1] = rindex[step];
+                rindex[step + 1] = rindex[step];
-            wsize = rcount[step];
+                sindex[step + 1] = rindex[step];
-            step++;
+                wsize = rcount[step];
+                step++;
+            }
         }
-    }
+        /*
-    /*
+         * Assertion: each process has 1 / p' of the total reduction result:
-     * Assertion: each process has 1 / p' of the total reduction result:
+         * rcount[nsteps - 1] elements in the rbuf[rindex[nsteps - 1], ...].
-     * rcount[nsteps - 1] elements in the rbuf[rindex[nsteps - 1], ...].
+         */
-     */
 
-    /*
+        /*
-     * Step 3. Allgather by the recursive doubling algorithm.
+         * Step 3. Allgather by the recursive doubling algorithm.
-     * Each process has 1 / p' of the total reduction result:
+         * Each process has 1 / p' of the total reduction result:
-     * rcount[nsteps - 1] elements in the rbuf[rindex[nsteps - 1], ...].
+         * rcount[nsteps - 1] elements in the rbuf[rindex[nsteps - 1], ...].
-     * All exchanges are executed in reverse order relative
+         * All exchanges are executed in reverse order relative
-     * to recursive doubling (previous step).
+         * to recursive doubling (previous step).
-     */
+         */
 
-    if (vrank != -1) {
+        step--;
-        step = nsteps - 1; /* step = ilog2(p') - 1 */
 
         for (int mask = nprocs_pof2 >> 1; mask > 0; mask >>= 1) {
             int vdest = vrank ^ mask;

ompi/mca/coll/spacc/coll_spacc_component.c

@@ -21,8 +21,9 @@ const char *ompi_coll_spacc_component_version_string =
 /*
  * Global variable
  */
-int ompi_coll_spacc_priority = 5;
+int mca_coll_spacc_priority = 5;
-int ompi_coll_spacc_stream = -1;
+int mca_coll_spacc_stream = -1;
+int mca_coll_spacc_verbose = 0;
 
 /*
  * Local function
@@ -67,38 +68,33 @@ mca_coll_spacc_component_t mca_coll_spacc_component = {
 static int spacc_register(void)
 {
     /* Use a low priority, but allow other components to be lower */
-    ompi_coll_spacc_priority = 5;
+    mca_coll_spacc_priority = 5;
     (void)mca_base_component_var_register(&mca_coll_spacc_component.super.collm_version,
                                           "priority", "Priority of the spacc coll component",
                                           MCA_BASE_VAR_TYPE_INT, NULL, 0, 0,
-                                          OPAL_INFO_LVL_6,
+                                          OPAL_INFO_LVL_9,
                                           MCA_BASE_VAR_SCOPE_READONLY,
-                                          &ompi_coll_spacc_priority);
+                                          &mca_coll_spacc_priority);
+
+    (void)mca_base_component_var_register(&mca_coll_spacc_component.super.collm_version,
+                                          "verbose", "Verbose level of the spacc coll component",
+                                          MCA_BASE_VAR_TYPE_INT, NULL, 0, 0,
+                                          OPAL_INFO_LVL_9,
+                                          MCA_BASE_VAR_SCOPE_READONLY,
+                                          &mca_coll_spacc_verbose);
     return OMPI_SUCCESS;
 }
 
 static int spacc_open(void)
 {
-#if OPAL_ENABLE_DEBUG
+    mca_coll_spacc_stream = opal_output_open(NULL);
-    {
+    opal_output_set_verbosity(mca_coll_spacc_stream, mca_coll_spacc_verbose);
-        int param;
+    opal_output_verbose(30, mca_coll_spacc_stream, "coll:spacc:component_open: done");
-
-        param = mca_base_var_find("ompi", "coll", "base", "verbose");
-        if (param >= 0) {
-            const int *verbose = NULL;
-            mca_base_var_get_value(param, &verbose, NULL, NULL);
-            if (verbose && verbose[0] > 0) {
-                ompi_coll_spacc_stream = opal_output_open(NULL);
-            }
-        }
-    }
-#endif  /* OPAL_ENABLE_DEBUG */
-    OPAL_OUTPUT((ompi_coll_spacc_stream, "coll:spacc:component_open: done"));
     return OMPI_SUCCESS;
 }
 
 static int spacc_close(void)
 {
-    OPAL_OUTPUT((ompi_coll_spacc_stream, "coll:spacc:component_close: done"));
+    opal_output_verbose(30, mca_coll_spacc_stream, "coll:spacc:component_close: done");
     return OMPI_SUCCESS;
 }

ompi/mca/coll/spacc/coll_spacc_module.c

@@ -37,9 +37,11 @@ mca_coll_base_module_t *ompi_coll_spacc_comm_query(
 {
     mca_coll_spacc_module_t *spacc_module;
 
-    OPAL_OUTPUT((ompi_coll_spacc_stream, "coll:spacc:module_spacc query called"));
+    opal_output_verbose(30, mca_coll_spacc_stream, "coll:spacc:module_comm_query called");
 
     if (OMPI_COMM_IS_INTER(comm)) {
+        opal_output_verbose(20, mca_coll_spacc_stream,
+                            "coll:spacc:module_comm_query: spacc does not support inter-communicators");
         *priority = 0;
         return NULL;
     }
@@ -53,7 +55,7 @@ mca_coll_base_module_t *ompi_coll_spacc_comm_query(
     if (NULL == spacc_module)
         return NULL;
 
-    *priority = ompi_coll_spacc_priority;
+    *priority = mca_coll_spacc_priority;
 
     spacc_module->super.coll_module_enable = spacc_module_enable;
     spacc_module->super.ft_event = NULL;
@@ -84,7 +86,7 @@ mca_coll_base_module_t *ompi_coll_spacc_comm_query(
 static int spacc_module_enable(mca_coll_base_module_t *module,
                                struct ompi_communicator_t *comm)
 {
-    OPAL_OUTPUT((ompi_coll_spacc_stream, "coll:spacc:module_enable called."));
+    opal_output_verbose(30, mca_coll_spacc_stream, "coll:spacc:module_enable called");
     return OMPI_SUCCESS;
 }
 

ompi/mca/coll/spacc/coll_spacc_reduce.c

@@ -89,18 +89,19 @@ int mca_coll_spacc_reduce_intra_redscat_gather(
     int comm_size = ompi_comm_size(comm);
     int rank = ompi_comm_rank(comm);
 
-    OPAL_OUTPUT((ompi_coll_spacc_stream,
+    opal_output_verbose(30, mca_coll_spacc_stream,
-                "coll:spacc:reduce_intra_redscat_gather: rank %d/%d, root %d",
+                        "coll:spacc:reduce_intra_redscat_gather: rank %d/%d, root %d",
-                rank, comm_size, root));
+                        rank, comm_size, root);
 
     /* Find nearest power-of-two less than or equal to comm_size */
     int nsteps = opal_hibit(comm_size, comm->c_cube_dim + 1);   /* ilog2(comm_size) */
+    assert(nsteps >= 0);
     int nprocs_pof2 = 1 << nsteps;                              /* flp2(comm_size) */
 
     if (count < nprocs_pof2 || !ompi_op_is_commute(op)) {
-        OPAL_OUTPUT((ompi_coll_spacc_stream,
+        opal_output_verbose(20, mca_coll_spacc_stream,
-                    "coll:spacc:reduce_intra_redscat_gather: rank %d/%d count %d switching to base reduce",
+                            "coll:spacc:reduce_intra_redscat_gather: rank %d/%d count %d switching to base reduce",
-                    rank, comm_size, count));
+                            rank, comm_size, count);
         return ompi_coll_base_reduce_intra_basic_linear(sbuf, rbuf, count, dtype,
                                                         op, root, comm, module);
     }
@@ -290,10 +291,12 @@ int mca_coll_spacc_reduce_intra_redscat_gather(
                            rcount[step], dtype);
 
             /* Move the current window to the received message */
-            rindex[step + 1] = rindex[step];
+            if (step + 1 < nsteps) {
-            sindex[step + 1] = rindex[step];
+                rindex[step + 1] = rindex[step];
-            wsize = rcount[step];
+                sindex[step + 1] = rindex[step];
-            step++;
+                wsize = rcount[step];
+                step++;
+            }
         }
     }
     /*