[Openmp-commits] [PATCH] D132005: [OpenMP] Add non-blocking support for target nowait regions

Mon Oct 17 17:09:32 PDT 2022

tianshilei1992 added inline comments.

================
Comment at: openmp/runtime/src/kmp_tasking.cpp:5157
+  void *handle = NULL;
+  kmp_info_t *thread = __kmp_thread_from_gtid(gtid);
+  kmp_taskdata_t *taskdata = thread->th.th_current_task;
----------------
gValarini wrote:
> gValarini wrote:
> > tianshilei1992 wrote:
> > > `libomptarget` (for now) doesn't require the thread must be an OpenMP thread. Using `libomp`'s gtid generally breaks. Either we add the requirement, which needs to be discussed further, or there it needs an alternative method to implement that. If `libomp` is executed on another fresh thread, a new root will be created.
> > Uhm, I did not know about that. Although I think such a requirement makes sense, it may be out of the scope of this patch. 
> > 
> > What we could do is check if the current thread is registered inside `libomp` somehow, falling back to the current execution path that does not depend on the task team information. Do you know we can use `__kmpc_global_thread_num`'s return value to verify that? Maybe assert that returned `GTID` is valid and within a well-known range (e.g., [0, NUM_REGISTERED_OMP_THREADS]).
> > 
> > Just a note, NUM_REGISTERED_OMP_THREADS is not a valid variable. I just don't know where, or even if, such information is stored. Do you know where can I find this?
> @tianshilei1992 any comments on this?
IIRC some return values, except those real thread ids, are for specific purposes. That's one of the reason that I didn't use negative thread id for hidden helper thread. I don't know for sure if there is a value designed to say it is not an OpenMP managed thread. We can probably add one.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D132005