[OpenMP][libomptarget] Add event query plugin API

[ye-luo]

Summary

Introduce an plugin interface to query the status of an event.
Also record whether a device support events or not.

[ye-luo]

openmp/libomptarget/include/omptarget.h

184

I'm not sure about the naming convention for this enum type name.
Also if this is the right location to add this enum.

[jdoerfert]

What is the use case of the API and of the new member?

[ye-luo]

In D109017#2975462, @jdoerfert wrote:

What is the use case of the API and of the new member?

The new member is needed for D107656 to opt-in using events for synchronization if event is supported.
D107656 also need query_event to decide if a task should be resumed.

[jdoerfert]

I think we need something like this for sure soon. @tianshilei1992 thoughts?

[tianshilei1992]

openmp/libomptarget/plugins/cuda/src/rtl.cpp

162

IMO, it's better to extend the return value, aka OFFLOAD_SUCCESS, instead of using this method. Currently we have two status, 0, and ~0. We can add plenty of status.

[ye-luo]

openmp/libomptarget/plugins/cuda/src/rtl.cpp

162

Add D109277 to warmup and detect potential issues.

[ye-luo]

Expand macro for the moment.

[ye-luo]

openmp/libomptarget/plugins/cuda/src/rtl.cpp

162

Change macro to enum takes a bit longer. So just keep using macro for the moment.

[tianshilei1992]

openmp/libomptarget/include/omptargetplugin.h

155

Actually, I don't see query_event is mandatory here. We don't have to query it before using it. From my understanding, query_event is more like an option that we may want to do something if the event is still not fulfilled.

openmp/libomptarget/src/device.cpp

25

HasEventSupport is not necessary here. If any event related interface is nullptr, it is not supported, and we will not call it. There are basically two options to implement them.

• Set a Boolean variable when DeviceTy is initialized. Every time we call an underlying RTLInfoTy function, we check the Boolean variable. That's what you proposed here.
• Every time we call an underlying RTLInfoTy function, we check if the function pointer is nulltpr. That's we currently used.

I didn't see any issue from what we are using. As long as the behavior is consistent, they are same. Setting a Boolean variable will not save any branch instruction.

[ye-luo]

openmp/libomptarget/include/omptargetplugin.h

155

query_event is not mandatory nor sync_event I guess. You can choose one way or another.
In some cases if we use wait_event, neither query_event nor sync_event may be needed.
What give you the impression that query_event is mandatory? Do I need to change something here to make things more clear?

openmp/libomptarget/src/device.cpp

25

Currently when event is not supported, event APIs are still called and get OFFLOAD_SUCCESS. I find it hard to follow the code for example in D104418.
So prefer a boolean to skip large chunk of calls to any event related APIs. For example in D107656, I can directly decide to use event or queue.

[tianshilei1992]

openmp/libomptarget/include/omptargetplugin.h

155

query_event is not mandatory nor sync_event I guess. You can choose one way or another.

IIRC, you mentioned somewhere previously that, stream synchronization is just spinning, which really hurts performance. If sync_event is something like conditional variable, we can use it to replace stream synchronization. That's what I expected when I wrote those lines. If it is not the case, we could add "(optional)" or add "if needed" or something like that.

openmp/libomptarget/src/device.cpp

25

Currently when event is not supported, event APIs are still called and get OFFLOAD_SUCCESS. I find it hard to follow the code for example in D104418.

Returning OFFLOAD_SUCCESS is like a convention we are using in libomptarget. When an optional feature (basically a function pointer) is not supported, it should behave like it doesn't break anything by returning OFFLOAD_SUCCESS. Well, of course we could use a Boolean variable for each feature, but then we will have a bunch of Boolean variables, which is unnecessary.
Sometime ago I also thought to query what features a device (plugin) can support, and store them in bit set. In this way, when user code has require, we know what feature is supported and which is not immediately. From my perspective, that's the right way to do that.
So back to your code, you could still check if the corresponding function pointer is nullptr. All data members are public, so there is nothing stopping you.

[ye-luo]

openmp/libomptarget/include/omptargetplugin.h

155

When using event API to achieve synchronization, libomptarget can choose query_event or sync_event depending on the use case. or event call __tgt_rtl_wait_event and sync the stream.

When implementing event related APIs in a plugin, all APIs are required. No holes should be left. If we really have to make certain API optional. Then some fine-grain flags are need to indicate a specific API not supported. I think we can do such change when a need shows up.

openmp/libomptarget/src/device.cpp

25

Returning OFFLOAD_SUCCESS is not the issue. The issue is what is the expected behavior of these APIs when events are not supported. right now it sounds like undefined behaviors and thus I'd like to skip calling them. Codes are also much more readable.

> So back to your code, you could still check if the corresponding function pointer is nullptr. All data members are public, so there is nothing stopping you.
I feel this worse than a boolean. All function pointer member being public is already fishy. Since the change of unique_ptr, I can actually mark the boolean const and it is set by the constructor only.

Using bitset or boolean can be done separately. using "requires" can be a separate topic. right now, I think we can just handle the scenario of missing event support of plugins inside lbiomptarget.

[tianshilei1992]

openmp/libomptarget/include/omptargetplugin.h

155

I mean, the "optional" should be like, the function is not mandatory to be called for the completeness of the whole event mechanism. For example, we could just create event and set dependence on it. We don't have to synchronize it or query it.

openmp/libomptarget/src/device.cpp

25

Returning OFFLOAD_SUCCESS is not the issue. The issue is what is the expected behavior of these APIs when events are not supported. right now it sounds like undefined behaviors and thus I'd like to skip calling them. Codes are also much more readable.

Like I said before, the reason the optional features (interfaces) are called optional is that even if they are not there, they should be transparent and not break anything. It is defined behavior. What you are trying to do is to introduce inconsistency: for some of other optional APIs, when they are not supported, it returns success. For this specific set of APIs, when they are not supported, they are not called.

I feel this worse than a boolean. All function pointer member being public is already fishy. Since the change of unique_ptr, I can actually mark the boolean const and it is set by the constructor only.

It's as good (or bad) as using a Boolean variable.

Using bitset or boolean can be done separately. using "requires" can be a separate topic. right now, I think we can just handle the scenario of missing event support of plugins inside lbiomptarget.

I don't doubt that. I just expect to have the feature enabled, and also work like existing code.

openmp/libomptarget/include/omptarget.h

 
 #include <deque>
 #include <stddef.h>
 #include <stdint.h>
 
 #include <SourceInfo.h>
 
 #define OFFLOAD_SUCCESS (0)
+#define OFFLOAD_EVENT_NOT_READY 600
 #define OFFLOAD_FAIL (~0)
 
 #define OFFLOAD_DEVICE_DEFAULT -1
 
 // Don't format out enums and structs.
 // clang-format off
 
 /// Data attributes for each data reference used in an OpenMP target region.

 
 /// This struct is a record of non-contiguous information
 struct __tgt_target_non_contig {
   uint64_t Offset;
   uint64_t Count;
   uint64_t Stride;
 };
 
 #ifdef __cplusplus
 extern "C" {
 #endif
 
 int omp_get_num_devices(void);
 int omp_get_initial_device(void);
 void *omp_target_alloc(size_t size, int device_num);
 void omp_target_free(void *device_ptr, int device_num);
 int omp_target_is_present(const void *ptr, int device_num);

openmp/libomptarget/include/omptargetplugin.h

 void __tgt_rtl_print_device_info(int32_t ID);
 
 // Event related interfaces. It is expected to use the interfaces in the
 // following way:
 // 1) Create an event on the target device (__tgt_rtl_create_event).
 // 2) Record the event based on the status of \p AsyncInfo->Queue at the moment
 // of function call to __tgt_rtl_record_event. An event becomes "meaningful"
 // once it is recorded, such that others can depend on it.
-// 3) Call __tgt_rtl_wait_event to set dependence on the event. Whether the
+// 3) Call __tgt_rtl_query_event to query the status of all work currently
+// captured by an event
+// 4) Call __tgt_rtl_wait_event to set dependence on the event. Whether the
 // operation is blocking or non-blocking depends on the target. It is expected
 // to be non-blocking, just set dependence and return.
-// 4) Call __tgt_rtl_sync_event to sync the event. It is expected to block the
+// 5) Call __tgt_rtl_sync_event to sync the event. It is expected to block the
 // thread calling the function.
-// 5) Destroy the event (__tgt_rtl_destroy_event).
+// 6) Destroy the event (__tgt_rtl_destroy_event).
 // {
 int32_t __tgt_rtl_create_event(int32_t ID, void **Event);
 
 int32_t __tgt_rtl_record_event(int32_t ID, void *Event,
                                __tgt_async_info *AsyncInfo);
 
+int32_t __tgt_rtl_query_event(int32_t ID, void *Event);
+
 int32_t __tgt_rtl_wait_event(int32_t ID, void *Event,
                              __tgt_async_info *AsyncInfo);
 
 int32_t __tgt_rtl_sync_event(int32_t ID, void *Event);
 
 int32_t __tgt_rtl_destroy_event(int32_t ID, void *Event);
 // }
 
 #ifdef __cplusplus
 }
 #endif
 
 #endif // _OMPTARGETPLUGIN_H_

openmp/libomptarget/plugins/cuda/dynamic_cuda/cuda.h

 typedef struct CUfunc_st *CUfunction;
 typedef struct CUstream_st *CUstream;
 typedef struct CUevent_st *CUevent;
 
 typedef enum cudaError_enum {
   CUDA_SUCCESS = 0,
   CUDA_ERROR_INVALID_VALUE = 1,
   CUDA_ERROR_INVALID_HANDLE = 400,
+  CUDA_ERROR_NOT_READY = 600,
 } CUresult;
 
 typedef enum CUstream_flags_enum {
   CU_STREAM_DEFAULT = 0x0,
   CU_STREAM_NON_BLOCKING = 0x1,
 } CUstream_flags;
 
 typedef enum CUlimit_enum {

 CUresult cuMemcpyPeerAsync(CUdeviceptr, CUcontext, CUdeviceptr, CUcontext,
                            size_t, CUstream);
 
 CUresult cuCtxGetLimit(size_t *, CUlimit);
 CUresult cuCtxSetLimit(CUlimit, size_t);
 
 CUresult cuEventCreate(CUevent *, unsigned int);
 CUresult cuEventRecord(CUevent, CUstream);
+CUresult cuEventQuery(CUevent);
 CUresult cuStreamWaitEvent(CUstream, CUevent, unsigned int);
 CUresult cuEventSynchronize(CUevent);
 CUresult cuEventDestroy(CUevent);
 
 #endif

openmp/libomptarget/plugins/cuda/dynamic_cuda/cuda.cpp

 //===--- cuda/dynamic_cuda/cuda.pp ------------------------------- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Implement subset of cuda api by calling into cuda library via dlopen
 // Does the dlopen/dlsym calls as part of the call to cuInit
 //
 //===----------------------------------------------------------------------===//
 
 #include "cuda.h"
 #include "Debug.h"
 #include "dlwrap.h"
 
 #include <string>
 #include <unordered_map>
 
 #include <dlfcn.h>
 
 DLWRAP_INTERNAL(cuInit, 1);

 DLWRAP(cuCtxEnablePeerAccess, 2);
 DLWRAP(cuMemcpyPeerAsync, 6);
 
 DLWRAP(cuCtxGetLimit, 2);
 DLWRAP(cuCtxSetLimit, 2);
 
 DLWRAP(cuEventCreate, 2);
 DLWRAP(cuEventRecord, 2);
+DLWRAP(cuEventQuery, 1);
 DLWRAP(cuStreamWaitEvent, 3);
 DLWRAP(cuEventSynchronize, 1);
 DLWRAP(cuEventDestroy, 1);
 
 DLWRAP_FINALIZE();
 
 #ifndef DYNAMIC_CUDA_PATH
 #define DYNAMIC_CUDA_PATH "libcuda.so"

openmp/libomptarget/plugins/cuda/src/rtl.cpp

 //===----RTLs/cuda/src/rtl.cpp - Target RTLs Implementation ------- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // RTL for CUDA machine
 //
 //===----------------------------------------------------------------------===//
 
 #include <cassert>
 #include <cstddef>
 #include <cuda.h>
 #include <list>
 #include <memory>
 #include <mutex>
 #include <string>
 #include <unordered_map>
 #include <vector>
 
 #include "Debug.h"
DP("Error when recording event. stream = " DPxMOD ", event = " DPxMOD "\n",
        DPxPTR(Stream), DPxPTR(Event));
     CUDA_ERR_STRING(Err);
     return OFFLOAD_FAIL;
   }
 
   return OFFLOAD_SUCCESS;
 }
 
+int queryEvent(void *EventPtr) {
+  CUevent Event = reinterpret_cast<CUevent>(EventPtr);
+  CUresult Err = cuEventQuery(Event);
+
+  if (Err == CUDA_ERROR_NOT_READY) {
+    DP("Captured work is incomplete. Event = " DPxMOD "\n", DPxPTR(Event));
+    return OFFLOAD_EVENT_NOT_READY;
+  }
+
+  if (Err != CUDA_SUCCESS) {
+    DP("Error when querying an event. Event = " DPxMOD "\n", DPxPTR(Event));
+    CUDA_ERR_STRING(Err);
+    return OFFLOAD_FAIL;
+  }
+
+  return OFFLOAD_SUCCESS;
+}
+
 int syncEvent(void *EventPtr) {
   CUevent Event = reinterpret_cast<CUevent>(EventPtr);
 
   CUresult Err = cuEventSynchronize(Event);
   if (Err != CUDA_SUCCESS) {
     DP("Error when syncing event = " DPxMOD "\n", DPxPTR(Event));
     CUDA_ERR_STRING(Err);
     return OFFLOAD_FAIL;
int32_t __tgt_rtl_create_event(int32_t device_id, void **event) {
   assert(event && "event is nullptr");
   return createEvent(event);
 }
 
 int32_t __tgt_rtl_record_event(int32_t device_id, void *event_ptr,
                                __tgt_async_info *async_info_ptr) {
   assert(async_info_ptr && "async_info_ptr is nullptr");
   assert(async_info_ptr->Queue && "async_info_ptr->Queue is nullptr");
-  assert(event_ptr && "event_ptr is nullptr");
+  assert(event_ptr && "event is nullptr");
 
   return recordEvent(event_ptr, async_info_ptr);
 }
 
+int32_t __tgt_rtl_query_event(int32_t device_id, void *event_ptr) {
+  assert(event_ptr && "event is nullptr");
+  return queryEvent(event_ptr);
+}
+
 int32_t __tgt_rtl_wait_event(int32_t device_id, void *event_ptr,
                              __tgt_async_info *async_info_ptr) {
   assert(DeviceRTL.isValidDeviceId(device_id) && "device_id is invalid");
   assert(async_info_ptr && "async_info_ptr is nullptr");
   assert(event_ptr && "event is nullptr");
 
   return DeviceRTL.waitEvent(device_id, async_info_ptr, event_ptr);
 }

openmp/libomptarget/plugins/exports

global:
     __tgt_rtl_synchronize;
     __tgt_rtl_register_lib;
     __tgt_rtl_unregister_lib;
     __tgt_rtl_supports_empty_images;
     __tgt_rtl_set_info_flag;
     __tgt_rtl_print_device_info;
     __tgt_rtl_create_event;
     __tgt_rtl_record_event;
+    __tgt_rtl_query_event;
     __tgt_rtl_wait_event;
     __tgt_rtl_sync_event;
     __tgt_rtl_destroy_event;
   local:
     *;
 };

openmp/libomptarget/src/device.h

 struct DeviceTy {
   int32_t DeviceID;
   RTLInfoTy *RTL;
   int32_t RTLDeviceID;
 
   bool IsInit;
   std::once_flag InitFlag;
   bool HasPendingGlobals;
+  bool HasEventSupport;
 
   HostDataToTargetListTy HostDataToTargetMap;
   PendingCtorsDtorsPerLibrary PendingCtorsDtors;
 
   ShadowPtrListTy ShadowPtrMap;
 
   std::mutex DataMapMtx, PendingGlobalsMtx, ShadowMtx;
 
struct DeviceTy {
   /// device RTL to obtain the information of the specific device.
   bool printDeviceInfo(int32_t RTLDevID);
 
   /// Event related interfaces.
   /// {
   /// Create an event.
   int32_t createEvent(void **Event);
 
+  /// query the status of an event.
+  int32_t queryEvent(void *Event);
+
   /// Record the event based on status in AsyncInfo->Queue at the moment the
   /// function is called.
   int32_t recordEvent(void *Event, AsyncInfoTy &AsyncInfo);
 
   /// Wait for an event. This function can be blocking or non-blocking,
   /// depending on the implmentation. It is expected to set a dependence on the
   /// event such that corresponding operations shall only start once the event
   /// is fulfilled.

openmp/libomptarget/src/device.cpp

 
 #include <cassert>
 #include <climits>
 #include <cstdio>
 #include <string>
 
 DeviceTy::DeviceTy(RTLInfoTy *RTL)
     : DeviceID(-1), RTL(RTL), RTLDeviceID(-1), IsInit(false), InitFlag(),
-      HasPendingGlobals(false), HostDataToTargetMap(), PendingCtorsDtors(),
-      ShadowPtrMap(), DataMapMtx(), PendingGlobalsMtx(), ShadowMtx() {}
+      HasPendingGlobals(false),
+      HasEventSupport(RTL->create_event ? true : false), HostDataToTargetMap(),
+      PendingCtorsDtors(), ShadowPtrMap(), DataMapMtx(), PendingGlobalsMtx(),
+      ShadowMtx() {}
 
 DeviceTy::~DeviceTy() {
   if (DeviceID == -1 || !(getInfoLevel() & OMP_INFOTYPE_DUMP_TABLE))
     return;
 
   ident_t loc = {0, 0, 0, 0, ";libomptarget;libomptarget;0;0;;"};
   dumpTargetPointerMappings(&loc, *this);
 }

 
 int32_t DeviceTy::recordEvent(void *Event, AsyncInfoTy &AsyncInfo) {
   if (RTL->record_event)
     return RTL->record_event(RTLDeviceID, Event, AsyncInfo);
 
   return OFFLOAD_SUCCESS;
 }
 
+int32_t DeviceTy::queryEvent(void *Event) {
+  if (RTL->query_event)
+    return RTL->query_event(RTLDeviceID, Event);
+
+  return OFFLOAD_SUCCESS;
+}
+
 int32_t DeviceTy::waitEvent(void *Event, AsyncInfoTy &AsyncInfo) {
   if (RTL->wait_event)
     return RTL->wait_event(RTLDeviceID, Event, AsyncInfo);
 
   return OFFLOAD_SUCCESS;
 }
 
 int32_t DeviceTy::syncEvent(void *Event) {

openmp/libomptarget/src/rtl.h

struct RTLInfoTy {
   typedef int64_t(init_requires_ty)(int64_t);
   typedef int32_t(synchronize_ty)(int32_t, __tgt_async_info *);
   typedef int32_t (*register_lib_ty)(__tgt_bin_desc *);
   typedef int32_t(supports_empty_images_ty)();
   typedef void(print_device_info_ty)(int32_t);
   typedef void(set_info_flag_ty)(uint32_t);
   typedef int32_t(create_event_ty)(int32_t, void **);
   typedef int32_t(record_event_ty)(int32_t, void *, __tgt_async_info *);
+  typedef int32_t(query_event_ty)(int32_t, void *);
   typedef int32_t(wait_event_ty)(int32_t, void *, __tgt_async_info *);
   typedef int32_t(sync_event_ty)(int32_t, void *);
   typedef int32_t(destroy_event_ty)(int32_t, void *);
 
   int32_t Idx = -1;             // RTL index, index is the number of devices
                                 // of other RTLs that were registered before,
                                 // i.e. the OpenMP index of the first device
                                 // to be registered with this RTL.
#endif
   synchronize_ty *synchronize = nullptr;
   register_lib_ty register_lib = nullptr;
   register_lib_ty unregister_lib = nullptr;
   supports_empty_images_ty *supports_empty_images = nullptr;
   set_info_flag_ty *set_info_flag = nullptr;
   print_device_info_ty *print_device_info = nullptr;
   create_event_ty *create_event = nullptr;
   record_event_ty *record_event = nullptr;
+  query_event_ty *query_event = nullptr;
   wait_event_ty *wait_event = nullptr;
   sync_event_ty *sync_event = nullptr;
   destroy_event_ty *destroy_event = nullptr;
 
   // Are there images associated with this RTL.
   bool isUsed = false;
 
   // Mutex for thread-safety when calling RTL interface functions.

openmp/libomptarget/src/rtl.cpp

#endif
     *((void **)&R.set_info_flag) =
         dlsym(dynlib_handle, "__tgt_rtl_set_info_flag");
     *((void **)&R.print_device_info) =
         dlsym(dynlib_handle, "__tgt_rtl_print_device_info");
     *((void **)&R.create_event) =
         dlsym(dynlib_handle, "__tgt_rtl_create_event");
     *((void **)&R.record_event) =
         dlsym(dynlib_handle, "__tgt_rtl_record_event");
+    *((void **)&R.query_event) = dlsym(dynlib_handle, "__tgt_rtl_query_event");
     *((void **)&R.wait_event) = dlsym(dynlib_handle, "__tgt_rtl_wait_event");
     *((void **)&R.sync_event) = dlsym(dynlib_handle, "__tgt_rtl_sync_event");
     *((void **)&R.destroy_event) =
         dlsym(dynlib_handle, "__tgt_rtl_destroy_event");
   }
 
 #if OMPT_SUPPORT
   DP("OMPT_SUPPORT is enabled in libomptarget\n");