[Openmp-commits] [PATCH] D98838: [OpenMP] Fixed a crash in hidden helper thread

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 created this revision.
tianshilei1992 added reviewers: ronlieb, protze.joachim, lebedev.ri, JonChesterfield, jdoerfert.
Herald added subscribers: guansong, yaxunl.
tianshilei1992 requested review of this revision.
Herald added subscribers: openmp-commits, sstefan1.
Herald added a project: OpenMP.

It is reported that after enabling hidden helper thread, the program
can hit the assertion `new_gtid < __kmp_threads_capacity` sometimes. The root
cause is explained as follows. Let's say the default `__kmp_threads_capacity` is
`N`. If hidden helper thread is enabled, `__kmp_threads_capacity` will be offset
to `N+8` by default. If the number of threads we need exceeds `N+8`, e.g. via
`num_threads` clause, we need to expand `__kmp_threads`. In
`__kmp_expand_threads`, the expansion starts from `__kmp_threads_capacity`, and
repeatedly doubling it until the new capacity meets the requirement. Let's
assume the new requirement is `Y`.  If `Y` happens to meet the constraint
`(N+8)*2^X=Y` where `X` is the number of iterations, the new capacity is not
enough because we have 8 slots for hidden helper threads.

Here is an example.

  #include <vector>
  
  int main(int argc, char *argv[]) {
    constexpr const size_t N = 1344;
    std::vector<int> data(N);
  
  #pragma omp parallel for
    for (unsigned i = 0; i < N; ++i) {
      data[i] = i;
    }
  
  #pragma omp parallel for num_threads(N)
    for (unsigned i = 0; i < N; ++i) {
      data[i] += i;
    }
  
    return 0;
  }

My CPU is 20C40T, then `__kmp_threads_capacity` is 160. After offset,
`__kmp_threads_capacity` becomes 168. `1344 = (160+8)*2^3`, then the assertions
hit.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98838

Files:
  openmp/runtime/src/kmp_runtime.cpp
  openmp/runtime/src/kmp_settings.cpp
  openmp/runtime/test/tasking/hidden_helper_task/num_threads.cpp


Index: openmp/runtime/test/tasking/hidden_helper_task/num_threads.cpp
===================================================================
--- /dev/null
+++ openmp/runtime/test/tasking/hidden_helper_task/num_threads.cpp
@@ -0,0 +1,35 @@
+// RUN: %libomp-cxx-compile-and-run
+
+#include <omp.h>
+
+#include <algorithm>
+#include <cassert>
+#include <vector>
+
+int main(int argc, char *argv[]) {
+  constexpr const int __kmp_hidden_helper_threads_num = 8;
+  const int __kmp_threads_capacity =
+      std::min(std::max(std::max(32, 4 * omp_get_num_threads()),
+                        4 * omp_get_num_procs()),
+               std::numeric_limits<int>::max());
+  const int capacity = __kmp_threads_capacity + __kmp_hidden_helper_threads_num;
+  const int N = 2 * capacity;
+
+  std::vector<int> data(N);
+
+#pragma omp parallel for
+  for (unsigned i = 0; i < N; ++i) {
+    data[i] = i;
+  }
+
+#pragma omp parallel for num_threads(N)
+  for (unsigned i = 0; i < N; ++i) {
+    data[i] += i;
+  }
+
+  for (unsigned i = 0; i < N; ++i) {
+    assert(data[i] == 2 * i);
+  }
+
+  return 0;
+}
Index: openmp/runtime/src/kmp_settings.cpp
===================================================================
--- openmp/runtime/src/kmp_settings.cpp
+++ openmp/runtime/src/kmp_settings.cpp
@@ -504,9 +504,10 @@
     nth = (4 * __kmp_xproc);
 
   // If hidden helper task is enabled, we initialize the thread capacity with
-  // extra
-  // __kmp_hidden_helper_threads_num.
-  nth += __kmp_hidden_helper_threads_num;
+  // extra __kmp_hidden_helper_threads_num.
+  if (__kmp_enable_hidden_helper) {
+    nth += __kmp_hidden_helper_threads_num;
+  }
 
   if (nth > __kmp_max_nth)
     nth = __kmp_max_nth;
Index: openmp/runtime/src/kmp_runtime.cpp
===================================================================
--- openmp/runtime/src/kmp_runtime.cpp
+++ openmp/runtime/src/kmp_runtime.cpp
@@ -3547,6 +3547,10 @@
     newCapacity = newCapacity <= (__kmp_sys_max_nth >> 1) ? (newCapacity << 1)
                                                           : __kmp_sys_max_nth;
   } while (newCapacity < minimumRequiredCapacity);
+  // If hidden helper thread is enabled, we also need to count the number
+  if (__kmp_enable_hidden_helper) {
+    newCapacity += __kmp_hidden_helper_threads_num;
+  }
   newThreads = (kmp_info_t **)__kmp_allocate(
       (sizeof(kmp_info_t *) + sizeof(kmp_root_t *)) * newCapacity + CACHE_LINE);
   newRoot =


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98838.331440.patch
Type: text/x-patch
Size: 2431 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20210318/87b63d8d/attachment.bin>