Skip to content

Fix shadowing in ompi #8593

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 23, 2021
Merged

Fix shadowing in ompi #8593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6d366ba
Add -Wshadow to --picky-compiler.
awlauria Mar 11, 2021
0058f19
Fix shadowing warnings in ompi.
awlauria Mar 12, 2021
7d32da4
Fix some misc gcc warnings.
awlauria Mar 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/opal_setup_cc.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,7 @@ AC_DEFUN([OPAL_SETUP_CC],[
_OPAL_CHECK_SPECIFIC_CFLAGS(-Wmissing-prototypes, Wmissing_prototypes)
_OPAL_CHECK_SPECIFIC_CFLAGS(-Wstrict-prototypes, Wstrict_prototypes)
_OPAL_CHECK_SPECIFIC_CFLAGS(-Wcomment, Wcomment)
_OPAL_CHECK_SPECIFIC_CFLAGS(-Wshadow, Wshadow)
_OPAL_CHECK_SPECIFIC_CFLAGS(-Werror-implicit-function-declaration, Werror_implicit_function_declaration)
_OPAL_CHECK_SPECIFIC_CFLAGS(-Wno-long-double, Wno_long_double, int main() { long double x; })
_OPAL_CHECK_SPECIFIC_CFLAGS(-fno-strict-aliasing, fno_strict_aliasing, int main() { long double x; })
Expand Down
5 changes: 2 additions & 3 deletions ompi/communicator/ft/comm_ft_detector.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,8 +418,7 @@ static int fd_heartbeat_request_cb(ompi_communicator_t* comm, ompi_comm_heartbea
detector->hb_sstamp = 0.;

if( comm_detector_use_rdma_hb ) {
ompi_communicator_t* comm = detector->comm;
ompi_proc_t* proc = ompi_comm_peer_lookup(comm, msg->from);
ompi_proc_t* proc = ompi_comm_peer_lookup(detector->comm, msg->from);
assert( NULL != proc );
mca_bml_base_endpoint_t* endpoint = mca_bml_base_get_endpoint(proc);
assert( NULL != endpoint );
Expand Down Expand Up @@ -460,7 +459,7 @@ static int fd_heartbeat_request_cb(ompi_communicator_t* comm, ompi_comm_heartbea

static void fd_event_cb(int fd, short flags, void* pdetector)
{
comm_detector_t* detector = pdetector;;
comm_detector_t* detector = pdetector;
double stamp = PMPI_Wtime();
double lastpeek = detector->hb_lastpeek;
detector->hb_lastpeek = stamp;
Expand Down
76 changes: 33 additions & 43 deletions ompi/datatype/ompi_datatype_args.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,57 +71,45 @@ typedef struct __dt_args {
#define OMPI_DATATYPE_ALIGN_PTR(PTR, TYPE)
#endif /* OPAL_ALIGN_WORD_SIZE_INTEGERS */

/**
* Some architectures require 64 bits pointers (to pointers) to
* be 64 bits aligned. As in the ompi_datatype_args_t structure we have
* 2 such array of pointers and one to an array of ints, if we start by
* setting the 64 bits aligned one we will not have any trouble. Problem
* originally reported on SPARC 64.
*/
#define ALLOC_ARGS(PDATA, IC, AC, DC) \
do { \
int length = sizeof(ompi_datatype_args_t) + (IC) * sizeof(int) + \
(AC) * sizeof(ptrdiff_t) + (DC) * sizeof(MPI_Datatype); \
char* buf = (char*)malloc( length ); \
ompi_datatype_args_t* pArgs = (ompi_datatype_args_t*)buf; \
pArgs->ci = (IC); \
pArgs->ca = (AC); \
pArgs->cd = (DC); \
buf += sizeof(ompi_datatype_args_t); \
if( pArgs->ca == 0 ) pArgs->a = NULL; \
else { \
pArgs->a = (ptrdiff_t*)buf; \
buf += pArgs->ca * sizeof(ptrdiff_t); \
} \
if( pArgs->cd == 0 ) pArgs->d = NULL; \
else { \
pArgs->d = (ompi_datatype_t**)buf; \
buf += pArgs->cd * sizeof(MPI_Datatype); \
} \
if( pArgs->ci == 0 ) pArgs->i = NULL; \
else pArgs->i = (int*)buf; \
pArgs->ref_count = 1; \
pArgs->total_pack_size = (4 + (IC) + (DC)) * sizeof(int) + \
(AC) * sizeof(ptrdiff_t); \
(PDATA)->args = (void*)pArgs; \
(PDATA)->packed_description = 0; \
} while(0)


int32_t ompi_datatype_set_args( ompi_datatype_t* pData,
int32_t ci, const int32_t** i,
int32_t ca, const ptrdiff_t* a,
int32_t cd, ompi_datatype_t* const * d, int32_t type)
{
int pos;
ompi_datatype_args_t* pArgs;

assert( NULL == pData->args );
ALLOC_ARGS( pData, ci, ca, cd );

pArgs = (ompi_datatype_args_t*)pData->args;
int length = sizeof(ompi_datatype_args_t) + ci * sizeof(int) +
ca * sizeof(ptrdiff_t) + cd * sizeof(MPI_Datatype);
char* buf = (char*)malloc( length );
ompi_datatype_args_t* pArgs = (ompi_datatype_args_t*)buf;
pArgs->ci = ci; pArgs->i = NULL;
pArgs->ca = ca; pArgs->a = NULL;
pArgs->cd = cd; pArgs->d = NULL;
pArgs->create_type = type;

/**
* Some architectures require 64 bits pointers (to pointers) to
* be 64 bits aligned. As in the ompi_datatype_args_t structure we have
* 2 such array of pointers and one to an array of ints, if we start by
* setting the 64 bits aligned one we will not have any trouble. Problem
* originally reported on SPARC 64.
*/
buf += sizeof(ompi_datatype_args_t);
if( 0 != pArgs->ca ) {
pArgs->a = (ptrdiff_t*)buf;
buf += pArgs->ca * sizeof(ptrdiff_t);
}
if( 0 != pArgs->cd ) {
pArgs->d = (ompi_datatype_t**)buf;
buf += pArgs->cd * sizeof(MPI_Datatype);
}
if( 0 != pArgs->ci ) pArgs->i = (int*)buf;

pArgs->ref_count = 1;
pArgs->total_pack_size = (4 + ci) * sizeof(int) +
cd * sizeof(MPI_Datatype) +
ca * sizeof(ptrdiff_t);
switch(type) {

case MPI_COMBINER_DUP:
Expand Down Expand Up @@ -229,7 +217,7 @@ int32_t ompi_datatype_set_args( ompi_datatype_t* pData,
pArgs->d[pos] = d[pos];
if( !(ompi_datatype_is_predefined(d[pos])) ) {
/* We handle a user defined datatype. We should make sure that the
* user will not have the oportunity to destroy it before all derived
* user will not have the opportunity to destroy it before all derived
* datatypes are destroyed. As we keep pointers to every datatype
* (for MPI_Type_get_content and MPI_Type_get_envelope) we have to make
* sure that those datatype will be available if the user ask for them.
Expand All @@ -243,10 +231,12 @@ int32_t ompi_datatype_set_args( ompi_datatype_t* pData,
pArgs->total_pack_size += sizeof(int); /* each data has an ID */
}

pData->args = (void*)pArgs;
pData->packed_description = 0;

return OMPI_SUCCESS;
}


int32_t ompi_datatype_print_args( const ompi_datatype_t* pData )
{
int32_t i;
Expand Down
3 changes: 3 additions & 0 deletions ompi/datatype/ompi_datatype_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,9 @@ int32_t ompi_datatype_init( void )
#define MOOG(name, index) \
do { \
int rc; \
/* Silence 'unused' compiler warning in optimized builds, \
where assert() is removed. */ \
(void) rc; \
ompi_mpi_##name.dt.d_f_to_c_index = index; \
rc = opal_pointer_array_set_item(&ompi_datatype_f_to_c_table, \
index, &ompi_mpi_##name); \
Expand Down
4 changes: 2 additions & 2 deletions ompi/mca/coll/base/coll_base_alltoallv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ mca_coll_base_alltoallv_intra_basic_inplace(const void *rbuf, const int *rcounts
if (i == rank) {
continue;
}
size_t size = opal_datatype_span(&rdtype->super, rcounts[i], &gap);
max_size = size > max_size ? size : max_size;
size_t cur_size = opal_datatype_span(&rdtype->super, rcounts[i], &gap);
max_size = cur_size > max_size ? cur_size : max_size;
}
/* The gap will always be the same as we are working on the same datatype */

Expand Down
8 changes: 4 additions & 4 deletions ompi/mca/coll/libnbc/nbc_internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -549,13 +549,13 @@ static inline int NBC_Unpack(void *src, int srccount, MPI_Datatype srctype, void
return OMPI_SUCCESS;
}

/* deletes elements from dict until low watermark is reached */
static inline void NBC_SchedCache_dictwipe(hb_tree *dict, int *size) {
/* deletes elements from dict_in until low watermark is reached */
static inline void NBC_SchedCache_dictwipe(hb_tree *dict_in, int *size) {
hb_itor *itor;

itor = hb_itor_new(dict);
itor = hb_itor_new(dict_in);
for (; hb_itor_valid(itor) && (*size>NBC_SCHED_DICT_LOWER); hb_itor_next(itor)) {
hb_tree_remove(dict, hb_itor_key(itor), 0);
hb_tree_remove(dict_in, hb_itor_key(itor), 0);
*size = *size-1;
}
hb_itor_destroy(itor);
Expand Down
4 changes: 2 additions & 2 deletions ompi/mca/coll/sm/coll_sm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ BEGIN_C_DECLS
great while. Use a "goto" label for expdiency to exit loops. */
#define SPIN_CONDITION_MAX 100000
#define SPIN_CONDITION(cond, exit_label) \
do { int i; \
do { \
if (cond) goto exit_label; \
for (i = 0; i < SPIN_CONDITION_MAX; ++i) { \
for (int spin_cond_i = 0; spin_cond_i < SPIN_CONDITION_MAX; ++spin_cond_i) { \
if (cond) { goto exit_label; } \
} \
opal_progress(); \
Expand Down
3 changes: 1 addition & 2 deletions ompi/mca/coll/sm/coll_sm_module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,6 @@ int ompi_coll_sm_lazy_enable(mca_coll_base_module_t *module,
static int bootstrap_comm(ompi_communicator_t *comm,
mca_coll_sm_module_t *module)
{
int i;
char *shortpath, *fullpath;
mca_coll_sm_component_t *c = &mca_coll_sm_component;
mca_coll_sm_comm_t *data = module->sm_comm_data;
Expand All @@ -511,7 +510,7 @@ static int bootstrap_comm(ompi_communicator_t *comm,
with the lowest ORTE name to form a unique filename. */
proc = ompi_group_peer_lookup(comm->c_local_group, 0);
lowest_name = OMPI_CAST_RTE_NAME(&proc->super.proc_name);
for (i = 1; i < comm_size; ++i) {
for (int i = 1; i < comm_size; ++i) {
proc = ompi_group_peer_lookup(comm->c_local_group, i);
if (ompi_rte_compare_name_fields(OMPI_RTE_CMP_ALL,
OMPI_CAST_RTE_NAME(&proc->super.proc_name),
Expand Down
1 change: 0 additions & 1 deletion ompi/mca/osc/rdma/osc_rdma_component.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,6 @@ static int ompi_osc_rdma_query_alternate_btls (ompi_communicator_t *comm, ompi_o
mca_btl_base_selected_module_t *item;
char **btls_to_use = opal_argv_split (ompi_osc_rdma_btl_alternate_names, ',');
int btls_found = 0;
void *tmp;

btls_to_use = opal_argv_split (ompi_osc_rdma_btl_alternate_names, ',');
if (NULL == btls_to_use) {
Expand Down
6 changes: 0 additions & 6 deletions ompi/mca/osc/rdma/osc_rdma_peer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ int ompi_osc_rdma_new_peer (struct ompi_osc_rdma_module_t *module, int peer_id,
struct mca_btl_base_endpoint_t *endpoint;
ompi_osc_rdma_peer_t *peer;
uint8_t module_btl_index = UINT8_MAX;
mca_btl_base_module_t *btl = NULL;

*peer_out = NULL;

Expand All @@ -90,10 +89,6 @@ int ompi_osc_rdma_new_peer (struct ompi_osc_rdma_module_t *module, int peer_id,
return ret;
}

if (endpoint) {
btl = ompi_osc_rdma_selected_btl (module, module_btl_index);
}

if (MPI_WIN_FLAVOR_DYNAMIC == module->flavor) {
peer = (ompi_osc_rdma_peer_t *) OBJ_NEW(ompi_osc_rdma_peer_dynamic_t);
} else if (module->same_size && module->same_disp_unit) {
Expand Down Expand Up @@ -127,7 +122,6 @@ static int ompi_osc_rdma_peer_setup (ompi_osc_rdma_module_t *module, ompi_osc_rd
ompi_osc_rdma_peer_extended_t *ex_peer = (ompi_osc_rdma_peer_extended_t *) peer;
uint64_t peer_data_size;
uint64_t peer_data_offset, array_pointer;
mca_btl_base_module_t *array_btl;
struct mca_btl_base_endpoint_t *array_endpoint;
ompi_osc_rdma_region_t *array_peer_data, *node_peer_data;
ompi_osc_rdma_rank_data_t rank_data;
Expand Down
8 changes: 4 additions & 4 deletions ompi/mca/pml/ob1/pml_ob1_rdma.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ size_t mca_pml_ob1_rdma_pipeline_btls_count (mca_bml_base_endpoint_t* bml_endpoi
/* NTH: go ahead and use an rdma btl if is the only one */
bool ignore = !mca_pml_ob1.use_all_rdma;

for (int i = 0 ; i < num_eager_btls && ignore ; ++i) {
mca_bml_base_btl_t *eager_btl = mca_bml_base_btl_array_get_index (&bml_endpoint->btl_eager, i);
for (int j = 0 ; j < num_eager_btls && ignore ; ++j) {
mca_bml_base_btl_t *eager_btl = mca_bml_base_btl_array_get_index (&bml_endpoint->btl_eager, j);
if (eager_btl->btl_endpoint == bml_btl->btl_endpoint) {
ignore = false;
break;
Expand Down Expand Up @@ -154,8 +154,8 @@ size_t mca_pml_ob1_rdma_pipeline_btls( mca_bml_base_endpoint_t* bml_endpoint,
/* NTH: go ahead and use an rdma btl if is the only one */
bool ignore = !mca_pml_ob1.use_all_rdma;

for (int i = 0 ; i < num_eager_btls && ignore ; ++i) {
mca_bml_base_btl_t *eager_btl = mca_bml_base_btl_array_get_index (&bml_endpoint->btl_eager, i);
for (int j = 0 ; j < num_eager_btls && ignore ; ++j) {
mca_bml_base_btl_t *eager_btl = mca_bml_base_btl_array_get_index (&bml_endpoint->btl_eager, j);
if (eager_btl->btl_endpoint == bml_btl->btl_endpoint) {
ignore = false;
break;
Expand Down
2 changes: 1 addition & 1 deletion ompi/mca/sharedfp/lockedfile/sharedfp_lockedfile.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ struct mca_sharedfp_lockedfile_data
char* filename;
};

typedef struct mca_sharedfp_lockedfile_data lockedfile_data;
typedef struct mca_sharedfp_lockedfile_data lockedfile_data_global;


int mca_sharedfp_lockedfile_request_position (struct mca_sharedfp_base_data_t * sh,
Expand Down
2 changes: 1 addition & 1 deletion ompi/mca/sharedfp/lockedfile/sharedfp_lockedfile_file_open.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ int mca_sharedfp_lockedfile_file_close (ompio_file_t *fh)
}
sh = fh->f_sharedfp_data;

module_data = (lockedfile_data*)(sh->selected_module_data);
module_data = (lockedfile_data_global*)(sh->selected_module_data);
if ( module_data) {
/*Close lockedfile handle*/
if ( module_data->handle) {
Expand Down
2 changes: 1 addition & 1 deletion ompi/mca/sharedfp/sm/sharedfp_sm.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ struct mca_sharedfp_sm_data
char *sem_name; /* Name of the semaphore */
};

typedef struct mca_sharedfp_sm_data sm_data;
typedef struct mca_sharedfp_sm_data sm_data_global;


int mca_sharedfp_sm_request_position (ompio_file_t *fh,
Expand Down
2 changes: 1 addition & 1 deletion ompi/mca/sharedfp/sm/sharedfp_sm_file_open.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ int mca_sharedfp_sm_file_close (ompio_file_t *fh)
*/
fh->f_comm->c_coll->coll_barrier (fh->f_comm, fh->f_comm->c_coll->coll_barrier_module );

file_data = (sm_data*)(sh->selected_module_data);
file_data = (sm_data_global*)(sh->selected_module_data);
if (file_data) {
/*Close sm handle*/
if (file_data->sm_offset_ptr) {
Expand Down
8 changes: 4 additions & 4 deletions ompi/mca/topo/base/topo_base_cart_create.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,12 @@ int mca_topo_base_cart_create(mca_topo_base_module_t *topo,
return OMPI_ERR_OUT_OF_RESOURCE;
}
{ /* setup the cartesian topology */
int nprocs = num_procs, rank = new_rank;
int n_procs = num_procs, rank = new_rank;

for (i = 0; i < ndims; ++i) {
nprocs /= cart->dims[i];
cart->coords[i] = rank / nprocs;
rank %= nprocs;
n_procs /= cart->dims[i];
cart->coords[i] = rank / n_procs;
rank %= n_procs;
}
}
}
Expand Down
13 changes: 8 additions & 5 deletions ompi/mca/topo/treematch/topo_treematch_dist_graph_create.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,6 @@ int mca_topo_treematch_dist_graph_create(mca_topo_base_module_t* topo_module,

/* Centralized Reordering */
if (0 == mca_topo_treematch_component.reorder_mode) {
int *k = NULL;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a little unsure of this change. I think this is equivalent, but I'm not sure what k is supposed to be, and this function is huge.

int *obj_mapping = NULL;
int num_objs_total = 0;

Expand Down Expand Up @@ -693,12 +692,14 @@ int mca_topo_treematch_dist_graph_create(mca_topo_base_module_t* topo_module,
&newrank, 1, MPI_INT,
0, comm_old,
comm_old->c_coll->coll_scatter_module))) {
if (NULL != k) free(k);
if (NULL != k) { free(k); k = NULL; }
goto release_and_return;
}

if ( 0 == rank )
if ( 0 == rank ) {
free(k);
k = NULL;
}

/* this needs to be optimized but will do for now */
if (OMPI_SUCCESS != (err = ompi_comm_split(comm_old, 0, newrank, newcomm, false))) {
Expand Down Expand Up @@ -903,7 +904,7 @@ int mca_topo_treematch_dist_graph_create(mca_topo_base_module_t* topo_module,
&newrank, 1, MPI_INT,
0, localcomm,
localcomm->c_coll->coll_scatter_module))) {
if (NULL != k) free(k);
if (NULL != k) { free(k); k = NULL; };
ompi_comm_free(&localcomm);
free(lrank_to_grank);
free(grank_to_lrank);
Expand Down Expand Up @@ -932,8 +933,10 @@ int mca_topo_treematch_dist_graph_create(mca_topo_base_module_t* topo_module,
newrank += offset;
free(marked);

if (rank == lindex_to_grank[0])
if (rank == lindex_to_grank[0]) {
free(k);
k = NULL;
}

/* this needs to be optimized but will do for now */
if (OMPI_SUCCESS != (err = ompi_comm_split(comm_old, 0, newrank, newcomm, false))) {
Expand Down
Loading