[Openmp-commits] [PATCH] D77609: [OpenMP][WIP] Added the support for unshackled task in RTL

[Alex via Phabricator via Openmp-commits]

adurang added inline comments.


================
Comment at: openmp/runtime/src/kmp_tasking.cpp:3771
+  if (task_team)
+    while (KMP_ATOMIC_LD_ACQ(&task_team->tt.tt_unfinished_unshackled_tasks))
+      ;
----------------
ye-luo wrote:
> tianshilei1992 wrote:
> > adurang wrote:
> > > tianshilei1992 wrote:
> > > > tianshilei1992 wrote:
> > > > > adurang wrote:
> > > > > > It seems to me that this is forcing any taskwait (and possibly taskgroup) to wait for any outstanding "async target" that exist on the team irrespectively of being a part of that synchronization domain or not. 
> > > > > Not any async target. Only those created in the team.
> > > > Sorry I still didn't understand this part. Could you please expatiate it?
> > > 
> > > For the code below:
> > > 
> > > ```
> > > #pragma omp parallel num_threads(2)
> > > {
> > > #pragma omp target nowait
> > >    blah()
> > > #pragma omp taskwait
> > > }
> > > ```
> > > 
> > > With your current code (because you're using a shared counter for the whole team), both thread 1 and 2 are waiting for each others target regions ( so for example, even if target-th1 was finished thread1 would be blocked until target-th2 was completed). Each taskwait should only be waiting for their own child target tasks. 
> > > 
> > > Hope this helps.
> > Thanks for the explanation. But this lines of code are in the function `__kmp_task_team_wait` that is not called by `__kmpc_omp_taskwait`. If I understand correctly, `__kmp_task_team_wait` is called by the master thread of a team to wait for all tasks created in the team to finish so that it can proceed. So we need to wait for all unshackled tasks encountered/created in the task team.
> @adurang your example demonstrates exactly the code pattern I use. taskwait should only wait for the child tasks.
Ah sorry, I didn't notice the patch changing functions. I should really look at the whole file!

But if taskwait is working correctly the flag.wait call in __kmp_task_team_wait should also make sure that no outstanding unshackled tasks are left and then I don't think the extra check shouldn't be needed. Do you have any tests with taskgroup/taskwait? 

In any case, could you move it inside the same if statement as the other checks? Also, you need to set tt_unfinished_unshackled_tasks to FALSE in case the same task_team structure is reused (Note that is done the same for various fields just above).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77609