Skip to content

Commit fc11c37

Browse files
ggouaillardetggouaillardet
authored
Merge pull request #3646 from ggouaillardet/spacc-fix-coverity-warnings
coll/spacc: misc fixes
2 parents 7bea824 + 44acc92 commit fc11c37

File tree

5 files changed

+65
-62
lines changed

5 files changed

+65
-62
lines changed

‎ompi/mca/coll/spacc/coll_spacc.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,9 @@
1717
BEGIN_C_DECLS
1818

1919
/* Globally exported variables */
20-
extern int ompi_coll_spacc_stream;
21-
extern int ompi_coll_spacc_priority;
20+
extern int mca_coll_spacc_stream;
21+
extern int mca_coll_spacc_priority;
22+
extern int mca_coll_spacc_verbose;
2223

2324
/* API functions */
2425

‎ompi/mca/coll/spacc/coll_spacc_allreduce.c

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -87,18 +87,19 @@ int mca_coll_spacc_allreduce_intra_redscat_allgather(
8787
int comm_size = ompi_comm_size(comm);
8888
int rank = ompi_comm_rank(comm);
8989

90-
OPAL_OUTPUT((ompi_coll_spacc_stream,
91-
"coll:spacc:allreduce_intra_redscat_allgather: rank %d/%d",
92-
rank, comm_size));
90+
opal_output_verbose(30, mca_coll_spacc_stream,
91+
"coll:spacc:allreduce_intra_redscat_allgather: rank %d/%d",
92+
rank, comm_size);
9393

9494
/* Find nearest power-of-two less than or equal to comm_size */
9595
int nsteps = opal_hibit(comm_size, comm->c_cube_dim + 1); /* ilog2(comm_size) */
96+
assert(nsteps >= 0);
9697
int nprocs_pof2 = 1 << nsteps; /* flp2(comm_size) */
9798

9899
if (count < nprocs_pof2 || !ompi_op_is_commute(op)) {
99-
OPAL_OUTPUT((ompi_coll_spacc_stream,
100-
"coll:spacc:allreduce_intra_redscat_allgather: rank %d/%d count %d switching to base allreduce",
101-
rank, comm_size, count));
100+
opal_output_verbose(20, mca_coll_spacc_stream,
101+
"coll:spacc:allreduce_intra_redscat_allgather: rank %d/%d count %d switching to base allreduce",
102+
rank, comm_size, count);
102103
return ompi_coll_base_allreduce_intra_basic_linear(sbuf, rbuf, count, dtype,
103104
op, comm, module);
104105
}
@@ -275,27 +276,27 @@ int mca_coll_spacc_allreduce_intra_redscat_allgather(
275276
rcount[step], dtype);
276277

277278
/* Move the current window to the received message */
278-
rindex[step + 1] = rindex[step];
279-
sindex[step + 1] = rindex[step];
280-
wsize = rcount[step];
281-
step++;
279+
if (step + 1 < nsteps) {
280+
rindex[step + 1] = rindex[step];
281+
sindex[step + 1] = rindex[step];
282+
wsize = rcount[step];
283+
step++;
284+
}
282285
}
283-
}
284-
/*
285-
* Assertion: each process has 1 / p' of the total reduction result:
286-
* rcount[nsteps - 1] elements in the rbuf[rindex[nsteps - 1], ...].
287-
*/
288-
289-
/*
290-
* Step 3. Allgather by the recursive doubling algorithm.
291-
* Each process has 1 / p' of the total reduction result:
292-
* rcount[nsteps - 1] elements in the rbuf[rindex[nsteps - 1], ...].
293-
* All exchanges are executed in reverse order relative
294-
* to recursive doubling (previous step).
295-
*/
296-
297-
if (vrank != -1) {
298-
step = nsteps - 1; /* step = ilog2(p') - 1 */
286+
/*
287+
* Assertion: each process has 1 / p' of the total reduction result:
288+
* rcount[nsteps - 1] elements in the rbuf[rindex[nsteps - 1], ...].
289+
*/
290+
291+
/*
292+
* Step 3. Allgather by the recursive doubling algorithm.
293+
* Each process has 1 / p' of the total reduction result:
294+
* rcount[nsteps - 1] elements in the rbuf[rindex[nsteps - 1], ...].
295+
* All exchanges are executed in reverse order relative
296+
* to recursive doubling (previous step).
297+
*/
298+
299+
step--;
299300

300301
for (int mask = nprocs_pof2 >> 1; mask > 0; mask >>= 1) {
301302
int vdest = vrank ^ mask;

‎ompi/mca/coll/spacc/coll_spacc_component.c

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@ const char *ompi_coll_spacc_component_version_string =
2121
/*
2222
* Global variable
2323
*/
24-
int ompi_coll_spacc_priority = 5;
25-
int ompi_coll_spacc_stream = -1;
24+
int mca_coll_spacc_priority = 5;
25+
int mca_coll_spacc_stream = -1;
26+
int mca_coll_spacc_verbose = 0;
2627

2728
/*
2829
* Local function
@@ -67,38 +68,33 @@ mca_coll_spacc_component_t mca_coll_spacc_component = {
6768
static int spacc_register(void)
6869
{
6970
/* Use a low priority, but allow other components to be lower */
70-
ompi_coll_spacc_priority = 5;
71+
mca_coll_spacc_priority = 5;
7172
(void)mca_base_component_var_register(&mca_coll_spacc_component.super.collm_version,
7273
"priority", "Priority of the spacc coll component",
7374
MCA_BASE_VAR_TYPE_INT, NULL, 0, 0,
74-
OPAL_INFO_LVL_6,
75+
OPAL_INFO_LVL_9,
7576
MCA_BASE_VAR_SCOPE_READONLY,
76-
&ompi_coll_spacc_priority);
77+
&mca_coll_spacc_priority);
78+
79+
(void)mca_base_component_var_register(&mca_coll_spacc_component.super.collm_version,
80+
"verbose", "Verbose level of the spacc coll component",
81+
MCA_BASE_VAR_TYPE_INT, NULL, 0, 0,
82+
OPAL_INFO_LVL_9,
83+
MCA_BASE_VAR_SCOPE_READONLY,
84+
&mca_coll_spacc_verbose);
7785
return OMPI_SUCCESS;
7886
}
7987

8088
static int spacc_open(void)
8189
{
82-
#if OPAL_ENABLE_DEBUG
83-
{
84-
int param;
85-
86-
param = mca_base_var_find("ompi", "coll", "base", "verbose");
87-
if (param >= 0) {
88-
const int *verbose = NULL;
89-
mca_base_var_get_value(param, &verbose, NULL, NULL);
90-
if (verbose && verbose[0] > 0) {
91-
ompi_coll_spacc_stream = opal_output_open(NULL);
92-
}
93-
}
94-
}
95-
#endif /* OPAL_ENABLE_DEBUG */
96-
OPAL_OUTPUT((ompi_coll_spacc_stream, "coll:spacc:component_open: done"));
90+
mca_coll_spacc_stream = opal_output_open(NULL);
91+
opal_output_set_verbosity(mca_coll_spacc_stream, mca_coll_spacc_verbose);
92+
opal_output_verbose(30, mca_coll_spacc_stream, "coll:spacc:component_open: done");
9793
return OMPI_SUCCESS;
9894
}
9995

10096
static int spacc_close(void)
10197
{
102-
OPAL_OUTPUT((ompi_coll_spacc_stream, "coll:spacc:component_close: done"));
98+
opal_output_verbose(30, mca_coll_spacc_stream, "coll:spacc:component_close: done");
10399
return OMPI_SUCCESS;
104100
}

‎ompi/mca/coll/spacc/coll_spacc_module.c

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,11 @@ mca_coll_base_module_t *ompi_coll_spacc_comm_query(
3737
{
3838
mca_coll_spacc_module_t *spacc_module;
3939

40-
OPAL_OUTPUT((ompi_coll_spacc_stream, "coll:spacc:module_spacc query called"));
40+
opal_output_verbose(30, mca_coll_spacc_stream, "coll:spacc:module_comm_query called");
4141

4242
if (OMPI_COMM_IS_INTER(comm)) {
43+
opal_output_verbose(20, mca_coll_spacc_stream,
44+
"coll:spacc:module_comm_query: spacc does not support inter-communicators");
4345
*priority = 0;
4446
return NULL;
4547
}
@@ -53,7 +55,7 @@ mca_coll_base_module_t *ompi_coll_spacc_comm_query(
5355
if (NULL == spacc_module)
5456
return NULL;
5557

56-
*priority = ompi_coll_spacc_priority;
58+
*priority = mca_coll_spacc_priority;
5759

5860
spacc_module->super.coll_module_enable = spacc_module_enable;
5961
spacc_module->super.ft_event = NULL;
@@ -84,7 +86,7 @@ mca_coll_base_module_t *ompi_coll_spacc_comm_query(
8486
static int spacc_module_enable(mca_coll_base_module_t *module,
8587
struct ompi_communicator_t *comm)
8688
{
87-
OPAL_OUTPUT((ompi_coll_spacc_stream, "coll:spacc:module_enable called."));
89+
opal_output_verbose(30, mca_coll_spacc_stream, "coll:spacc:module_enable called");
8890
return OMPI_SUCCESS;
8991
}
9092

‎ompi/mca/coll/spacc/coll_spacc_reduce.c

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -89,18 +89,19 @@ int mca_coll_spacc_reduce_intra_redscat_gather(
8989
int comm_size = ompi_comm_size(comm);
9090
int rank = ompi_comm_rank(comm);
9191

92-
OPAL_OUTPUT((ompi_coll_spacc_stream,
93-
"coll:spacc:reduce_intra_redscat_gather: rank %d/%d, root %d",
94-
rank, comm_size, root));
92+
opal_output_verbose(30, mca_coll_spacc_stream,
93+
"coll:spacc:reduce_intra_redscat_gather: rank %d/%d, root %d",
94+
rank, comm_size, root);
9595

9696
/* Find nearest power-of-two less than or equal to comm_size */
9797
int nsteps = opal_hibit(comm_size, comm->c_cube_dim + 1); /* ilog2(comm_size) */
98+
assert(nsteps >= 0);
9899
int nprocs_pof2 = 1 << nsteps; /* flp2(comm_size) */
99100

100101
if (count < nprocs_pof2 || !ompi_op_is_commute(op)) {
101-
OPAL_OUTPUT((ompi_coll_spacc_stream,
102-
"coll:spacc:reduce_intra_redscat_gather: rank %d/%d count %d switching to base reduce",
103-
rank, comm_size, count));
102+
opal_output_verbose(20, mca_coll_spacc_stream,
103+
"coll:spacc:reduce_intra_redscat_gather: rank %d/%d count %d switching to base reduce",
104+
rank, comm_size, count);
104105
return ompi_coll_base_reduce_intra_basic_linear(sbuf, rbuf, count, dtype,
105106
op, root, comm, module);
106107
}
@@ -290,10 +291,12 @@ int mca_coll_spacc_reduce_intra_redscat_gather(
290291
rcount[step], dtype);
291292

292293
/* Move the current window to the received message */
293-
rindex[step + 1] = rindex[step];
294-
sindex[step + 1] = rindex[step];
295-
wsize = rcount[step];
296-
step++;
294+
if (step + 1 < nsteps) {
295+
rindex[step + 1] = rindex[step];
296+
sindex[step + 1] = rindex[step];
297+
wsize = rcount[step];
298+
step++;
299+
}
297300
}
298301
}
299302
/*

0 commit comments

Comments
 (0)