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

[kevinsala]

Summary

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.

[jhuber6]

LGTM overall, we probably should stand up CI testing for this stuff however.

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.h

protected:
   std::mutex PeerAccessesLock;
 };
 
 /// Class implementing common functionalities of offload plugins. Each plugin
 /// should define the specific plugin class, derive from this generic one, and
 /// 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;
-  }
-
-  /// 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");
-    }
+  /// Construct a plugin instance.
+  GenericPluginTy()
+      : RequiresFlags(OMP_REQ_UNDEFINED), GlobalHandler(nullptr) {}
 
-    --NumActiveInstances;
-  }
+  virtual ~GenericPluginTy() {}
+
+  /// 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) {
     assert(isValidDeviceId(DeviceId) && "Invalid device id");
     assert(Devices[DeviceId] && "Device is unitialized");
 
     return *Devices[DeviceId];
   }
struct GenericPluginTy {
   /// Indicate if an image is compatible with the plugin devices. Notice that
   /// this function may be called before actually initializing the devices. So
   /// we could not move this function into GenericDeviceTy.
   virtual Expected<bool> isImageCompatible(__tgt_image_info *Info) const = 0;
 
   /// 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());
   }
 
 private:
   /// Number of devices available for the plugin.
   int32_t NumDevices;
 
   /// Array of pointers to the devices. Initially, they are all set to nullptr.
   /// Once a device is initialized, the pointer is stored in the position given
   /// by its device id. A position with nullptr means that the corresponding
   /// device was not initialized yet.
   llvm::SmallVector<GenericDeviceTy *> Devices;
 
   /// OpenMP requires flags.
   int64_t RequiresFlags;
 
   /// Pointer to the global handler for this plugin.
   GenericGlobalHandlerTy *GlobalHandler;
 
   /// Internal allocator for different 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();
+
+    return Error::success();
+  }
 
-  /// Deinitialize the plugin if it was not deinitialized yet.
-  static Error deinit();
+  // 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.
   template <typename... ArgsTy>
   static Error error(const char *ErrFmt, ArgsTy... Args) {
     return createStringError(inconvertibleErrorCode(), ErrFmt, Args...);
   }
 
   /// Check the plugin-specific error code and return an error or success
   /// accordingly. In case of an error, create a string error with the error
   /// description. The ErrFmt should follow the format:
   ///     "Error in <function name>[<optional info>]: %s"
   /// The last format specifier "%s" is mandatory and will be used to place the
   /// error code's description. Notice this function should be only called from
   /// 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
 /// as a reference to a device resource, such as a stream, and requires some
 /// basic functions to be implemented. The derived class should define an empty
 /// constructor that creates an empty and invalid resource reference. Do not
 /// create a new resource on the ctor, but on the create() function instead.
 struct GenericDeviceResourceRef {

openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp

 #include <cstdint>
 #include <limits>
 
 using namespace llvm;
 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.
   // In that case, and assuming the current status code is OK, we will
   // synchronize explicitly when the object is deleted.
   if (AsyncInfoPtr == &LocalAsyncInfo && LocalAsyncInfo.Queue && !Err)
     Err = Device.synchronize(&LocalAsyncInfo);
 }

 }
 
 Error GenericDeviceTy::initDeviceInfo(__tgt_device_info *DeviceInfo) {
   assert(DeviceInfo && "Invalid device info");
 
   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();
 }
 
 Error GenericDeviceTy::createEvent(void **EventPtrStorage) {
   return createEventImpl(EventPtrStorage);
 }
Error GenericDeviceTy::waitEvent(void *EventPtr, __tgt_async_info *AsyncInfo) {
   Err = waitEventImpl(EventPtr, AsyncInfoWrapper);
   return Err;
 }
 
 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.
 #ifdef __cplusplus
 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());
 
   return (bool)Err;
 }
 
 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());
 
   return (bool)Err;
 }
 
 int32_t __tgt_rtl_is_valid_binary(__tgt_device_image *TgtImage) {

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

Error getGlobalMetadataFromDevice(GenericDeviceTy &Device,
 
     DeviceGlobal.setPtr(reinterpret_cast<void *>(CUPtr));
     return Plugin::success();
   }
 };
 
 /// 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) {
       CUdevice Device;
       CUresult Res = cuDeviceGet(&Device, DevId);
       if (auto Err = Plugin::check(Res, "Error in cuDeviceGet: %s"))
         return std::move(Err);
 
if (CanAccessPeer) {
     return Plugin::check(Res, "Error in cuMemcpyPeerAsync: %s");
   }
 
   // Fallback to D2D copy.
   Res = cuMemcpyDtoDAsync(CUDstPtr, CUSrcPtr, Size, Stream);
   return Plugin::check(Res, "Error in cuMemcpyDtoDAsync: %s");
 }
 
-Error Plugin::init() {
-  // Call the getter to intialize the CUDA plugin.
-  get();
-  return Plugin::success();
-}
+GenericPluginTy *Plugin::createPlugin() { return new CUDAPluginTy(); }
 
-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");
-
-  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>
 Error Plugin::check(int32_t Code, const char *ErrFmt, ArgsTy... Args) {
   CUresult ResultCode = static_cast<CUresult>(Code);
   if (ResultCode == CUDA_SUCCESS)
     return Error::success();
 

openmp/libomptarget/plugins-nextgen/generic-elf-64bit/src/rtl.cpp

Error getGlobalMetadataFromDevice(GenericDeviceTy &GenericDevice,
     DeviceGlobal.setPtr(Addr);
 
     return Plugin::success();
   }
 };
 
 /// 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;
   }
 
   /// All images (ELF-compatible) should be compatible with this plugin.
   Expected<bool> isImageCompatible(__tgt_image_info *Info) const override {
     return true;
   }
 };
 
-Error Plugin::init() {
-  // Call the getter to intialize the GenELF64 plugin.
-  get();
-  return Plugin::success();
-}
+GenericPluginTy *Plugin::createPlugin() { return new GenELF64PluginTy(); }
 
-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");
-
-  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>
 Error Plugin::check(int32_t Code, const char *ErrMsg, ArgsTy... Args) {
   if (Code == 0)
     return Error::success();
 
   return createStringError<ArgsTy..., const char *>(
       inconvertibleErrorCode(), ErrMsg, Args..., std::to_string(Code).data());
 }
 
 } // namespace plugin
 } // namespace target
 } // namespace omp
 } // namespace llvm