[Openmp-commits] [openmp] 2cb83cd - [OpenMP][libomptarget] Improve NextGen plugin interface for initialization

Kevin Sala via Openmp-commits openmp-commits at lists.llvm.org
Sat Dec 3 13:26:17 PST 2022


Author: Kevin Sala
Date: 2022-12-03T22:25:15+01:00
New Revision: 2cb83cd288fd6543bb22713ff3eba50faadc20e0

URL: https://github.com/llvm/llvm-project/commit/2cb83cd288fd6543bb22713ff3eba50faadc20e0
DIFF: https://github.com/llvm/llvm-project/commit/2cb83cd288fd6543bb22713ff3eba50faadc20e0.diff

LOG: [OpenMP][libomptarget] Improve NextGen plugin interface for initialization

This patch modifies the PluginInterface to define functions for initializing
and deinitializing GenericPluginTy instances instead of using the constructor
and destructor. This way, we can return errors from these functions. Also, it
defines some functions that each plugin should implement for creating
plugin-specific objects.

This patch prepares the PluginInterface for the new AMDGPU NextGen plugin.

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

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
    openmp/libomptarget/plugins-nextgen/generic-elf-64bit/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 a69470e864df5..0dff68901e468 100644
--- a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp
@@ -23,7 +23,7 @@ using namespace omp;
 using namespace target;
 using namespace plugin;
 
-uint32_t GenericPluginTy::NumActiveInstances = 0;
+GenericPluginTy *Plugin::SpecificPlugin = nullptr;
 
 AsyncInfoWrapperTy::~AsyncInfoWrapperTy() {
   // If we used a local async info object we want synchronous behavior.
@@ -452,33 +452,6 @@ Error GenericDeviceTy::initDeviceInfo(__tgt_device_info *DeviceInfo) {
   return initDeviceInfoImpl(DeviceInfo);
 }
 
-Error GenericPluginTy::initDevice(int32_t DeviceId) {
-  assert(!Devices[DeviceId] && "Device already initialized");
-
-  // Create the device and save the reference.
-  GenericDeviceTy &Device = createDevice(DeviceId);
-  Devices[DeviceId] = &Device;
-
-  // Initialize the device and its resources.
-  return Device.init(*this);
-}
-
-Error GenericPluginTy::deinitDevice(int32_t DeviceId) {
-  // The device may be already deinitialized.
-  if (Devices[DeviceId] == nullptr)
-    return Plugin::success();
-
-  // Deinitialize the device and release its resources.
-  if (auto Err = Devices[DeviceId]->deinit())
-    return Err;
-
-  // Delete the device and invalidate its reference.
-  delete Devices[DeviceId];
-  Devices[DeviceId] = nullptr;
-
-  return Plugin::success();
-}
-
 Error GenericDeviceTy::printInfo() {
   // TODO: Print generic information here
   return printInfoImpl();
@@ -511,6 +484,72 @@ Error GenericDeviceTy::syncEvent(void *EventPtr) {
   return syncEventImpl(EventPtr);
 }
 
+Error GenericPluginTy::init() {
+  auto NumDevicesOrErr = initImpl();
+  if (!NumDevicesOrErr)
+    return NumDevicesOrErr.takeError();
+
+  NumDevices = *NumDevicesOrErr;
+  if (NumDevices == 0)
+    return Plugin::success();
+
+  assert(Devices.size() == 0 && "Plugin already initialized");
+  Devices.resize(NumDevices, nullptr);
+
+  GlobalHandler = Plugin::createGlobalHandler();
+  assert(GlobalHandler && "Invalid global handler");
+
+  return Plugin::success();
+}
+
+Error GenericPluginTy::deinit() {
+  // There is no global handler if no device is available.
+  if (GlobalHandler)
+    delete GlobalHandler;
+
+  // Deinitialize all active devices.
+  for (int32_t DeviceId = 0; DeviceId < NumDevices; ++DeviceId) {
+    if (Devices[DeviceId]) {
+      if (auto Err = deinitDevice(DeviceId))
+        return Err;
+    }
+    assert(!Devices[DeviceId] && "Device was not deinitialized");
+  }
+
+  // Perform last deinitializations on the plugin.
+  return deinitImpl();
+}
+
+Error GenericPluginTy::initDevice(int32_t DeviceId) {
+  assert(!Devices[DeviceId] && "Device already initialized");
+
+  // Create the device and save the reference.
+  GenericDeviceTy *Device = Plugin::createDevice(DeviceId, NumDevices);
+  assert(Device && "Invalid device");
+
+  // Save the device reference into the list.
+  Devices[DeviceId] = Device;
+
+  // Initialize the device and its resources.
+  return Device->init(*this);
+}
+
+Error GenericPluginTy::deinitDevice(int32_t DeviceId) {
+  // The device may be already deinitialized.
+  if (Devices[DeviceId] == nullptr)
+    return Plugin::success();
+
+  // Deinitialize the device and release its resources.
+  if (auto Err = Devices[DeviceId]->deinit())
+    return Err;
+
+  // Delete the device and invalidate its reference.
+  delete Devices[DeviceId];
+  Devices[DeviceId] = nullptr;
+
+  return Plugin::success();
+}
+
 /// Exposed library API function, basically wrappers around the GenericDeviceTy
 /// functionality with the same name. All non-async functions are redirected
 /// to the async versions right away with a NULL AsyncInfoPtr.
@@ -519,7 +558,7 @@ extern "C" {
 #endif
 
 int32_t __tgt_rtl_init_plugin() {
-  auto Err = Plugin::init();
+  auto Err = Plugin::initIfNeeded();
   if (Err)
     REPORT("Failure to initialize plugin " GETNAME(TARGET_NAME) ": %s\n",
            toString(std::move(Err)).data());
@@ -528,7 +567,7 @@ int32_t __tgt_rtl_init_plugin() {
 }
 
 int32_t __tgt_rtl_deinit_plugin() {
-  auto Err = Plugin::deinit();
+  auto Err = Plugin::deinitIfNeeded();
   if (Err)
     REPORT("Failure to deinitialize plugin " GETNAME(TARGET_NAME) ": %s\n",
            toString(std::move(Err)).data());

diff  --git a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
index 3df2162f72038..43ae7da2a11f5 100644
--- a/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
+++ b/openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h
@@ -444,31 +444,21 @@ struct GenericDeviceTy : public DeviceAllocatorTy {
 /// implement the necessary virtual function members.
 struct GenericPluginTy {
 
-  /// Construct a plugin instance. The number of active instances should be
-  /// always be either zero or one.
-  GenericPluginTy() : RequiresFlags(OMP_REQ_UNDEFINED), GlobalHandler(nullptr) {
-    ++NumActiveInstances;
-  }
+  /// Construct a plugin instance.
+  GenericPluginTy()
+      : RequiresFlags(OMP_REQ_UNDEFINED), GlobalHandler(nullptr) {}
 
-  /// Destroy the plugin instance and release all its resources. Also decrease
-  /// the number of instances.
-  virtual ~GenericPluginTy() {
-    // There is no global handler if no device is available.
-    if (GlobalHandler)
-      delete GlobalHandler;
-
-    // Deinitialize all active devices.
-    for (int32_t DeviceId = 0; DeviceId < NumDevices; ++DeviceId) {
-      if (Devices[DeviceId]) {
-        if (auto Err = deinitDevice(DeviceId))
-          REPORT("Failure to deinitialize device %d: %s\n", DeviceId,
-                 toString(std::move(Err)).data());
-      }
-      assert(!Devices[DeviceId] && "Device was not deinitialized");
-    }
+  virtual ~GenericPluginTy() {}
 
-    --NumActiveInstances;
-  }
+  /// Initialize the plugin.
+  Error init();
+
+  /// Initialize the plugin and return the number of available devices.
+  virtual Expected<int32_t> initImpl() = 0;
+
+  /// Deinitialize the plugin and release the resources.
+  Error deinit();
+  virtual Error deinitImpl() = 0;
 
   /// Get the reference to the device with a certain device id.
   GenericDeviceTy &getDevice(int32_t DeviceId) {
@@ -522,26 +512,7 @@ struct GenericPluginTy {
   /// Indicate whether the plugin supports empty images.
   virtual bool supportsEmptyImages() const { return false; }
 
-  /// Indicate whether there is any active plugin instance.
-  static bool hasAnyActiveInstance() {
-    assert(NumActiveInstances <= 1 && "Invalid number of instances");
-    return (NumActiveInstances > 0);
-  }
-
 protected:
-  /// Initialize the plugin and prepare for initializing its devices.
-  void init(int NumDevices, GenericGlobalHandlerTy *GlobalHandler) {
-    this->NumDevices = NumDevices;
-    this->GlobalHandler = GlobalHandler;
-
-    assert(Devices.size() == 0 && "Plugin already intialized");
-
-    Devices.resize(NumDevices, nullptr);
-  }
-
-  /// Create a new device with a specific device id.
-  virtual GenericDeviceTy &createDevice(int32_t DeviceId) = 0;
-
   /// Indicate whether a device id is valid.
   bool isValidDeviceId(int32_t DeviceId) const {
     return (DeviceId >= 0 && DeviceId < getNumDevices());
@@ -565,41 +536,98 @@ struct GenericPluginTy {
 
   /// Internal allocator for 
diff erent structures.
   BumpPtrAllocator Allocator;
-
-  /// Indicates the number of active plugin instances. Actually, we should only
-  /// have one active instance per plugin library. But we use a counter for
-  /// simplicity.
-  static uint32_t NumActiveInstances;
 };
 
 /// Class for simplifying the getter operation of the plugin. Anywhere on the
-/// code, the current plugin can be retrieved by Plugin::get(). The init(),
-/// deinit(), get() and check() functions should be defined by each plugin
-/// implementation.
+/// code, the current plugin can be retrieved by Plugin::get(). The class also
+/// declares functions to create plugin-specific object instances. The check(),
+/// createPlugin(), createDevice() and createGlobalHandler() functions should be
+/// defined by each plugin implementation.
 class Plugin {
-  /// Avoid instances of this class.
-  Plugin() {}
+  // Reference to the plugin instance.
+  static GenericPluginTy *SpecificPlugin;
+
+  Plugin() {
+    if (auto Err = init())
+      REPORT("Failed to initialize plugin: %s\n",
+             toString(std::move(Err)).data());
+  }
+
+  ~Plugin() {
+    if (auto Err = deinit())
+      REPORT("Failed to deinitialize plugin: %s\n",
+             toString(std::move(Err)).data());
+  }
+
   Plugin(const Plugin &) = delete;
   void operator=(const Plugin &) = delete;
 
+  /// Create and intialize the plugin instance.
+  static Error init() {
+    assert(!SpecificPlugin && "Plugin already created");
+
+    // Create the specific plugin.
+    SpecificPlugin = createPlugin();
+    assert(SpecificPlugin && "Plugin was not created");
+
+    // Initialize the plugin.
+    return SpecificPlugin->init();
+  }
+
+  // Deinitialize and destroy the plugin instance.
+  static Error deinit() {
+    assert(SpecificPlugin && "Plugin no longer valid");
+
+    // Deinitialize the plugin.
+    if (auto Err = SpecificPlugin->deinit())
+      return Err;
+
+    // Delete the plugin instance.
+    delete SpecificPlugin;
+
+    // Invalidate the plugin reference.
+    SpecificPlugin = nullptr;
+
+    return Plugin::success();
+  }
+
 public:
-  /// Initialize the plugin if it was not initialized yet.
-  static Error init();
+  /// Initialize the plugin if needed. The plugin could have been initialized by
+  /// a previous call to Plugin::get().
+  static Error initIfNeeded() {
+    // Trigger the initialization if needed.
+    get();
 
-  /// Deinitialize the plugin if it was not deinitialized yet.
-  static Error deinit();
+    return Error::success();
+  }
+
+  // Deinitialize the plugin if needed. The plugin could have been deinitialized
+  // because the plugin library was exiting.
+  static Error deinitIfNeeded() {
+    // Do nothing. The plugin is deinitialized automatically.
+    return Plugin::success();
+  }
 
   /// Get a reference (or create if it was not created) to the plugin instance.
-  static GenericPluginTy &get();
+  static GenericPluginTy &get() {
+    // This static variable will initialize the underlying plugin instance in
+    // case there was no previous explicit initialization. The initialization is
+    // thread safe.
+    static Plugin Plugin;
+
+    assert(SpecificPlugin && "Plugin is not active");
+    return *SpecificPlugin;
+  }
 
   /// Get a reference to the plugin with a specific plugin-specific type.
   template <typename Ty> static Ty &get() { return static_cast<Ty &>(get()); }
 
-  /// Indicate if the plugin is currently active. Actually, we check if there is
-  /// any active instances.
-  static bool isActive() { return GenericPluginTy::hasAnyActiveInstance(); }
+  /// Indicate whether the plugin is active.
+  static bool isActive() { return SpecificPlugin != nullptr; }
 
-  /// Create a success error.
+  /// Create a success error. This is the same as calling Error::success(), but
+  /// it is recommended to use this one for consistency with Plugin::error() and
+  /// Plugin::check().
   static Error success() { return Error::success(); }
 
   /// Create a string error.
@@ -617,6 +645,15 @@ class Plugin {
   /// the plugin-specific code.
   template <typename... ArgsTy>
   static Error check(int32_t ErrorCode, const char *ErrFmt, ArgsTy... Args);
+
+  /// Create a plugin instance.
+  static GenericPluginTy *createPlugin();
+
+  /// Create a plugin-specific device.
+  static GenericDeviceTy *createDevice(int32_t DeviceId, int32_t NumDevices);
+
+  /// Create a plugin-specific global handler.
+  static GenericGlobalHandlerTy *createGlobalHandler();
 };
 
 /// Auxiliary interface class for GenericDeviceResourcePoolTy. This class acts

diff  --git a/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp b/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
index 70ad05f13e609..6f117a7b1bc90 100644
--- a/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
+++ b/openmp/libomptarget/plugins-nextgen/cuda/src/rtl.cpp
@@ -848,59 +848,50 @@ class CUDAGlobalHandlerTy final : public GenericGlobalHandlerTy {
 
 /// Class implementing the CUDA-specific functionalities of the plugin.
 struct CUDAPluginTy final : public GenericPluginTy {
-  /// Create a CUDA plugin and initialize the CUDA driver.
-  CUDAPluginTy() : GenericPluginTy() {
+  /// Create a CUDA plugin.
+  CUDAPluginTy() : GenericPluginTy() {}
+
+  /// This class should not be copied.
+  CUDAPluginTy(const CUDAPluginTy &) = delete;
+  CUDAPluginTy(CUDAPluginTy &&) = delete;
+
+  /// Initialize the plugin and return the number of devices.
+  Expected<int32_t> initImpl() override {
     CUresult Res = cuInit(0);
     if (Res == CUDA_ERROR_INVALID_HANDLE) {
       // Cannot call cuGetErrorString if dlsym failed.
       DP("Failed to load CUDA shared library\n");
-      return;
+      return 0;
     }
 
     if (Res == CUDA_ERROR_NO_DEVICE) {
       // Do not initialize if there are no devices.
       DP("There are no devices supporting CUDA.\n");
-      return;
+      return 0;
     }
 
-    if (auto Err = Plugin::check(Res, "Error in cuInit: %s")) {
-      REPORT("%s\n", toString(std::move(Err)).data());
-      return;
-    }
+    if (auto Err = Plugin::check(Res, "Error in cuInit: %s"))
+      return std::move(Err);
 
     // Get the number of devices.
     int NumDevices;
     Res = cuDeviceGetCount(&NumDevices);
-    if (auto Err = Plugin::check(Res, "Error in cuDeviceGetCount: %s")) {
-      REPORT("%s\n", toString(std::move(Err)).data());
-      return;
-    }
+    if (auto Err = Plugin::check(Res, "Error in cuDeviceGetCount: %s"))
+      return std::move(Err);
 
     // Do not initialize if there are no devices.
-    if (NumDevices == 0) {
+    if (NumDevices == 0)
       DP("There are no devices supporting CUDA.\n");
-      return;
-    }
 
-    // Initialize the generic plugin structure.
-    GenericPluginTy::init(NumDevices, new CUDAGlobalHandlerTy());
+    return NumDevices;
   }
 
-  /// This class should not be copied.
-  CUDAPluginTy(const CUDAPluginTy &) = delete;
-  CUDAPluginTy(CUDAPluginTy &&) = delete;
-
-  ~CUDAPluginTy() {}
+  /// Deinitialize the plugin.
+  Error deinitImpl() override { return Plugin::success(); }
 
   /// Get the ELF code for recognizing the compatible image binary.
   uint16_t getMagicElfBits() const override { return ELF::EM_CUDA; }
 
-  /// Create a CUDA device with a specific id.
-  CUDADeviceTy &createDevice(int32_t DeviceId) override {
-    CUDADeviceTy *Device = new CUDADeviceTy(DeviceId, getNumDevices());
-    return *Device;
-  }
-
   /// Check whether the image is compatible with the available CUDA devices.
   Expected<bool> isImageCompatible(__tgt_image_info *Info) const override {
     for (int32_t DevId = 0; DevId < getNumDevices(); ++DevId) {
@@ -1002,32 +993,14 @@ Error CUDADeviceTy::dataExchangeImpl(const void *SrcPtr,
   return Plugin::check(Res, "Error in cuMemcpyDtoDAsync: %s");
 }
 
-Error Plugin::init() {
-  // Call the getter to intialize the CUDA plugin.
-  get();
-  return Plugin::success();
-}
-
-Error Plugin::deinit() {
-  // The CUDA plugin and the CUDA driver should already be deinitialized
-  // at this point. So do nothing for this plugin.
-  if (Plugin::isActive())
-    return Plugin::error("CUDA plugin is not deinitialized");
+GenericPluginTy *Plugin::createPlugin() { return new CUDAPluginTy(); }
 
-  return Plugin::success();
+GenericDeviceTy *Plugin::createDevice(int32_t DeviceId, int32_t NumDevices) {
+  return new CUDADeviceTy(DeviceId, NumDevices);
 }
 
-GenericPluginTy &Plugin::get() {
-  // The CUDA plugin instance is built the first time that Plugin::get() is
-  // called thanks to the following static variable. The ideal implementation
-  // would initialize the plugin in Plugin::init() (__tgt_rtl_plugin_init) and
-  // destroy it in Plugin::deinit() (__tgt_rtl_plugin_deinit). However, at the
-  // time Plugin::deinit() is called, the CUDA driver is already shut down. That
-  // is caused by the fact that __tgt_rtl_plugin_deinit is called from a dtor
-  // in libomptarget. Thus, this is a workaround until that aspect is fixed.
-  static CUDAPluginTy CUDAPlugin;
-  assert(Plugin::isActive() && "Plugin is not active");
-  return CUDAPlugin;
+GenericGlobalHandlerTy *Plugin::createGlobalHandler() {
+  return new CUDAGlobalHandlerTy();
 }
 
 template <typename... ArgsTy>

diff  --git a/openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp b/openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp
index 553dca03f1608..ff09b962d7b97 100644
--- a/openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp
+++ b/openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp
@@ -333,28 +333,22 @@ class GenELF64GlobalHandlerTy final : public GenericGlobalHandlerTy {
 
 /// Class implementing the plugin functionalities for GenELF64.
 struct GenELF64PluginTy final : public GenericPluginTy {
-  /// Create the plugin.
-  GenELF64PluginTy() : GenericPluginTy() {
-    // Initialize the generic plugin structure with multiple devices and a
-    // global handler.
-    GenericPluginTy::init(NUM_DEVICES, new GenELF64GlobalHandlerTy());
-  }
+  /// Create the GenELF64 plugin.
+  GenELF64PluginTy() : GenericPluginTy() {}
 
   /// This class should not be copied.
   GenELF64PluginTy(const GenELF64PluginTy &) = delete;
   GenELF64PluginTy(GenELF64PluginTy &&) = delete;
 
-  ~GenELF64PluginTy() {}
+  /// Initialize the plugin and return the number of devices.
+  Expected<int32_t> initImpl() override { return NUM_DEVICES; }
+
+  /// Deinitialize the plugin.
+  Error deinitImpl() override { return Plugin::success(); }
 
   /// Get the ELF code to recognize the compatible binary images.
   uint16_t getMagicElfBits() const override { return TARGET_ELF_ID; }
 
-  /// Create a GenELF64 device with a specific id.
-  GenELF64DeviceTy &createDevice(int32_t DeviceId) override {
-    GenELF64DeviceTy *Device = new GenELF64DeviceTy(DeviceId, getNumDevices());
-    return *Device;
-  }
-
   /// This plugin does not support exchanging data between two devices.
   bool isDataExchangable(int32_t SrcDeviceId, int32_t DstDeviceId) override {
     return false;
@@ -366,24 +360,14 @@ struct GenELF64PluginTy final : public GenericPluginTy {
   }
 };
 
-Error Plugin::init() {
-  // Call the getter to intialize the GenELF64 plugin.
-  get();
-  return Plugin::success();
-}
-
-Error Plugin::deinit() {
-  // The Generic ELF64 plugin should already be deinitialized at this point.
-  if (Plugin::isActive())
-    return Plugin::error("Generic ELF64 plugin is not deinitialized");
+GenericPluginTy *Plugin::createPlugin() { return new GenELF64PluginTy(); }
 
-  return Plugin::success();
+GenericDeviceTy *Plugin::createDevice(int32_t DeviceId, int32_t NumDevices) {
+  return new GenELF64DeviceTy(DeviceId, NumDevices);
 }
 
-GenericPluginTy &Plugin::get() {
-  static GenELF64PluginTy GenELF64Plugin;
-  assert(Plugin::isActive() && "Plugin is not active");
-  return GenELF64Plugin;
+GenericGlobalHandlerTy *Plugin::createGlobalHandler() {
+  return new GenELF64GlobalHandlerTy();
 }
 
 template <typename... ArgsTy>


        


More information about the Openmp-commits mailing list