Cleanup some OpenMP group code. (#267)

Signed-off-by: Samuel K. Gutierrez <[email protected]>

Author: samuelkgutierrez

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;

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);
     }
 };
 

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 <omp.h>
 
-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<int> 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