[Openmp-commits] [PATCH] D107121: [OpenMP] Fix performance regression reported in bug #51235

[Joachim Protze via Phabricator via Openmp-commits]

protze.joachim added a comment.

I still get the same assertions.

It looks like the last task is not guaranteed to finish before the taskwait. The taskwait function spins on `td_incomplete_child_tasks` to become 0. Is it possible, that this variable is not properly maintained for hidden helper tasks?

This is how I tried to refine the tests to see, what is going wrong:

  diff --git a/openmp/runtime/test/tasking/hidden_helper_task/depend.cpp b/openmp/runtime/test/tasking/hidden_helper_task/depend.cpp
  index 4bc27c1..a6e8cbb 100644
  --- a/openmp/runtime/test/tasking/hidden_helper_task/depend.cpp
  +++ b/openmp/runtime/test/tasking/hidden_helper_task/depend.cpp
  @@ -44,6 +44,10 @@ template <int I>
   kmp_int32 omp_task_entry(kmp_int32 gtid, kmp_task_t_with_privates *task) {
     auto shareds = reinterpret_cast<anon *>(task->task.shareds);
     auto p = shareds->data;
  +  auto v = *p;
  +  if(v!=I-1)
  +    printf("Fail: %i expect %i\n", v, I-1);
  +  assert(v == I-1);
     *p += I;
     return 0;
   }
  @@ -118,6 +122,8 @@ int main(int argc, char *argv[]) {
       // Wait for all tasks
       __kmpc_omp_taskwait(nullptr, gtid);
   
  +    if(data!=15)
  +      printf("Fail: %i\n",data);
       assert(data == 15);
     }
   
  diff --git a/openmp/runtime/test/tasking/hidden_helper_task/gtid.cpp b/openmp/runtime/test/tasking/hidden_helper_task/gtid.cpp
  index 8cec95b..8cab6c9 100644
  --- a/openmp/runtime/test/tasking/hidden_helper_task/gtid.cpp
  +++ b/openmp/runtime/test/tasking/hidden_helper_task/gtid.cpp
  @@ -50,11 +50,15 @@ kmp_int32 omp_task_entry(kmp_int32 gtid, kmp_task_t_with_privates *task) {
     return 0;
   }
   
  -template <bool hidden_helper_task> void assert_gtid(int v) {
  +template <bool hidden_helper_task> void assert_gtid(int v, int I) {
     if (__kmp_hidden_helper_threads_num) {
       if (hidden_helper_task) {
  +      if(!(v > 0 && v <= __kmp_hidden_helper_threads_num))
  +        printf("Fail(%i, %i): v = %i\n", hidden_helper_task, I, v);
         assert(v > 0 && v <= __kmp_hidden_helper_threads_num);
       } else {
  +      if(!(v == 0 || v > __kmp_hidden_helper_threads_num))
  +        printf("Fail(%i, %i): v = %i\n", hidden_helper_task, I, v);
         assert(v == 0 || v > __kmp_hidden_helper_threads_num);
       }
     } else {
  @@ -68,7 +72,7 @@ int main(int argc, char *argv[]) {
     constexpr const int N = 1024;
   #pragma omp parallel for
     for (int i = 0; i < N; ++i) {
  -    int32_t data1 = -1, data2 = -1, data3 = -1;
  +    int32_t data1 = -4, data2 = -4, data3 = -4;
       int depvar;
       int32_t gtid = __kmpc_global_thread_num(nullptr);
   
  @@ -117,9 +121,9 @@ int main(int argc, char *argv[]) {
       __kmpc_omp_taskwait(nullptr, gtid);
   
       // FIXME: 8 here is not accurate
  -    assert_gtid<false>(data1);
  -    assert_gtid<true>(data2);
  -    assert_gtid<false>(data3);
  +    assert_gtid<false>(data1, 1);
  +    assert_gtid<true>(data2, 2);
  +    assert_gtid<false>(data3, 3);
     }
   
     std::cout << "PASS\n";
   


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107121