[Openmp-commits] [PATCH] D124652: [OpenMP] [OMPT] [amdgpu] [5/8] Implemented device init/fini/load callbacks

[Michael Halkenhäuser via Phabricator via Openmp-commits]

mhalk added inline comments.


================
Comment at: openmp/libomptarget/plugins-nextgen/common/PluginInterface/PluginInterface.cpp:1047
+#ifdef OMPT_SUPPORT
+  OmptCallbackInit();
+#endif
----------------
dhruvachak wrote:
> This will be a problem the way these patches currently are. GenericPluginTy::init() will kick in for multiple plugins, so multiple plugins will initialize the callbacks. Not a problem per se but where it will fail today is during tracking the finalizer in libomptarget. libomptarget is currently set up to track only 1 finalizer. 
> 
> See openmp/libomptarget/src/OmptCallback.cpp
> /// Object that will maintain the finalizer of the plugin
> static LibomptargetRtlFinalizer LibOmptTargetRTLFinalizer;
> 
> So multiple plugins will have callbacks initialized and the last finalizer will overwrite the rest, not a good thing. (We should actually assert on this condition.)
> 
> One solution is to move the call to OmptCallbackInit() within the device-specific plugin implementations, e.g. in AMDGPUPluginTy::initImpl().
Thanks and good catch -- I moved the calls to `ompt::connectLibrary();` into the specific amdgpu & cuda plugins.
(This comment will be removed in a following diff update. Oversight.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124652