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

[Joachim Protze via Phabricator via Openmp-commits]

protze.joachim added inline comments.


================
Comment at: openmp/runtime/src/kmp_runtime.cpp:4323
          ++new_gtid) {
       KMP_DEBUG_ASSERT(new_gtid < __kmp_threads_capacity);
     }
----------------
This assertion triggers for your test application, when the runtime is built in debug mode. 




================
Comment at: openmp/runtime/test/tasking/hidden_helper_task/num_threads.cpp:25
+
+#pragma omp parallel for num_threads(N)
+  for (unsigned i = 0; i < N; ++i) {
----------------
The assertion in kmp_runtime.cpp:4323 triggers also for `num_threads(__kmp_threads_capacity+1)`.

In that case, `__kmp_expand_threads` is never called, so your patch would have no effect at all.

To really fix the issue, all cases which try to calculate the required number of threads and eventually would call `__kmp_expand_threads` need to consider the extra space for the hidden threads, so that they 
  - first realize that space is missing and 
  - second request enough space for the hidden threads. 

With this consideration, `__kmp_expand_threads` does not need any change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98838/new/

https://reviews.llvm.org/D98838