[Openmp-commits] [PATCH] D121056: [OpenMP][NFC] Extend the event introduction to allow wait event actions

[Johannes Doerfert via Phabricator via Openmp-commits]

jdoerfert added inline comments.


================
Comment at: openmp/libomptarget/include/device.h:141
+  /// Returns OFFLOAD_FAIL if something went wrong, OFFLOAD_SUCCESS otherwise.
+  int waitEventIfNecessary(DeviceTy &Device, AsyncInfoTy &AsyncInfo) const;
 
----------------
ye-luo wrote:
> both recordEventIfNecessary and waitEventIfNecessary are confusing names "IfNecessary" part.
I don't understand what the problem is. Could you say what about it is confusing, maybe suggest an alternative wording or at least describe how that could look like?


================
Comment at: openmp/libomptarget/src/device.cpp:46
+  if (!Record && Device.waitEvent(Event, AsyncInfo) != OFFLOAD_SUCCESS) {
+    REPORT("Failed to wait on event " DPxMOD "\n", DPxPTR(Event));
     return OFFLOAD_FAIL;
----------------
ye-luo wrote:
> Cannot take the logic of Record
I do not understand what you mean here either.


================
Comment at: openmp/libomptarget/src/device.cpp:62
+                                             AsyncInfoTy &AsyncInfo) const {
+  return recordOrWaitForEventIfNecessary(*this, Device, AsyncInfo,
+                                         /* Record */ false);
----------------
ye-luo wrote:
> create  an event without recording it once, waitevent is undefined behavior.
Is it? I looked at the CUDA API manual and I did not get that idea. Do you happen to have the passage that says so or is it from experimentation? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121056