[Openmp-commits] [PATCH] D107656: [OpenMP] Use events and taskyield in target nowait task to unblock host threads

[Shilei Tian via Phabricator via Openmp-commits]

tianshilei1992 added inline comments.


================
Comment at: openmp/libomptarget/plugins/cuda/src/rtl.cpp:1189
+    }
+    Err = cuEventDestroy(Event);
+    if (Err != CUDA_SUCCESS) {
----------------
Event destroy worths a separate function. We could add a new return value such as `OFFLOAD_NOT_DONE` to indicate the event is not fulfilled. It is not a good idea to mix event query and event destroy.


================
Comment at: openmp/libomptarget/src/device.cpp:545
+    return RTL->record_event(RTLDeviceID, AsyncInfo);
+  } else {
+    AsyncInfo.setEventSupported(false);
----------------
early return


================
Comment at: openmp/libomptarget/src/omptarget.cpp:45
+        DP("No event support by the pluggin! Calling synchronize\n");
+        Result = Device.synchronize(*this);
+      } else {
----------------
use early return


================
Comment at: openmp/libomptarget/src/omptarget.cpp:51
+          Ret = Device.queryEvent(*this);
+        } while (Ret == OFFLOAD_SUCCESS && AsyncInfo.Event);
+
----------------
I'm thinking we can actually do more here. For example, set a count for every task yield. When the count reaches a threshold, fall back to stream synchronize. The threshold can be configured via env and so on.


================
Comment at: openmp/runtime/src/kmp_tasking.cpp:1443
+  // target task is untied defined in the specification
+  input_flags.tiedness = TASK_UNTIED;
   if (__kmp_enable_hidden_helper) {
----------------
This change worths a separate patch.


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

https://reviews.llvm.org/D107656