[OpenMP][libomp] Allow users to specify KMP_HW_SUBSET in any order

[jlpeyton]

Summary

Remove restriction forcing users to specify the KMP_HW_SUBSET value in
topology order. This patch sorts the user KMP_HW_SUBSET value before
trying to apply it. For example: 1s,4c,2t is equivalent to 2t,1s,4c

[ye-luo]

Can we have user documentation for KMP_XX environment variables on https://openmp.llvm.org//design/Runtimes.html#openmp-runtimes?

[jlpeyton]

In D112027#3071416, @ye-luo wrote:

Can we have user documentation for KMP_XX environment variables on https://openmp.llvm.org//design/Runtimes.html#openmp-runtimes?

We're trying to figure out if we can use the text from https://www.intel.com/content/www/us/en/develop/documentation/oneapi-dpcpp-cpp-compiler-dev-guide-and-reference/top/compilation/supported-environment-variables.html instead of re-doing the definitions in two separate places.

[AndreyChurbanov]

LGTM

The problem with absent documentation can be addressed in a separate patch I think.

openmp/runtime/src/kmp_affinity.h

#if KMP_AFFINITY_SUPPORTED
   void sort_compact() {
     qsort(hw_threads, num_hw_threads, sizeof(kmp_hw_thread_t),
           kmp_hw_thread_t::compare_compact);
   }
 #endif
   void print(const char *env_var = "KMP_AFFINITY") const;
   void dump() const;
 };
+extern kmp_topology_t *__kmp_topology;
 
 class kmp_hw_subset_t {
 public:
   struct item_t {
     int num;
     kmp_hw_t type;
     int offset;
   };
 
 private:
   int depth;
   int capacity;
   item_t *items;
   kmp_uint64 set;
   bool absolute;
   // The set must be able to handle up to KMP_HW_LAST number of layers
   KMP_BUILD_ASSERT(sizeof(set) * 8 >= KMP_HW_LAST);
+  // Sorting the KMP_HW_SUBSET items to follow topology order
+  // All unknown topology types will be at the beginning of the subset
+  static int hw_subset_compare(const void *i1, const void *i2) {
+    kmp_hw_t type1 = ((const item_t *)i1)->type;
+    kmp_hw_t type2 = ((const item_t *)i2)->type;
+    int level1 = __kmp_topology->get_level(type1);
+    int level2 = __kmp_topology->get_level(type2);
+    return level1 - level2;
+  }
 
 public:
   // Force use of allocate()/deallocate()
   kmp_hw_subset_t() = delete;
   kmp_hw_subset_t(const kmp_hw_subset_t &t) = delete;
   kmp_hw_subset_t(kmp_hw_subset_t &&t) = delete;
   kmp_hw_subset_t &operator=(const kmp_hw_subset_t &t) = delete;
   kmp_hw_subset_t &operator=(kmp_hw_subset_t &&t) = delete;
public:
   void remove(int index) {
     KMP_DEBUG_ASSERT(index >= 0 && index < depth);
     set &= ~(1ull << items[index].type);
     for (int j = index + 1; j < depth; ++j) {
       items[j - 1] = items[j];
     }
     depth--;
   }
+  void sort() {
+    KMP_DEBUG_ASSERT(__kmp_topology);
+    qsort(items, depth, sizeof(item_t), hw_subset_compare);
+  }
   bool specified(kmp_hw_t type) const { return ((set & (1ull << type)) > 0); }
   void dump() const {
     printf("**********************\n");
     printf("*** kmp_hw_subset: ***\n");
     printf("* depth: %d\n", depth);
     printf("* items:\n");
     for (int i = 0; i < depth; ++i) {
       printf("num: %d, type: %s, offset: %d\n", items[i].num,
              __kmp_hw_get_keyword(items[i].type), items[i].offset);
     }
     printf("* set: 0x%llx\n", set);
     printf("* absolute: %d\n", absolute);
     printf("**********************\n");
   }
 };
-
-extern kmp_topology_t *__kmp_topology;
 extern kmp_hw_subset_t *__kmp_hw_subset;
 
 /* A structure for holding machine-specific hierarchy info to be computed once
    at init. This structure represents a mapping of threads to the actual machine
    hierarchy, or to our best guess at what the hierarchy might be, for the
    purpose of performing an efficient barrier. In the worst case, when there is
    no machine hierarchy information, it produces a tree suitable for a barrier,
    similar to the tree used in the hyper barrier. */

openmp/runtime/src/kmp_affinity.cpp

 // Apply the KMP_HW_SUBSET envirable to the topology
 // Returns true if KMP_HW_SUBSET filtered any processors
 // otherwise, returns false
 bool kmp_topology_t::filter_hw_subset() {
   // If KMP_HW_SUBSET wasn't requested, then do nothing.
   if (!__kmp_hw_subset)
     return false;
 
+  // First, sort the KMP_HW_SUBSET items by the machine topology
+  __kmp_hw_subset->sort();
+
   // Check to see if KMP_HW_SUBSET is a valid subset of the detected topology
   int hw_subset_depth = __kmp_hw_subset->get_depth();
   kmp_hw_t specified[KMP_HW_LAST];
   KMP_ASSERT(hw_subset_depth > 0);
   KMP_FOREACH_HW_TYPE(i) { specified[i] = KMP_HW_UNKNOWN; }
   for (int i = 0; i < hw_subset_depth; ++i) {
     int max_count;
     int num = __kmp_hw_subset->at(i).num;
for (int i = 0; i < hw_subset_depth; ++i) {
     // either directly or through an equivalent type
     if (specified[equivalent_type] != KMP_HW_UNKNOWN) {
       KMP_WARNING(AffHWSubsetEqvLayers, __kmp_hw_get_catalog_string(type),
                   __kmp_hw_get_catalog_string(specified[equivalent_type]));
       return false;
     }
     specified[equivalent_type] = type;
 
-    // Check to see if layers are in order
-    if (i + 1 < hw_subset_depth) {
-      kmp_hw_t next_type = get_equivalent_type(__kmp_hw_subset->at(i + 1).type);
-      if (next_type == KMP_HW_UNKNOWN) {
-        KMP_WARNING(
-            AffHWSubsetNotExistGeneric,
-            __kmp_hw_get_catalog_string(__kmp_hw_subset->at(i + 1).type));
-        return false;
-      }
-      int next_topology_level = get_level(next_type);
-      if (level > next_topology_level) {
-        KMP_WARNING(AffHWSubsetOutOfOrder, __kmp_hw_get_catalog_string(type),
-                    __kmp_hw_get_catalog_string(next_type));
-        return false;
-      }
-    }
-
     // Check to see if each layer's num & offset parameters are valid
     max_count = get_ratio(level);
     if (max_count < 0 || num + offset > max_count) {
       bool plural = (num > 1);
       KMP_WARNING(AffHWSubsetManyGeneric,
                   __kmp_hw_get_catalog_string(type, plural));
       return false;
     }

openmp/runtime/test/affinity/kmp-hw-subset.c

 // RUN: %libomp-compile -D_GNU_SOURCE
 // RUN: env OMP_PLACES=threads %libomp-run
 // RUN: env OMP_PLACES=cores %libomp-run
 // RUN: env OMP_PLACES=sockets %libomp-run
+// RUN: env OMP_PLACES=cores RUN_OUT_OF_ORDER=1 %libomp-run
 // REQUIRES: linux
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include "libomp_test_affinity.h"
 #include "libomp_test_topology.h"
 
static int check_places() {
   // Use half the resources of every level
   if (nthreads_per_core > 1)
     nthreads_per_core /= 2;
   if (ncores_per_socket > 1)
     ncores_per_socket /= 2;
   if (nsockets > 1)
     nsockets /= 2;
 
+  if (getenv("RUN_OUT_OF_ORDER")) {
+    snprintf(buf, sizeof(buf), "%dt,%ds,%dc", nthreads_per_core, nsockets,
+             ncores_per_socket);
+  } else {
     snprintf(buf, sizeof(buf), "%ds,%dc,%dt", nsockets, ncores_per_socket,
              nthreads_per_core);
+  }
   setenv("KMP_HW_SUBSET", buf, 1);
 
   openmp_places = topology_alloc_openmp_places();
   status = compare_hw_subset_places(openmp_places, type, nsockets,
                                     ncores_per_socket, nthreads_per_core);
   topology_free_places(threads);
   topology_free_places(cores);
   topology_free_places(sockets);