[Openmp-commits] [PATCH] D131089: [Libomptarget] Explicitly init / deinit libomptarget from the user

Jon Chesterfield via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Fri Jan 20 08:31:57 PST 2023


JonChesterfield added inline comments.


================
Comment at: openmp/libomptarget/src/rtl.cpp:44
 
-static char *ProfileTraceFile = nullptr;
+static std::mutex PluginManagerMutex;
 
----------------
This probably puts a call in global ctors. Suggest the uglier pthread alternative which doesn't do that.


================
Comment at: openmp/libomptarget/src/rtl.cpp:104
+}
+
 void RTLsTy::loadRTLs() {
----------------
I think the above is right but it took me a while to parse. Control flow and data flow are a bit interleaved. Could we go with:

```
if (PM == nullptr) {
  PM == new PluginManager();
  PM->Refcount = 1; // Perhaps the plugin manager should init it to 1 on construction?
  PM->Load();
}
```

and
```
uint64_t before = PM->Refcount.fetch_sub(1);
if (before == 1) {
  PM->Unload();
  delete PM;
}
```

It seems somewhat strange that the plugin manager is a global object pointing to the heap. I wonder if that can be simplified, independent of this.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131089/new/

https://reviews.llvm.org/D131089



More information about the Openmp-commits mailing list