[Openmp-commits] [PATCH] D142008: [libomptarget] Delay recursive shared library registrations until offload RTLs are loaded

[Mészáros Gergely via Phabricator via Openmp-commits]

Maetveis created this revision.
Maetveis added a reviewer: jhuber6.
Maetveis added a project: OpenMP.
Herald added a project: All.
Maetveis requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: openmp-commits, sstefan1.

The target runtimes may (transitively) load shared libraries that need to be registered. Prior to this patch this would result in a deadlock as the recursive call attempted
to initialize the runtimes again waiting for the lock that the outer call is holding.
Detect these recursive calls and delay the initialization of the inner libraries until the runtimes are fully loaded in the outer calls.

Fixes: https://github.com/llvm/llvm-project/issues/60119


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D142008

Files:
  openmp/libomptarget/include/rtl.h
  openmp/libomptarget/src/interface.cpp
  openmp/libomptarget/src/rtl.cpp


Index: openmp/libomptarget/src/rtl.cpp
===================================================================
--- openmp/libomptarget/src/rtl.cpp
+++ openmp/libomptarget/src/rtl.cpp
@@ -353,6 +353,8 @@
   return __tgt_image_info{(*BinaryOrErr)->getArch().data()};
 }
 
+thread_local llvm::SmallVector<__tgt_bin_desc*> RTLsTy::DelayedDescs;
+
 void RTLsTy::registerRequires(int64_t Flags) {
   // TODO: add more elaborate check.
   // Minimal check: only set requires flags if previous value
Index: openmp/libomptarget/src/interface.cpp
===================================================================
--- openmp/libomptarget/src/interface.cpp
+++ openmp/libomptarget/src/interface.cpp
@@ -18,6 +18,9 @@
 
 #include "Utilities.h"
 
+#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallVector.h"
+
 #include <cassert>
 #include <cstdio>
 #include <cstdlib>
@@ -35,15 +38,28 @@
 /// adds a target shared library to the target execution image
 EXTERN void __tgt_register_lib(__tgt_bin_desc *Desc) {
   TIMESCOPE();
+
+  // `RTLsTy::loadRTLs` `dlopen`s the runtime plugins, which might load other libraries finally
+  // calling `__tgt_register_lib` during their initialization.
+  // When this happens delay the loading of the libraries until the outermost call finishes,
+  // to ensure that the plugins are initialized before any library is loaded.
+  RTLsTy::DelayedDescs.push_back(Desc);
+  if(RTLsTy::DelayedDescs.size() > 1) {
+    return;
+  }
   std::call_once(PM->RTLs.InitFlag, &RTLsTy::loadRTLs, &PM->RTLs);
-  for (auto &RTL : PM->RTLs.AllRTLs) {
-    if (RTL.register_lib) {
-      if ((*RTL.register_lib)(Desc) != OFFLOAD_SUCCESS) {
-        DP("Could not register library with %s", RTL.RTLName.c_str());
+
+  for (auto* DelayedDesc: RTLsTy::DelayedDescs) {
+    for (auto &RTL : PM->RTLs.AllRTLs) {
+      if (RTL.register_lib) {
+        if ((*RTL.register_lib)(DelayedDesc) != OFFLOAD_SUCCESS) {
+          DP("Could not register library with %s", RTL.RTLName.c_str());
+        }
       }
     }
+    PM->RTLs.registerLib(DelayedDesc);
   }
-  PM->RTLs.registerLib(Desc);
+  RTLsTy::DelayedDescs.clear();
 }
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -54,6 +70,16 @@
 /// unloads a target shared library
 EXTERN void __tgt_unregister_lib(__tgt_bin_desc *Desc) {
   TIMESCOPE();
+
+  // Similarly to `__tgt_register_lib`, libraries might be unregistered during `RTLsTy::loadRTLs`.
+  // When this happens just cancel their delayed registration.
+  for (auto It = RTLsTy::DelayedDescs.begin(), E = RTLsTy::DelayedDescs.end(); It != E; ++It) {
+    if(*It == Desc) {
+      RTLsTy::DelayedDescs.erase(It);
+      return;
+    }
+  }
+
   PM->RTLs.unregisterLib(Desc);
   for (auto &RTL : PM->RTLs.UsedRTLs) {
     if (RTL->unregister_lib) {
Index: openmp/libomptarget/include/rtl.h
===================================================================
--- openmp/libomptarget/include/rtl.h
+++ openmp/libomptarget/include/rtl.h
@@ -154,6 +154,8 @@
 
   int64_t RequiresFlags = OMP_REQ_UNDEFINED;
 
+  static thread_local llvm::SmallVector<__tgt_bin_desc*> DelayedDescs;
+
   explicit RTLsTy() = default;
 
   // Register the clauses of the requires directive.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D142008.490105.patch
Type: text/x-patch
Size: 3218 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/openmp-commits/attachments/20230118/4a30fcff/attachment.bin>