Skip to content

Commit 939162e

Browse files
yosefeyosefe
committed
oshmem/scoll: fix shmem_collect32/64 for zero-size length
Fixes scoll_basic failures with shmem_verifier, caused by recent changes in handling of zero-size collectives. - Check for zero-size length only for fixed size collect (shmem_fcollect), but not for variable-size collect (shmem_collect) - Add 'nlong_type' parameter to internal broadcast function, to indicate whether the 'nlong' parameter is valid on non-root PEs, since it's used by shmem_collect algorithm. Before this change, some components assumed it's true (scoll_mpi) while others assumed it's false (scoll_basic). - In scoll_basic, if nlong_type==false, do not exit if nlong==0, since this parameter may not be the same on all PEs. - In scoll_mpi, fallback to scoll_basic if nlong_type==false, since MPI requires the 'count' argument of MPI_Bcast to be valid on all ranks. Signed-off-by: Yossi Itigin <[email protected]>
1 parent e54496b commit 939162e

File tree

12 files changed

+45
-22
lines changed

12 files changed

+45
-22
lines changed

‎oshmem/mca/scoll/base/scoll_base_select.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ static int scoll_null_broadcast(struct oshmem_group_t *group,
7777
const void *source,
7878
size_t nlong,
7979
long *pSync,
80+
bool nlong_type,
8081
int alg)
8182
{
8283
if (oshmem_proc_group_is_member(group)) {

‎oshmem/mca/scoll/basic/scoll_basic.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ int mca_scoll_basic_broadcast(struct oshmem_group_t *group,
6161
const void *source,
6262
size_t nlong,
6363
long *pSync,
64+
bool nlong_type,
6465
int alg);
6566
int mca_scoll_basic_collect(struct oshmem_group_t *group,
6667
void *target,

‎oshmem/mca/scoll/basic/scoll_basic_broadcast.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ int mca_scoll_basic_broadcast(struct oshmem_group_t *group,
4141
const void *source,
4242
size_t nlong,
4343
long *pSync,
44+
bool nlong_type,
4445
int alg)
4546
{
4647
int rc = OSHMEM_SUCCESS;
@@ -56,7 +57,7 @@ int mca_scoll_basic_broadcast(struct oshmem_group_t *group,
5657
int i = 0;
5758

5859
/* Do nothing on zero-length request */
59-
if (OPAL_UNLIKELY(!nlong)) {
60+
if (OPAL_UNLIKELY(nlong_type && !nlong)) {
6061
return OSHMEM_SUCCESS;
6162
}
6263

‎oshmem/mca/scoll/basic/scoll_basic_collect.c‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,13 @@ int mca_scoll_basic_collect(struct oshmem_group_t *group,
6666
if ((rc == OSHMEM_SUCCESS) && oshmem_proc_group_is_member(group)) {
6767
int i = 0;
6868

69-
/* Do nothing on zero-length request */
70-
if (OPAL_UNLIKELY(!nlong)) {
71-
return OPAL_SUCCESS;
72-
}
73-
7469
if (nlong_type) {
70+
71+
/* Do nothing on zero-length request */
72+
if (OPAL_UNLIKELY(!nlong)) {
73+
return OPAL_SUCCESS;
74+
}
75+
7576
alg = (alg == SCOLL_DEFAULT_ALG ?
7677
mca_scoll_basic_param_collect_algorithm : alg);
7778
switch (alg) {
@@ -198,6 +199,7 @@ static int _algorithm_f_central_counter(struct oshmem_group_t *group,
198199
target,
199200
group->proc_count * nlong,
200201
(pSync + 1),
202+
true,
201203
SCOLL_DEFAULT_ALG);
202204
}
203205

@@ -308,6 +310,7 @@ static int _algorithm_f_tournament(struct oshmem_group_t *group,
308310
target,
309311
group->proc_count * nlong,
310312
(pSync + 1),
313+
true,
311314
SCOLL_DEFAULT_ALG);
312315
}
313316

@@ -629,6 +632,7 @@ static int _algorithm_central_collector(struct oshmem_group_t *group,
629632
target,
630633
offset,
631634
(pSync + 1),
635+
false,
632636
SCOLL_DEFAULT_ALG);
633637
}
634638

‎oshmem/mca/scoll/basic/scoll_basic_reduce.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ static int _algorithm_central_counter(struct oshmem_group_t *group,
242242
target,
243243
nlong,
244244
(pSync + 1),
245+
true,
245246
SCOLL_DEFAULT_ALG);
246247
}
247248

@@ -360,6 +361,7 @@ static int _algorithm_tournament(struct oshmem_group_t *group,
360361
target,
361362
nlong,
362363
(pSync + 1),
364+
true,
363365
SCOLL_DEFAULT_ALG);
364366
}
365367

@@ -639,6 +641,7 @@ static int _algorithm_linear(struct oshmem_group_t *group,
639641
target,
640642
nlong,
641643
(pSync + 1),
644+
true,
642645
SCOLL_DEFAULT_ALG);
643646
}
644647

@@ -807,6 +810,7 @@ static int _algorithm_log(struct oshmem_group_t *group,
807810
target,
808811
nlong,
809812
(pSync + 1),
813+
true,
810814
SCOLL_DEFAULT_ALG);
811815
}
812816

‎oshmem/mca/scoll/fca/scoll_fca.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ int mca_scoll_fca_broadcast(struct oshmem_group_t *group,
115115
const void *source,
116116
size_t nlong,
117117
long *pSync,
118+
bool nlong_type,
118119
int algorithm_type);
119120
int mca_scoll_fca_collect(struct oshmem_group_t *group,
120121
void *target,

‎oshmem/mca/scoll/fca/scoll_fca_ops.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ int mca_scoll_fca_broadcast(struct oshmem_group_t *group,
5050
const void *source,
5151
size_t nlong,
5252
long *pSync,
53+
bool nlong_type,
5354
int alg)
5455
{
5556
mca_scoll_fca_module_t *fca_module =

‎oshmem/mca/scoll/mpi/scoll_mpi.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ int mca_scoll_mpi_broadcast(struct oshmem_group_t *group,
9090
const void *source,
9191
size_t nlong,
9292
long *pSync,
93+
bool nlong_type,
9394
int alg);
9495

9596
int mca_scoll_mpi_collect(struct oshmem_group_t *group,

‎oshmem/mca/scoll/mpi/scoll_mpi_ops.c‎

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ int mca_scoll_mpi_broadcast(struct oshmem_group_t *group,
3838
const void *source,
3939
size_t nlong,
4040
long *pSync,
41+
bool nlong_type,
4142
int alg)
4243
{
4344
mca_scoll_mpi_module_t *mpi_module;
@@ -54,34 +55,36 @@ int mca_scoll_mpi_broadcast(struct oshmem_group_t *group,
5455
}
5556
dtype = &ompi_mpi_char.dt;
5657
root = oshmem_proc_group_find_id(group, PE_root);
57-
58-
/* Do nothing on zero-length request */
59-
if (OPAL_UNLIKELY(!nlong)) {
60-
return OSHMEM_SUCCESS;
61-
}
62-
6358
/* Open SHMEM specification has the following constrains (page 85):
6459
* "If using C/C++, nelems must be of type integer. If you are using Fortran, it must be a
6560
* default integer value". And also fortran signature says "INTEGER".
6661
* Since ompi coll components doesn't support size_t at the moment,
6762
* and considering this contradiction, we cast size_t to int here
6863
* in case if the value is less than INT_MAX and fallback to previous module otherwise. */
64+
if (OPAL_UNLIKELY(!nlong_type || (INT_MAX < nlong))) {
6965
#ifdef INCOMPATIBLE_SHMEM_OMPI_COLL_APIS
70-
if (INT_MAX < nlong) {
7166
MPI_COLL_VERBOSE(20,"RUNNING FALLBACK BCAST");
7267
PREVIOUS_SCOLL_FN(mpi_module, broadcast, group,
7368
PE_root,
7469
target,
7570
source,
7671
nlong,
7772
pSync,
73+
nlong_type,
7874
SCOLL_DEFAULT_ALG);
7975
return rc;
80-
}
81-
rc = mpi_module->comm->c_coll->coll_bcast(buf, (int)nlong, dtype, root, mpi_module->comm, mpi_module->comm->c_coll->coll_bcast_module);
8276
#else
83-
rc = mpi_module->comm->c_coll->coll_bcast(buf, nlong, dtype, root, mpi_module->comm, mpi_module->comm->c_coll->coll_bcast_module);
77+
MPI_COLL_ERROR(20, "variable broadcast length, or exceeds INT_MAX: %zu", nlong);
78+
return OSHMEM_ERR_NOT_SUPPORTED;
8479
#endif
80+
}
81+
82+
/* Do nothing on zero-length request */
83+
if (OPAL_UNLIKELY(!nlong)) {
84+
return OSHMEM_SUCCESS;
85+
}
86+
87+
rc = mpi_module->comm->c_coll->coll_bcast(buf, nlong, dtype, root, mpi_module->comm, mpi_module->comm->c_coll->coll_bcast_module);
8588
if (OMPI_SUCCESS != rc){
8689
MPI_COLL_VERBOSE(20,"RUNNING FALLBACK BCAST");
8790
PREVIOUS_SCOLL_FN(mpi_module, broadcast, group,
@@ -90,6 +93,7 @@ int mca_scoll_mpi_broadcast(struct oshmem_group_t *group,
9093
source,
9194
nlong,
9295
pSync,
96+
nlong_type,
9397
SCOLL_DEFAULT_ALG);
9498
}
9599
return rc;
@@ -111,12 +115,13 @@ int mca_scoll_mpi_collect(struct oshmem_group_t *group,
111115
MPI_COLL_VERBOSE(20,"RUNNING MPI ALLGATHER");
112116
mpi_module = (mca_scoll_mpi_module_t *) group->g_scoll.scoll_collect_module;
113117

114-
/* Do nothing on zero-length request */
115-
if (OPAL_UNLIKELY(!nlong)) {
116-
return OSHMEM_SUCCESS;
117-
}
118-
119118
if (nlong_type == true) {
119+
120+
/* Do nothing on zero-length request */
121+
if (OPAL_UNLIKELY(!nlong)) {
122+
return OSHMEM_SUCCESS;
123+
}
124+
120125
sbuf = (void *) source;
121126
rbuf = target;
122127
stype = &ompi_mpi_char.dt;

‎oshmem/mca/scoll/scoll.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,7 @@ typedef int (*mca_scoll_base_module_broadcast_fn_t)(struct oshmem_group_t *group
122122
const void *source,
123123
size_t nlong,
124124
long *pSync,
125+
bool nlong_type,
125126
int alg);
126127
typedef int (*mca_scoll_base_module_collect_fn_t)(struct oshmem_group_t *group,
127128
void *target,

0 commit comments

Comments
 (0)