From 9d062779d793b22c1c5f7a4923ceff1067ad47f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20K=2E=20Guti=C3=A9rrez?= Date: Mon, 15 Jul 2024 09:50:49 -0600 Subject: [PATCH] Improve code. (#227) Signed-off-by: Samuel K. Gutierrez --- src/quo-vadis-mpi.cc | 10 +++++----- src/quo-vadis-omp.cc | 4 ++-- src/quo-vadis-process.cc | 6 +++--- src/quo-vadis-pthread.cc | 12 +++++++----- src/quo-vadis.cc | 26 +++++++++++++------------- src/qvi-group-mpi.h | 6 +++--- src/qvi-group-omp.h | 6 +++--- src/qvi-group-process.h | 6 +++--- src/qvi-group.h | 4 ++-- src/qvi-mpi.cc | 6 +++--- src/qvi-mpi.h | 4 ++-- src/qvi-omp.cc | 20 +++++++++----------- src/qvi-omp.h | 6 +++--- src/qvi-process.cc | 6 +++--- src/qvi-process.h | 4 ++-- src/qvi-scope.cc | 12 ++++++------ 16 files changed, 69 insertions(+), 69 deletions(-) diff --git a/src/quo-vadis-mpi.cc b/src/quo-vadis-mpi.cc index 7f22a99..112e517 100644 --- a/src/quo-vadis-mpi.cc +++ b/src/quo-vadis-mpi.cc @@ -61,8 +61,8 @@ qvi_mpi_scope_get( *scope = nullptr; // Create and initialize the base group. qvi_zgroup_mpi_s *izgroup = nullptr; - int rc = qvi_new(&izgroup, comm); - if (rc != QV_SUCCESS) return rc; + const int rc = qvi_new(&izgroup, comm); + if (qvi_unlikely(rc != QV_SUCCESS)) return rc; return qvi_scope_get(izgroup, iscope, scope); } @@ -73,7 +73,7 @@ qv_mpi_scope_get( qv_scope_intrinsic_t iscope, qv_scope_t **scope ) { - if (comm == MPI_COMM_NULL || !scope) { + if (qvi_unlikely(comm == MPI_COMM_NULL || !scope)) { return QV_ERR_INVLD_ARG; } try { @@ -87,7 +87,7 @@ qvi_mpi_scope_comm_dup( qv_scope_t *scope, MPI_Comm *comm ) { - qvi_group_mpi_t *mpi_group = dynamic_cast( + qvi_group_mpi_t *const mpi_group = dynamic_cast( qvi_scope_group_get(scope) ); return mpi_group->comm_dup(comm); @@ -98,7 +98,7 @@ qv_mpi_scope_comm_dup( qv_scope_t *scope, MPI_Comm *comm ) { - if (!scope || !comm) { + if (qvi_unlikely(!scope || !comm)) { return QV_ERR_INVLD_ARG; } try { diff --git a/src/quo-vadis-omp.cc b/src/quo-vadis-omp.cc index 5aea550..158f793 100644 --- a/src/quo-vadis-omp.cc +++ b/src/quo-vadis-omp.cc @@ -31,7 +31,7 @@ qvi_omp_scope_get( // Create the base process group. qvi_group_omp_s *zgroup = nullptr; const int rc = qvi_new(&zgroup); - if (rc != QV_SUCCESS) { + if (qvi_unlikely(rc != QV_SUCCESS)) { *scope = nullptr; return rc; } @@ -43,7 +43,7 @@ qv_omp_scope_get( qv_scope_intrinsic_t iscope, qv_scope_t **scope ) { - if (!scope) { + if (qvi_unlikely(!scope)) { return QV_ERR_INVLD_ARG; } try { diff --git a/src/quo-vadis-process.cc b/src/quo-vadis-process.cc index dfd1d4e..110524b 100644 --- a/src/quo-vadis-process.cc +++ b/src/quo-vadis-process.cc @@ -23,8 +23,8 @@ qvi_process_scope_get( ) { // Create the base process group. qvi_group_process_s *zgroup = nullptr; - int rc = qvi_new(&zgroup); - if (rc != QV_SUCCESS) { + const int rc = qvi_new(&zgroup); + if (qvi_unlikely(rc != QV_SUCCESS)) { *scope = nullptr; return rc; } @@ -36,7 +36,7 @@ qv_process_scope_get( qv_scope_intrinsic_t iscope, qv_scope_t **scope ) { - if (!scope) { + if (qvi_unlikely(!scope)) { return QV_ERR_INVLD_ARG; } try { diff --git a/src/quo-vadis-pthread.cc b/src/quo-vadis-pthread.cc index 1c13357..baa50d2 100644 --- a/src/quo-vadis-pthread.cc +++ b/src/quo-vadis-pthread.cc @@ -60,7 +60,9 @@ qv_pthread_scope_split( int nthreads, qv_scope_t ***subscope ) { - if (!scope || npieces < 0 || !color_array || nthreads < 0 || !subscope) { + const bool invld_args = !scope || npieces < 0 || + !color_array || nthreads < 0 || !subscope; + if (qvi_unlikely(invld_args)) { return QV_ERR_INVLD_ARG; } try { @@ -80,7 +82,7 @@ qv_pthread_scope_split_at( int nthreads, qv_scope_t ***subscopes ) { - if (!scope || !color_array || nthreads < 0 || !subscopes) { + if (qvi_unlikely(!scope || !color_array || nthreads < 0 || !subscopes)) { return QV_ERR_INVLD_ARG; } try { @@ -95,7 +97,7 @@ int qv_pthread_create( pthread_t *thread, const pthread_attr_t *attr, - void *(*thread_routine)(void *arg), + qvi_pthread_routine_fun_ptr_t thread_routine, void *arg, qv_scope_t *scope ) { @@ -104,7 +106,7 @@ qv_pthread_create( const int rc = qvi_new(&arg_ptr, scope, thread_routine, arg); // Since this is meant to behave similarly to // pthread_create(), return a reasonable errno. - if (rc != QV_SUCCESS) return ENOMEM; + if (qvi_unlikely(rc != QV_SUCCESS)) return ENOMEM; return pthread_create(thread, attr, qvi_pthread_routine, arg_ptr); } @@ -114,7 +116,7 @@ qv_pthread_scopes_free( int nscopes, qv_scope_t **scopes ) { - if (nscopes < 0 || !scopes) { + if (qvi_unlikely(nscopes < 0 || !scopes)) { return QV_ERR_INVLD_ARG; } try { diff --git a/src/quo-vadis.cc b/src/quo-vadis.cc index 77be250..3b04703 100644 --- a/src/quo-vadis.cc +++ b/src/quo-vadis.cc @@ -22,7 +22,7 @@ qv_version( int *minor, int *patch ) { - if (!major || !minor || !patch) { + if (qvi_unlikely(!major || !minor || !patch)) { return QV_ERR_INVLD_ARG; } @@ -37,7 +37,7 @@ int qv_scope_bind_push( qv_scope_t *scope ) { - if (!scope) { + if (qvi_unlikely(!scope)) { return QV_ERR_INVLD_ARG; } try { @@ -50,7 +50,7 @@ int qv_scope_bind_pop( qv_scope_t *scope ) { - if (!scope) { + if (qvi_unlikely(!scope)) { return QV_ERR_INVLD_ARG; } try { @@ -65,7 +65,7 @@ qv_scope_bind_string( qv_bind_string_format_t format, char **str ) { - if (!scope || !str) { + if (qvi_unlikely(!scope || !str)) { return QV_ERR_INVLD_ARG; } try { @@ -78,7 +78,7 @@ int qv_scope_free( qv_scope_t *scope ) { - if (!scope) { + if (qvi_unlikely(!scope)) { return QV_ERR_INVLD_ARG; } try { @@ -94,7 +94,7 @@ qv_scope_nobjs( qv_hw_obj_type_t obj, int *nobjs ) { - if (!scope || !nobjs) { + if (qvi_unlikely(!scope || !nobjs)) { return QV_ERR_INVLD_ARG; } try { @@ -108,7 +108,7 @@ qv_scope_taskid( qv_scope_t *scope, int *taskid ) { - if (!scope || !taskid) { + if (qvi_unlikely(!scope || !taskid)) { return QV_ERR_INVLD_ARG; } try { @@ -122,7 +122,7 @@ qv_scope_ntasks( qv_scope_t *scope, int *ntasks ) { - if (!scope || !ntasks) { + if (qvi_unlikely(!scope || !ntasks)) { return QV_ERR_INVLD_ARG; } try { @@ -135,7 +135,7 @@ int qv_scope_barrier( qv_scope_t *scope ) { - if (!scope) { + if (qvi_unlikely(!scope)) { return QV_ERR_INVLD_ARG; } try { @@ -153,7 +153,7 @@ qv_scope_create( qv_scope_create_hints_t hints, qv_scope_t **subscope ) { - if (!scope || (nobjs < 0) || !subscope) { + if (qvi_unlikely(!scope || (nobjs < 0) || !subscope)) { return QV_ERR_INVLD_ARG; } try { @@ -171,7 +171,7 @@ qv_scope_split( int color, qv_scope_t **subscope ) { - if (!scope || (npieces <= 0) | !subscope) { + if (qvi_unlikely(!scope || (npieces <= 0) | !subscope)) { return QV_ERR_INVLD_ARG; } try { @@ -192,7 +192,7 @@ qv_scope_split_at( int group_id, qv_scope_t **subscope ) { - if (!scope || !subscope) { + if (qvi_unlikely(!scope || !subscope)) { return QV_ERR_INVLD_ARG; } try { @@ -211,7 +211,7 @@ qv_scope_get_device_id( qv_device_id_type_t id_type, char **dev_id ) { - if (!scope || (dev_index < 0) || !dev_id) { + if (qvi_unlikely(!scope || (dev_index < 0) || !dev_id)) { return QV_ERR_INVLD_ARG; } try { diff --git a/src/qvi-group-mpi.h b/src/qvi-group-mpi.h index d4a5b0e..a416c31 100644 --- a/src/qvi-group-mpi.h +++ b/src/qvi-group-mpi.h @@ -80,11 +80,11 @@ struct qvi_group_mpi_s : public qvi_group_s { gather( qvi_bbuff_t *txbuff, int root, - qvi_bbuff_t ***rxbuffs, - int *shared + bool *shared, + qvi_bbuff_t ***rxbuffs ) { return qvi_mpi_group_gather_bbuffs( - mpi_group, txbuff, root, rxbuffs, shared + mpi_group, txbuff, root, shared, rxbuffs ); } diff --git a/src/qvi-group-omp.h b/src/qvi-group-omp.h index 8a40266..af90e1e 100644 --- a/src/qvi-group-omp.h +++ b/src/qvi-group-omp.h @@ -74,11 +74,11 @@ struct qvi_group_omp_s : public qvi_group_s { gather( qvi_bbuff_t *txbuff, int root, - qvi_bbuff_t ***rxbuffs, - int *shared + bool *shared, + qvi_bbuff_t ***rxbuffs ) { return qvi_omp_group_gather_bbuffs( - th_group, txbuff, root, rxbuffs, shared + th_group, txbuff, root, shared, rxbuffs ); } diff --git a/src/qvi-group-process.h b/src/qvi-group-process.h index 84068ff..95e21fd 100644 --- a/src/qvi-group-process.h +++ b/src/qvi-group-process.h @@ -78,11 +78,11 @@ struct qvi_group_process_s : public qvi_group_s { gather( qvi_bbuff_t *txbuff, int root, - qvi_bbuff_t ***rxbuffs, - int *shared + bool *shared, + qvi_bbuff_t ***rxbuffs ) { return qvi_process_group_gather_bbuffs( - proc_group, txbuff, root, rxbuffs, shared + proc_group, txbuff, root, shared, rxbuffs ); } diff --git a/src/qvi-group.h b/src/qvi-group.h index c0d0ca9..6ee9d24 100644 --- a/src/qvi-group.h +++ b/src/qvi-group.h @@ -81,8 +81,8 @@ struct qvi_group_s { gather( qvi_bbuff_t *txbuff, int root, - qvi_bbuff_t ***rxbuffs, - int *shared + bool *shared, + qvi_bbuff_t ***rxbuffs ) = 0; /** Scatters bbuffs from specified root. */ virtual int diff --git a/src/qvi-mpi.cc b/src/qvi-mpi.cc index bc321c6..506f529 100644 --- a/src/qvi-mpi.cc +++ b/src/qvi-mpi.cc @@ -363,8 +363,8 @@ qvi_mpi_group_gather_bbuffs( qvi_mpi_group_t *group, qvi_bbuff_t *txbuff, int root, - qvi_bbuff_t ***rxbuffs, - int *shared_alloc + bool *shared_alloc, + qvi_bbuff_t ***rxbuffs ) { const int send_count = (int)qvi_bbuff_size(txbuff); const int group_id = group->qvcomm.rank; @@ -434,7 +434,7 @@ qvi_mpi_group_gather_bbuffs( bbuffs = nullptr; } *rxbuffs = bbuffs; - *shared_alloc = 0; + *shared_alloc = false; return rc; } diff --git a/src/qvi-mpi.h b/src/qvi-mpi.h index 1e6b401..8e8a808 100644 --- a/src/qvi-mpi.h +++ b/src/qvi-mpi.h @@ -151,8 +151,8 @@ qvi_mpi_group_gather_bbuffs( qvi_mpi_group_t *group, qvi_bbuff_t *txbuff, int root, - qvi_bbuff_t ***rxbuffs, - int *shared_alloc + bool *shared_alloc, + qvi_bbuff_t ***rxbuffs ); /** diff --git a/src/qvi-omp.cc b/src/qvi-omp.cc index 8569005..568405b 100644 --- a/src/qvi-omp.cc +++ b/src/qvi-omp.cc @@ -212,33 +212,31 @@ qvi_omp_group_gather_bbuffs( qvi_omp_group_t *group, qvi_bbuff_t *txbuff, int, - qvi_bbuff_t ***rxbuffs, - int *shared_alloc + bool *shared_alloc, + qvi_bbuff_t ***rxbuffs ) { const int group_size = group->size; - const int group_id = group->rank; + const int group_rank = group->rank; qvi_bbuff_t **bbuffs = nullptr; #pragma omp single copyprivate(bbuffs) bbuffs = new qvi_bbuff_t *[group_size](); - const int rc = qvi_bbuff_dup(txbuff, &bbuffs[group_id]); + const int rc = qvi_bbuff_dup(txbuff, &bbuffs[group_rank]); // Need to ensure that all threads have contributed to bbuffs. #pragma omp barrier if (rc != QV_SUCCESS) { #pragma omp single - { - if (bbuffs) { - for (int i = 0; i < group_size; ++i) { - qvi_bbuff_free(&bbuffs[i]); - } - delete[] bbuffs; + if (bbuffs) { + for (int i = 0; i < group_size; ++i) { + qvi_bbuff_free(&bbuffs[i]); } + delete[] bbuffs; } bbuffs = nullptr; } *rxbuffs = bbuffs; - *shared_alloc = 1; + *shared_alloc = true; return rc; } diff --git a/src/qvi-omp.h b/src/qvi-omp.h index 318f6fe..3494f10 100644 --- a/src/qvi-omp.h +++ b/src/qvi-omp.h @@ -92,9 +92,9 @@ int qvi_omp_group_gather_bbuffs( qvi_omp_group_t *group, qvi_bbuff_t *txbuff, - int root, - qvi_bbuff_t ***rxbuffs, - int *shared_alloc + int, + bool *shared_alloc, + qvi_bbuff_t ***rxbuffs ); int diff --git a/src/qvi-process.cc b/src/qvi-process.cc index 2ebe90b..c5183c4 100644 --- a/src/qvi-process.cc +++ b/src/qvi-process.cc @@ -63,8 +63,8 @@ qvi_process_group_gather_bbuffs( qvi_process_group_t *group, qvi_bbuff_t *txbuff, int root, - qvi_bbuff_t ***rxbuffs, - int *shared_alloc + bool *shared, + qvi_bbuff_t ***rxbuffs ) { const int group_size = qvi_process_group_size(group); // Make sure that we are dealing with a valid process group. @@ -84,7 +84,7 @@ qvi_process_group_gather_bbuffs( bbuffs = nullptr; } *rxbuffs = bbuffs; - *shared_alloc = 0; + *shared = false; return rc; } diff --git a/src/qvi-process.h b/src/qvi-process.h index fd49f46..a678845 100644 --- a/src/qvi-process.h +++ b/src/qvi-process.h @@ -68,8 +68,8 @@ qvi_process_group_gather_bbuffs( qvi_process_group_t *group, qvi_bbuff_t *txbuff, int root, - qvi_bbuff_t ***rxbuffs, - int *shared_alloc + bool *shared, + qvi_bbuff_t ***rxbuffs ); /** diff --git a/src/qvi-scope.cc b/src/qvi-scope.cc index c3bbfb1..0e75173 100644 --- a/src/qvi-scope.cc +++ b/src/qvi-scope.cc @@ -230,11 +230,11 @@ gather_values( ) { static_assert(std::is_trivially_copyable::value, ""); - int rc = QV_SUCCESS, shared = 0; + bool shared = false; const uint_t group_size = group->size(); qvi_bbuff_t *txbuff = nullptr, **bbuffs = nullptr; - rc = qvi_bbuff_new(&txbuff); + int rc = qvi_bbuff_new(&txbuff); if (rc != QV_SUCCESS) goto out; rc = qvi_bbuff_append( @@ -242,7 +242,7 @@ gather_values( ); if (rc != QV_SUCCESS) goto out; - rc = group->gather(txbuff, root, &bbuffs, &shared); + rc = group->gather(txbuff, root, &shared, &bbuffs); if (rc != QV_SUCCESS) goto out; if (group->id() == root) { @@ -276,17 +276,17 @@ gather_hwpools( qvi_hwpool_s *txpool, std::vector &rxpools ) { - int rc = QV_SUCCESS, shared = 0; + bool shared = false; const uint_t group_size = group->size(); qvi_bbuff_t *txbuff = nullptr, **bbuffs = nullptr; - rc = qvi_bbuff_new(&txbuff); + int rc = qvi_bbuff_new(&txbuff); if (rc != QV_SUCCESS) goto out; rc = txpool->pack(txbuff); if (rc != QV_SUCCESS) goto out; - rc = group->gather(txbuff, root, &bbuffs, &shared); + rc = group->gather(txbuff, root, &shared, &bbuffs); if (rc != QV_SUCCESS) goto out; if (group->id() == root) {