From 72ca9d52984ad4431188248884f763c12eac13f4 Mon Sep 17 00:00:00 2001 From: "Samuel K. Gutierrez" Date: Thu, 1 Aug 2024 20:26:12 -0600 Subject: [PATCH] Cleanup some OpenMP group code. Signed-off-by: Samuel K. Gutierrez --- src/qvi-group-omp.cc | 20 ++++--- src/qvi-group-omp.h | 16 +++--- src/qvi-omp.cc | 106 +++++++++++++++---------------------- src/qvi-omp.h | 121 +++++++++++++++++++++++-------------------- 4 files changed, 122 insertions(+), 141 deletions(-) diff --git a/src/qvi-group-omp.cc b/src/qvi-group-omp.cc index 5a8e51a..14a4ae6 100644 --- a/src/qvi-group-omp.cc +++ b/src/qvi-group-omp.cc @@ -25,12 +25,12 @@ qvi_group_omp::qvi_group_omp(void) { const int rc = qvi_new(&m_task); - if (rc != QV_SUCCESS) throw qvi_runtime_error(); + if (qvi_unlikely(rc != QV_SUCCESS)) throw qvi_runtime_error(); } qvi_group_omp::~qvi_group_omp(void) { - qvi_omp_group_delete(&m_ompgroup); + qvi_omp_group::destroy(&m_ompgroup); qvi_delete(&m_task); } @@ -42,7 +42,7 @@ qvi_group_omp::make_intrinsic( const int group_rank = omp_get_thread_num(); // NOTE: the provided scope doesn't affect how // we create the thread group, so we ignore it. - return qvi_omp_group_new(group_size, group_rank, &m_ompgroup); + return qvi_omp_group::create(group_size, group_rank, &m_ompgroup); } int @@ -53,11 +53,11 @@ qvi_group_omp::self( const int group_rank = 0; qvi_group_omp *ichild = nullptr; int rc = qvi_new(&ichild); - if (rc != QV_SUCCESS) goto out; + if (qvi_unlikely(rc != QV_SUCCESS)) goto out; // Create a group containing a single thread. - rc = qvi_omp_group_new(group_size, group_rank, &ichild->m_ompgroup); + rc = qvi_omp_group::create(group_size, group_rank, &ichild->m_ompgroup); out: - if (rc != QV_SUCCESS) { + if (qvi_unlikely(rc != QV_SUCCESS)) { qvi_delete(&ichild); } *child = ichild; @@ -72,13 +72,11 @@ qvi_group_omp::split( ) { qvi_group_omp *ichild = nullptr; int rc = qvi_new(&ichild); - if (rc != QV_SUCCESS) goto out; + if (qvi_unlikely(rc != QV_SUCCESS)) goto out; - rc = qvi_omp_group_create_from_split( - m_ompgroup, color, key, &ichild->m_ompgroup - ); + rc = m_ompgroup->split(color, key, &ichild->m_ompgroup); out: - if (rc != QV_SUCCESS) { + if (qvi_unlikely(rc != QV_SUCCESS)) { qvi_delete(&ichild); } *child = ichild; diff --git a/src/qvi-group-omp.h b/src/qvi-group-omp.h index ee41aaf..1e12883 100644 --- a/src/qvi-group-omp.h +++ b/src/qvi-group-omp.h @@ -29,7 +29,7 @@ struct qvi_group_omp : public qvi_group { /** Task associated with this group. */ qvi_task *m_task = nullptr; /** Underlying group instance. */ - qvi_omp_group_t *m_ompgroup = nullptr; + qvi_omp_group *m_ompgroup = nullptr; public: /** Constructor. */ qvi_group_omp(void); @@ -45,19 +45,19 @@ struct qvi_group_omp : public qvi_group { virtual int rank(void) const { - return qvi_omp_group_id(m_ompgroup); + return m_ompgroup->rank(); } virtual int size(void) const { - return qvi_omp_group_size(m_ompgroup); + return m_ompgroup->size(); } virtual int barrier(void) { - return qvi_omp_group_barrier(m_ompgroup); + return m_ompgroup->barrier(); } virtual int @@ -93,9 +93,7 @@ struct qvi_group_omp : public qvi_group { bool *shared, qvi_bbuff ***rxbuffs ) { - return qvi_omp_group_gather_bbuffs( - m_ompgroup, txbuff, root, shared, rxbuffs - ); + return m_ompgroup->gather(txbuff, root, shared, rxbuffs); } virtual int @@ -104,9 +102,7 @@ struct qvi_group_omp : public qvi_group { int root, qvi_bbuff **rxbuff ) { - return qvi_omp_group_scatter_bbuffs( - m_ompgroup, txbuffs, root, rxbuff - ); + return m_ompgroup->scatter(txbuffs, root, rxbuff); } }; diff --git a/src/qvi-omp.cc b/src/qvi-omp.cc index d1109a1..84faaf2 100644 --- a/src/qvi-omp.cc +++ b/src/qvi-omp.cc @@ -18,83 +18,67 @@ */ #include "qvi-omp.h" -#include "qvi-subgroup.h" #include "qvi-bbuff.h" #include "qvi-utils.h" #include -struct qvi_omp_group_s { - /** Group size. */ - int size = 0; - /** ID (rank) in group: this ID is unique to each thread. */ - int rank = 0; - /** Constructor. */ - qvi_omp_group_s(void) = delete; - /** Constructor. */ - qvi_omp_group_s( - int group_size, - int group_rank - ) : size(group_size) - , rank(group_rank) { } -}; +qvi_omp_group::qvi_omp_group( + int group_size, + int group_rank +) : m_size(group_size) + , m_rank(group_rank) { } int -qvi_omp_group_new( +qvi_omp_group::create( int group_size, int group_rank, - qvi_omp_group_t **group + qvi_omp_group **group ) { return qvi_new(group, group_size, group_rank); } void -qvi_omp_group_delete( - qvi_omp_group_t **group +qvi_omp_group::destroy( + qvi_omp_group **group ) { #pragma omp barrier qvi_delete(group); } int -qvi_omp_group_id( - const qvi_omp_group_t *group -) { - return group->rank; +qvi_omp_group::size(void) +{ + return m_size; } int -qvi_omp_group_size( - const qvi_omp_group_t *group -) { - return group->size; +qvi_omp_group::rank(void) +{ + return m_rank; } int -qvi_omp_group_barrier( - qvi_omp_group_t * -) { +qvi_omp_group::barrier(void) +{ // TODO(skg) What should we do about barriers here? In particular, we need // to be careful about sub-groups, etc. return QV_SUCCESS; } -static int -qvi_get_subgroup_info( - qvi_omp_group_t *parent, +int +qvi_omp_group::subgroup_info( int color, int key, qvi_subgroup_info_s *sginfo ) { - const int size = parent->size; - const int rank = parent->rank; qvi_subgroup_color_key_rank_s *ckrs = nullptr; #pragma omp single copyprivate(ckrs) - ckrs = new qvi_subgroup_color_key_rank_s[size]; + ckrs = new qvi_subgroup_color_key_rank_s[m_size]; // Gather colors and keys from ALL threads. - ckrs[rank].color = color; - ckrs[rank].key = key; - ckrs[rank].rank = rank; + ckrs[m_rank].color = color; + ckrs[m_rank].key = key; + ckrs[m_rank].rank = m_rank; // Barrier to be sure that all threads have contributed their values. #pragma omp barrier // Since these data are shared, only one thread has to sort them. The same @@ -105,12 +89,12 @@ qvi_get_subgroup_info( // Sort the color/key/rank array. First according to color, then by key, // but in the same color realm. If color and key are identical, sort by // the rank from given group. - std::sort(ckrs, ckrs + size, qvi_subgroup_color_key_rank_s::by_color); - std::sort(ckrs, ckrs + size, qvi_subgroup_color_key_rank_s::by_key); - std::sort(ckrs, ckrs + size, qvi_subgroup_color_key_rank_s::by_rank); + std::sort(ckrs, ckrs + m_size, qvi_subgroup_color_key_rank_s::by_color); + std::sort(ckrs, ckrs + m_size, qvi_subgroup_color_key_rank_s::by_key); + std::sort(ckrs, ckrs + m_size, qvi_subgroup_color_key_rank_s::by_rank); // Calculate the number of distinct colors provided. std::set color_set; - for (int i = 0; i < size; ++i) { + for (int i = 0; i < m_size; ++i) { color_set.insert(ckrs[i].color); } ncolors = color_set.size(); @@ -120,12 +104,12 @@ qvi_get_subgroup_info( // Compute my sub-group size and sub-group rank. int group_rank = 0; int group_size = 0; - for (int i = 0; i < size; ++i) { + for (int i = 0; i < m_size; ++i) { if (color != ckrs[i].color) continue; // Else we found the start of my color group. const int current_color = ckrs[i].color; - for (int j = i; j < size && current_color == ckrs[j].color; ++j) { - if (ckrs[j].rank == rank) { + for (int j = i; j < m_size && current_color == ckrs[j].color; ++j) { + if (ckrs[j].rank == m_rank) { sginfo->rank = group_rank; } group_size++; @@ -143,18 +127,15 @@ qvi_get_subgroup_info( } int -qvi_omp_group_create_from_split( - qvi_omp_group_t *parent, +qvi_omp_group::split( int color, int key, - qvi_omp_group_t **child + qvi_omp_group **child ) { - qvi_omp_group_t *ichild = nullptr; + qvi_omp_group *ichild = nullptr; qvi_subgroup_info_s sginfo; - int rc = qvi_get_subgroup_info( - parent, color, key, &sginfo - ); + int rc = subgroup_info(color, key, &sginfo); if (qvi_likely(rc == QV_SUCCESS)) { rc = qvi_new(&ichild, sginfo.size, sginfo.rank); } @@ -166,27 +147,23 @@ qvi_omp_group_create_from_split( } int -qvi_omp_group_gather_bbuffs( - qvi_omp_group_t *group, +qvi_omp_group::gather( qvi_bbuff *txbuff, int, bool *shared_alloc, qvi_bbuff ***rxbuffs ) { - const int group_size = group->size; - const int group_rank = group->rank; - qvi_bbuff **bbuffs = nullptr; #pragma omp single copyprivate(bbuffs) - bbuffs = new qvi_bbuff *[group_size](); + bbuffs = new qvi_bbuff *[m_size](); - const int rc = qvi_bbuff_dup(*txbuff, &bbuffs[group_rank]); + const int rc = qvi_bbuff_dup(*txbuff, &bbuffs[m_rank]); // Need to ensure that all threads have contributed to bbuffs. #pragma omp barrier - if (rc != QV_SUCCESS) { + if (qvi_unlikely(rc != QV_SUCCESS)) { #pragma omp single if (bbuffs) { - for (int i = 0; i < group_size; ++i) { + for (int i = 0; i < m_size; ++i) { qvi_bbuff_delete(&bbuffs[i]); } delete[] bbuffs; @@ -199,8 +176,7 @@ qvi_omp_group_gather_bbuffs( } int -qvi_omp_group_scatter_bbuffs( - qvi_omp_group_t *group, +qvi_omp_group::scatter( qvi_bbuff **txbuffs, int, qvi_bbuff **rxbuff @@ -211,7 +187,7 @@ qvi_omp_group_scatter_bbuffs( #pragma omp master *tmp = txbuffs; #pragma omp barrier - qvi_bbuff *inbuff = (*tmp)[group->rank]; + qvi_bbuff *inbuff = (*tmp)[m_rank]; qvi_bbuff *mybbuff = nullptr; const int rc = qvi_bbuff_dup(*inbuff, &mybbuff); #pragma omp barrier diff --git a/src/qvi-omp.h b/src/qvi-omp.h index cca9dba..ab1070f 100644 --- a/src/qvi-omp.h +++ b/src/qvi-omp.h @@ -21,10 +21,7 @@ #define QVI_OMP_H #include "qvi-common.h" - -// Forward declarations. -struct qvi_omp_group_s; -typedef struct qvi_omp_group_s qvi_omp_group_t; +#include "qvi-subgroup.h" #if 0 /** @@ -53,57 +50,71 @@ typedef struct qv_layout_s { } qv_layout_t; #endif -int -qvi_omp_group_new( - int group_size, - int group_rank, - qvi_omp_group_t **group -); - -void -qvi_omp_group_delete( - qvi_omp_group_t **group -); - -int -qvi_omp_group_size( - const qvi_omp_group_t *group -); - -int -qvi_omp_group_id( - const qvi_omp_group_t *group -); - -int -qvi_omp_group_barrier( - qvi_omp_group_t *group -); - -int -qvi_omp_group_create_from_split( - qvi_omp_group_t *parent, - int color, - int key, - qvi_omp_group_t **child -); - -int -qvi_omp_group_gather_bbuffs( - qvi_omp_group_t *group, - qvi_bbuff *txbuff, - int, - bool *shared_alloc, - qvi_bbuff ***rxbuffs -); - -int -qvi_omp_group_scatter_bbuffs( - qvi_omp_group_t *group, - qvi_bbuff **txbuffs, - int root, - qvi_bbuff **rxbuff -); +struct qvi_omp_group { +private: + /** Group size. */ + int m_size = 0; + /** ID (rank) in group: this ID is unique to each thread. */ + int m_rank = 0; + /** */ + int + subgroup_info( + int color, + int key, + qvi_subgroup_info_s *sginfo + ); +public: + /** Constructor. */ + qvi_omp_group(void) = delete; + /** Constructor. */ + qvi_omp_group( + int group_size, + int group_rank + ); + + static int + create( + int group_size, + int group_rank, + qvi_omp_group **group + ); + + static void + destroy( + qvi_omp_group **group + ); + + int + size(void); + + int + rank(void); + + int + barrier(void); + + int + split( + int color, + int key, + qvi_omp_group **child + ); + + int + gather( + qvi_bbuff *txbuff, + int root, + bool *shared, + qvi_bbuff ***rxbuffs + ); + + int + scatter( + qvi_bbuff **txbuffs, + int root, + qvi_bbuff **rxbuff + ); +}; #endif