[Openmp-commits] [PATCH] D107316: [OpenMP] Add missing `tt_hidden_helper_task_encountered` along with `tt_found_proxy_tasks`

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 marked an inline comment as done.
tianshilei1992 added inline comments.


================
Comment at: openmp/runtime/src/kmp_tasking.cpp:3080
     if (nthreads == 1 &&
-        KMP_ATOMIC_LD_ACQ(&current_task->td_incomplete_child_tasks))
+        KMP_ATOMIC_LD_ACQ(&current_task->td_incomplete_child_tasks) > 1)
       use_own_tasks = 1;
----------------
AndreyChurbanov wrote:
> tianshilei1992 wrote:
> > That is the reason that we can observe hangs in the past. Basically, D107496 changed the way to track incomplete children. Once we encountered a hidden helper task or detachable task, all following tasks will be tracked. In the simple case we added to this patch, we have a regular target region depending on a hidden helper task. The allocation of the regular target region increments the counter of incomplete children. Since it is a regular target region with dependences, clang lowers it to a function call to `__kmpc_omp_wait_deps` first followed by invoking the task directly. At this moment, `current_task->td_incomplete_child_tasks` is either 2 or 1, depending on whether the hidden helper task is finished, but it will never be 0. Therefore, we are trapped into the infinite loop here.
> > I simply change it to check if the `td_incomplete_child_tasks` is greater than 1 which can exclude itself. Since we don't know if the function call to `__kmpc_omp_wait_deps` is for `taskwait` or for a `if0` task, I don't have any other good idea on the top of my mind to check here. We might have very tiny performance loss, caused by returning from this function and then entering it again, for the fairly rare case.
> I think the correct fix here would be addition of condition `&& !task_team->tt.tt_hidden_helper_task_encountered` instead of changing the condition on `current_task->td_incomplete_child_tasks`.  That should cause the thread to exit task execution loop only in the presence of a hidden helper task to check if it still needs to wait for this task or can proceed. In other cases the thread may need to execute tasks from its own queue without exiting the loop.
I fixed it in another way that is to add another check of the flag before set `use_own_tasks`. We do have two similar checks in different code path, but in the test case added to this patch, existing check cannot cover.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107316