[Openmp-commits] [openmp] 2257e3d - [openmp] Workaround for HSA in issue 60119

[Jon Chesterfield via Openmp-commits]

Author: Jon Chesterfield
Date: 2023-01-21T12:01:14Z
New Revision: 2257e3d2e55a52d70db10e9f4ed1669ab79ace3f

URL: https://github.com/llvm/llvm-project/commit/2257e3d2e55a52d70db10e9f4ed1669ab79ace3f
DIFF: https://github.com/llvm/llvm-project/commit/2257e3d2e55a52d70db10e9f4ed1669ab79ace3f.diff

LOG: [openmp] Workaround for HSA in issue 60119

Move plugin initialization to libomptarget initialization.

Removes the call_once control, probably fractionally faster overall.
Fixes issue 60119 because the plugin initialization, which might
try to dlopen unrelated shared libraries, is no longer nested within
a call from application code.

Fixes #60119

Reviewed By: Maetveis, jhuber6

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

Added: 
    

Modified: 
    openmp/libomptarget/include/device.h
    openmp/libomptarget/include/rtl.h
    openmp/libomptarget/plugins/remote/server/Server.cpp
    openmp/libomptarget/src/interface.cpp
    openmp/libomptarget/src/rtl.cpp

Removed: 
    


################################################################################
diff  --git a/openmp/libomptarget/include/device.h b/openmp/libomptarget/include/device.h
index a223475515bff..0ddf75f2d264c 100644
--- a/openmp/libomptarget/include/device.h
+++ b/openmp/libomptarget/include/device.h
@@ -530,6 +530,31 @@ struct PluginManager {
   /// Flag to indicate if we use events to ensure the atomicity of
   /// map clauses or not. Can be modified with an environment variable.
   const bool UseEventsForAtomicTransfers;
+
+  // Work around for plugins that call dlopen on shared libraries that call
+  // tgt_register_lib during their initialisation. Stash the pointers in a
+  // vector until the plugins are all initialised and then register them.
+  bool maybeDelayRegisterLib(__tgt_bin_desc *Desc) {
+    if (!RTLsLoaded) {
+      // Only reachable from libomptarget constructor
+      DelayedBinDesc.push_back(Desc);
+      return true;
+    } else {
+      return false;
+    }
+  }
+
+  void registerDelayedLibraries() {
+    // Only called by libomptarget constructor
+    RTLsLoaded = true;
+    for (auto *Desc : DelayedBinDesc)
+      __tgt_register_lib(Desc);
+    DelayedBinDesc.clear();
+  }
+
+private:
+  bool RTLsLoaded = false;
+  llvm::SmallVector<__tgt_bin_desc *> DelayedBinDesc;
 };
 
 extern PluginManager *PM;

diff  --git a/openmp/libomptarget/include/rtl.h b/openmp/libomptarget/include/rtl.h
index 4c881fe2e0965..063acc153f830 100644
--- a/openmp/libomptarget/include/rtl.h
+++ b/openmp/libomptarget/include/rtl.h
@@ -171,10 +171,8 @@ struct RTLsTy {
   // Unregister a shared library from all RTLs.
   void unregisterLib(__tgt_bin_desc *Desc);
 
-  // Mutex-like object to guarantee thread-safety and unique initialization
-  // (i.e. the library attempts to load the RTLs (plugins) only once).
-  std::once_flag InitFlag;
-  void loadRTLs(); // not thread-safe
+  // not thread-safe, called from global constructor (i.e. once)
+  void loadRTLs();
 
 private:
   static bool attemptLoadRTL(const std::string &RTLName, RTLInfoTy &RTL);

diff  --git a/openmp/libomptarget/plugins/remote/server/Server.cpp b/openmp/libomptarget/plugins/remote/server/Server.cpp
index c87ae5458f10a..372ca94128a8f 100644
--- a/openmp/libomptarget/plugins/remote/server/Server.cpp
+++ b/openmp/libomptarget/plugins/remote/server/Server.cpp
@@ -90,8 +90,6 @@ Status RemoteOffloadImpl::IsValidBinary(ServerContext *Context,
 Status RemoteOffloadImpl::GetNumberOfDevices(ServerContext *Context,
                                              const Null *Null,
                                              I32 *NumberOfDevices) {
-  std::call_once(PM->RTLs.initFlag, &RTLsTy::LoadRTLs, &PM->RTLs);
-
   int32_t Devices = 0;
   PM->RTLsMtx.lock();
   for (auto &RTL : PM->RTLs.AllRTLs)

diff  --git a/openmp/libomptarget/src/interface.cpp b/openmp/libomptarget/src/interface.cpp
index 3871e6828a57e..a53d963d81f20 100644
--- a/openmp/libomptarget/src/interface.cpp
+++ b/openmp/libomptarget/src/interface.cpp
@@ -35,7 +35,9 @@ EXTERN void __tgt_register_requires(int64_t Flags) {
 /// adds a target shared library to the target execution image
 EXTERN void __tgt_register_lib(__tgt_bin_desc *Desc) {
   TIMESCOPE();
-  std::call_once(PM->RTLs.InitFlag, &RTLsTy::loadRTLs, &PM->RTLs);
+  if (PM->maybeDelayRegisterLib(Desc))
+    return;
+
   for (auto &RTL : PM->RTLs.AllRTLs) {
     if (RTL.register_lib) {
       if ((*RTL.register_lib)(Desc) != OFFLOAD_SUCCESS) {

diff  --git a/openmp/libomptarget/src/rtl.cpp b/openmp/libomptarget/src/rtl.cpp
index 6f84aa238976c..1c490c40590e0 100644
--- a/openmp/libomptarget/src/rtl.cpp
+++ b/openmp/libomptarget/src/rtl.cpp
@@ -64,6 +64,9 @@ __attribute__((constructor(101))) void init() {
   // TODO: add a configuration option for time granularity
   if (ProfileTraceFile)
     timeTraceProfilerInitialize(500 /* us */, "libomptarget");
+
+  PM->RTLs.loadRTLs();
+  PM->registerDelayedLibraries();
 }
 
 __attribute__((destructor(101))) void deinit() {