[Openmp-commits] [openmp] 73a6cd2 - [OpenMP][libomptarget] Add minor fixes to NextGen plugins

[Kevin Sala via Openmp-commits]

Author: Kevin Sala
Date: 2022-12-03T22:10:31+01:00
New Revision: 73a6cd23a478c8d71041b25a807499d95249c368

URL: https://github.com/llvm/llvm-project/commit/73a6cd23a478c8d71041b25a807499d95249c368
DIFF: https://github.com/llvm/llvm-project/commit/73a6cd23a478c8d71041b25a807499d95249c368.diff

LOG: [OpenMP][libomptarget] Add minor fixes to NextGen plugins

List of fixes:
  - omptarget_device_environment symbol is not mandatory in device images
  - Do not synchronize in ~AsyncInfoWrapperTy() if the async info's queue is null
  - GenericDeviceResourceRef's create() and destroy() require the device as parameter

Differential Revision: https://reviews.llvm.org/D138619

Added: 
    

Modified: 
    openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
    openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
    openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
index ea27452bb7c76..a69470e864df5 100644
--- a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
@@ -29,7 +29,7 @@ AsyncInfoWrapperTy::~AsyncInfoWrapperTy() {
   // If we used a local async info object we want synchronous behavior.
   // In that case, and assuming the current status code is OK, we will
   // synchronize explicitly when the object is deleted.
-  if (AsyncInfoPtr == &LocalAsyncInfo && !Err)
+  if (AsyncInfoPtr == &LocalAsyncInfo && LocalAsyncInfo.Queue && !Err)
     Err = Device.synchronize(&LocalAsyncInfo);
 }
 
@@ -236,12 +236,17 @@ Error GenericDeviceTy::setupDeviceEnvironment(GenericPluginTy &Plugin,
   DeviceEnvironment.DynamicMemSize = OMPX_SharedMemorySize;
 
   // Create the metainfo of the device environment global.
-  GlobalTy DeviceEnvGlobal("omptarget_device_environment",
-                           sizeof(DeviceEnvironmentTy), &DeviceEnvironment);
+  GlobalTy DevEnvGlobal("omptarget_device_environment",
+                        sizeof(DeviceEnvironmentTy), &DeviceEnvironment);
 
   // Write device environment values to the device.
-  GenericGlobalHandlerTy &GlobalHandler = Plugin.getGlobalHandler();
-  return GlobalHandler.writeGlobalToDevice(*this, Image, DeviceEnvGlobal);
+  GenericGlobalHandlerTy &GHandler = Plugin.getGlobalHandler();
+  if (auto Err = GHandler.writeGlobalToDevice(*this, Image, DevEnvGlobal)) {
+    DP("Missing symbol %s, continue execution anyway.\n",
+       DevEnvGlobal.getName().data());
+    consumeError(std::move(Err));
+  }
+  return Plugin::success();
 }
 
 Error GenericDeviceTy::registerOffloadEntries(DeviceImageTy &Image) {

diff  --git a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
index c5dd7fa70a008..3df2162f72038 100644
--- a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
@@ -626,10 +626,10 @@ class Plugin {
 /// create a new resource on the ctor, but on the create() function instead.
 struct GenericDeviceResourceRef {
   /// Create a new resource and stores a reference.
-  virtual Error create() = 0;
+  virtual Error create(GenericDeviceTy &Device) = 0;
 
   /// Destroy and release the resources pointed by the reference.
-  virtual Error destroy() = 0;
+  virtual Error destroy(GenericDeviceTy &Device) = 0;
 
 protected:
   ~GenericDeviceResourceRef() = default;
@@ -679,6 +679,10 @@ template <typename ResourceRef> class GenericDeviceResourcePoolTy {
   /// Get resource from the pool or create new resources.
   ResourceRef getResource() {
     const std::lock_guard<std::mutex> Lock(Mutex);
+
+    assert(NextAvailable <= ResourcePool.size() &&
+           "Resource pool is corrupted");
+
     if (NextAvailable == ResourcePool.size()) {
       // By default we double the resource pool every time.
       if (auto Err = ResourcePoolTy::resizeResourcePool(NextAvailable * 2)) {
@@ -694,6 +698,8 @@ template <typename ResourceRef> class GenericDeviceResourcePoolTy {
   /// Return resource to the pool.
   void returnResource(ResourceRef Resource) {
     const std::lock_guard<std::mutex> Lock(Mutex);
+
+    assert(NextAvailable > 0 && "Resource pool is corrupted");
     ResourcePool[--NextAvailable] = Resource;
   }
 
@@ -709,13 +715,13 @@ template <typename ResourceRef> class GenericDeviceResourcePoolTy {
     if (OldSize < NewSize) {
       // Create new resources.
       for (uint32_t I = OldSize; I < NewSize; ++I) {
-        if (auto Err = ResourcePool[I].create())
+        if (auto Err = ResourcePool[I].create(Device))
           return Err;
       }
     } else {
       // Destroy the obsolete resources.
       for (uint32_t I = NewSize; I < OldSize; ++I) {
-        if (auto Err = ResourcePool[I].destroy())
+        if (auto Err = ResourcePool[I].destroy(Device))
           return Err;
       }
     }

diff  --git a/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp b/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
index ae1e4b7472346..70ad05f13e609 100644
--- a/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
+++ b/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
@@ -97,7 +97,7 @@ class CUDAStreamRef final : public GenericDeviceResourceRef {
 
   /// Create a new stream and save the reference. The reference must be empty
   /// before calling to this function.
-  Error create() override {
+  Error create(GenericDeviceTy &Device) override {
     if (Stream)
       return Plugin::error("Creating an existing stream");
 
@@ -110,7 +110,7 @@ class CUDAStreamRef final : public GenericDeviceResourceRef {
 
   /// Destroy the referenced stream and invalidate the reference. The reference
   /// must be to a valid stream before calling to this function.
-  Error destroy() override {
+  Error destroy(GenericDeviceTy &Device) override {
     if (!Stream)
       return Plugin::error("Destroying an invalid stream");
 
@@ -140,7 +140,7 @@ class CUDAEventRef final : public GenericDeviceResourceRef {
 
   /// Create a new event and save the reference. The reference must be empty
   /// before calling to this function.
-  Error create() override {
+  Error create(GenericDeviceTy &Device) override {
     if (Event)
       return Plugin::error("Creating an existing event");
 
@@ -153,7 +153,7 @@ class CUDAEventRef final : public GenericDeviceResourceRef {
 
   /// Destroy the referenced event and invalidate the reference. The reference
   /// must be to a valid event before calling to this function.
-  Error destroy() override {
+  Error destroy(GenericDeviceTy &Device) override {
     if (!Event)
       return Plugin::error("Destroying an invalid event");