Cleanup some more code. (#264)

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

Author: samuelkgutierrez

src/qvi-hwpool.cc

@@ -123,6 +123,12 @@ pool_release_cpus_by_cpuset(
 }
 #endif
 
+qv_scope_create_hints_t
+qvi_hwpool_res_s::hints(void)
+{
+    return m_hints;
+}
+
 qvi_hwloc_bitmap_s &
 qvi_hwpool_cpu_s::cpuset(void)
 {

src/qvi-hwpool.h

@@ -26,14 +26,18 @@ struct qvi_hwpool_res_s {
 protected:
     /** Resource hint flags. */
     qv_scope_create_hints_t m_hints = QV_SCOPE_CREATE_HINT_NONE;
+public:
+    /** Returns the resource's create hints. */
+    qv_scope_create_hints_t
+    hints(void);
 };
 
 /**
  * Defines a hardware pool CPU. A CPU here may have multiple
  * processing units (PUs), which are defined in the CPU's cpuset.
  */
 struct qvi_hwpool_cpu_s : qvi_hwpool_res_s {
-protected:
+private:
     /** The cpuset of the CPU's PUs. */
     qvi_hwloc_bitmap_s m_cpuset;
 public:

src/qvi-hwsplit.cc

@@ -49,7 +49,7 @@ qvi_hwsplit_s::~qvi_hwsplit_s(void)
 void
 qvi_hwsplit_s::reserve(void)
 {
-    m_taskids.resize(m_group_size);
+    m_group_tids.resize(m_group_size);
     m_hwpools.resize(m_group_size);
     m_colors.resize(m_group_size);
     m_affinities.resize(m_group_size);
@@ -237,7 +237,7 @@ qvi_hwsplit_s::split_devices_affinity_preserving(void)
         }
         // Store device affinities.
         qvi_hwloc_cpusets_t devaffs;
-        for (auto &dev : devs) {
+        for (const auto &dev : devs) {
             devaffs.push_back(dev->affinity());
         }
 
@@ -429,7 +429,7 @@ qvi_hwsplit_s::split(void)
     return rc;
 }
 
-qvi_coll_hwsplit_s::qvi_coll_hwsplit_s(
+qvi_hwsplit_coll_s::qvi_hwsplit_coll_s(
     qv_scope_t *parent,
     uint_t npieces,
     int color,
@@ -438,7 +438,7 @@ qvi_coll_hwsplit_s::qvi_coll_hwsplit_s(
   , m_color(color)
 {
     const qvi_group_t *const pgroup = m_parent->group();
-    if (pgroup->rank() == qvi_coll_hwsplit_s::s_rootid) {
+    if (pgroup->rank() == qvi_hwsplit_coll_s::s_rootid) {
         m_hwsplit = qvi_hwsplit_s(
             m_parent, pgroup->size(), npieces, split_at_type
         );
@@ -447,7 +447,7 @@ qvi_coll_hwsplit_s::qvi_coll_hwsplit_s(
 
 template <typename TYPE>
 int
-qvi_coll_hwsplit_s::scatter_values(
+qvi_hwsplit_coll_s::scatter_values(
     const std::vector<TYPE> &values,
     TYPE *value
 ) {
@@ -490,7 +490,7 @@ qvi_coll_hwsplit_s::scatter_values(
 
 template <typename TYPE>
 int
-qvi_coll_hwsplit_s::bcast_value(
+qvi_hwsplit_coll_s::bcast_value(
     TYPE *value
 ) {
     static_assert(std::is_trivially_copyable<TYPE>::value, "");
@@ -506,7 +506,7 @@ qvi_coll_hwsplit_s::bcast_value(
 
 template <typename TYPE>
 int
-qvi_coll_hwsplit_s::gather_values(
+qvi_hwsplit_coll_s::gather_values(
     TYPE invalue,
     std::vector<TYPE> &outvals
 ) {
@@ -554,7 +554,7 @@ qvi_coll_hwsplit_s::gather_values(
 }
 
 int
-qvi_coll_hwsplit_s::gather_hwpools(
+qvi_hwsplit_coll_s::gather_hwpools(
     qvi_hwpool_s *txpool,
     std::vector<qvi_hwpool_s *> &rxpools
 ) {
@@ -597,20 +597,19 @@ qvi_coll_hwsplit_s::gather_hwpools(
 }
 
 int
-qvi_coll_hwsplit_s::gather(void)
+qvi_hwsplit_coll_s::gather(void)
 {
-    int rc = gather_values(qvi_task_t::mytid(), m_hwsplit.m_taskids);
+    int rc = gather_values(qvi_task_t::mytid(), m_hwsplit.m_group_tids);
+    if (qvi_unlikely(rc != QV_SUCCESS)) return rc;
+    rc = gather_values(m_color, m_hwsplit.m_colors);
     if (qvi_unlikely(rc != QV_SUCCESS)) return rc;
     // Note that the result hwpools are copies, so we can modify them freely.
     rc = gather_hwpools(m_parent->hwpool(), m_hwsplit.m_hwpools);
     if (qvi_unlikely(rc != QV_SUCCESS)) return rc;
 
-    rc = gather_values(m_color, m_hwsplit.m_colors);
-    if (qvi_unlikely(rc != QV_SUCCESS)) return rc;
-
-    const int myid = m_parent->group()->rank();
+    const int myrank = m_parent->group()->rank();
     const uint_t group_size = m_parent->group()->size();
-    if (myid == qvi_coll_hwsplit_s::s_rootid) {
+    if (myrank == qvi_hwsplit_coll_s::s_rootid) {
         m_hwsplit.m_affinities.resize(group_size);
         for (uint_t tid = 0; tid < group_size; ++tid) {
             hwloc_cpuset_t cpuset = nullptr;
@@ -627,7 +626,7 @@ qvi_coll_hwsplit_s::gather(void)
 }
 
 int
-qvi_coll_hwsplit_s::scatter_hwpools(
+qvi_hwsplit_coll_s::scatter_hwpools(
     const std::vector<qvi_hwpool_s *> &pools,
     qvi_hwpool_s **pool
 ) {
@@ -667,7 +666,7 @@ qvi_coll_hwsplit_s::scatter_hwpools(
 }
 
 int
-qvi_coll_hwsplit_s::scatter(
+qvi_hwsplit_coll_s::scatter(
     int *colorp,
     qvi_hwpool_s **result
 ) {
@@ -677,13 +676,13 @@ qvi_coll_hwsplit_s::scatter(
 }
 
 int
-qvi_coll_hwsplit_s::barrier(void)
+qvi_hwsplit_coll_s::barrier(void)
 {
     return m_parent->group()->barrier();
 }
 
 int
-qvi_coll_hwsplit_s::split(
+qvi_hwsplit_coll_s::split(
     int *colorp,
     qvi_hwpool_s **result
 ) {

src/qvi-hwsplit.h

@@ -20,14 +20,14 @@
 #include "qvi-map.h"
 
 /**
- * Split aggregation: a collection of data relevant to split operations
- * requiring aggregated (e.g., global) knowledge to perform a split.
+ * Hardware split aggregation: a collection of information relevant to split
+ * operations requiring aggregated (e.g., global) knowledge to perform a split.
  *
  * NOTE: since splitting and mapping operations are performed by a single
  * process, this structure does not support collective operations that require
  * coordination between cooperating tasks. The structure for that is
- * qvi_scope_coll_data_s. Typically, collective operations will fill in a
- * qvi_scope_split_agg_s, but that isn't a requirement.
+ * qvi_hwsplit_coll_s. Typically, collective operations will fill in a
+ * this structure, but that isn't a requirement.
  */
 struct qvi_hwsplit_s {
 //private:
@@ -47,12 +47,12 @@ struct qvi_hwsplit_s {
      */
     qv_hw_obj_type_t m_split_at_type;
     /**
-     * Vector of task IDs, one for each member of the group. Note that the
+     * Vector of task TIDs, one for each member of the group. Note that the
      * number of task IDs will always match the group size and that their array
      * index corresponds to a task ID. It is handy to have the task IDs for
-     * splitting so we can query task characteristics during a splitting.
+     * splitting so we can query task characteristics during a split.
      */
-    std::vector<pid_t> m_taskids;
+    std::vector<pid_t> m_group_tids;
     /**
      * Vector of hardware pools, one for each member of the group. Note that the
      * number of hardware pools will always match the group size and that their
@@ -153,7 +153,7 @@ struct qvi_hwsplit_s {
  * split operations requiring aggregated resource knowledge AND coordination
  * between tasks in the parent scope to perform a split.
  */
-struct qvi_coll_hwsplit_s {
+struct qvi_hwsplit_coll_s {
     /**
      * The root task ID used for collective operations.
      * We use 0 as the root because 0 will always exist.
@@ -169,7 +169,7 @@ struct qvi_coll_hwsplit_s {
      */
     qvi_hwsplit_s m_hwsplit;
     /** Constructor. */
-    qvi_coll_hwsplit_s(void) = delete;
+    qvi_hwsplit_coll_s(void) = delete;
     /** Constructor. */
     /**
      * Hardware resources will be split based on the provided split parameters:
@@ -179,7 +179,7 @@ struct qvi_coll_hwsplit_s {
      *   maybe_obj_type: Potentially the object type that we are splitting at. This
      *   value influences how the splitting algorithms perform their mapping.
      */
-    qvi_coll_hwsplit_s(
+    qvi_hwsplit_coll_s(
         qv_scope_t *parent,
         uint_t npieces,
         int color,

src/qvi-omp.cc

@@ -73,6 +73,8 @@ int
 qvi_omp_group_barrier(
     qvi_omp_group_t *
 ) {
+    // TODO(skg) What should we do about barriers here? In particular, we need
+    // to be careful about sub-groups, etc.
     return QV_SUCCESS;
 }
 

src/qvi-scope.cc

@@ -244,7 +244,7 @@ qv_scope_s::split(
     qvi_group_t *group = nullptr;
     qv_scope_t *ichild = nullptr;
     // Split the hardware resources based on the provided split parameters.
-    qvi_coll_hwsplit_s chwsplit(
+    qvi_hwsplit_coll_s chwsplit(
         this, npieces, color, maybe_obj_type
     );
     rc = chwsplit.split(&colorp, &hwpool);
@@ -307,7 +307,7 @@ qv_scope_s::thsplit(
         rc = qvi_dup(*m_hwpool, &hwsplit.m_hwpools[i]);
         if (rc != QV_SUCCESS) break;
         // Since this is called by a single task, replicate its task ID, too.
-        hwsplit.m_taskids[i] = taskid;
+        hwsplit.m_group_tids[i] = taskid;
         // Same goes for the task's affinity.
         hwsplit.m_affinities[i].set(task_affinity);
     }

src/qvi-utils.h

@@ -39,6 +39,7 @@ struct qvi_refc_s {
     void
     release(void) const
     {
+        assert(refc > 0);
         if (--refc == 0) {
             delete this;
         }