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

[Joseph Huber via Phabricator via Openmp-commits]

jhuber6 added inline comments.


================
Comment at: openmp/libomptarget/src/rtl.cpp:44
 
-static char *ProfileTraceFile = nullptr;
+static std::mutex PluginManagerMutex;
 
----------------
JonChesterfield wrote:
> This probably puts a call in global ctors. Suggest the uglier pthread alternative which doesn't do that.
Do you have an example of that? It wouldn't work with `std::scoped_lock` then right.


================
Comment at: openmp/libomptarget/src/rtl.cpp:104
+}
+
 void RTLsTy::loadRTLs() {
----------------
JonChesterfield wrote:
> 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.
Logically I just wanted the `load / unload` routines to fire when the reference count is one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131089