[Openmp-commits] [PATCH] D130714: [openmp][amdgpu] Tear down amdgpu plugin accurately

Jon Chesterfield via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Thu Jul 28 10:07:00 PDT 2022


JonChesterfield added a comment.

The order of execution of constructors is an emergent property of how the compiler annotated functions (e.g. `__attribute__((destructor(101)))` above), what structures the linker built from those (which usually depends on linker invocation) and the C runtime, how the linker associates dynamic objects with the executable, how the loader processes those objects and those passed by dlopen. To a good approximation it is unpredictable. Destructors probably execute in reverse order of constructors but I wouldn't bet on it where dlopen (and present or missing dlclose) are involved.

The plugin lifetime cannot encompass the libomptarget lifetime because they are dlopened by libomptarget. In principle the plugins could live beyond libomptarget, but doing so would mean we cannot close them explicitly and must rely on the emergent behaviour of destructors. That is what is presently segfaulting on the 15 release branch on Joseph's machine.

`Any call from d'tor of libomptarget to plugins will lead to UB`

This is untrue in general. If the plugin global destructor has executed before the libomptarget destructor and left the plugin in a state where calls to it will segfault then yes, it will segfault.

This patch changes the amdgpu plugin to have well defined lifetime between calls to plugin_init and plugin_deinit, and puts the call to plugin_deinit in the libomptarget destructor. That strictly nests the lifetime of the plugin within that of libomptarget while arranging for no calls to follow the one to plugin_deinit. That is the only nesting that can be correct.

However the order of global constructors / destructors within the plugin is not especially likely to respect this definition, so the fix there is not not have global constructor / destructors within the plugin. Strictly speaking there's still a statically allocated 8 byte pointer here, but as the destructor for that emits no code we're fine.

Finally this patch makes no difference to nvptx whatsoever as it hasn't implemented init_plugin or deinit_plugin, so nvptx maintains the current roll the die and hope semantics. It can opt into well defined ones, like this patch does to amdgpu, whenever we'd like to stop it unexpectedly segfaulting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130714



More information about the Openmp-commits mailing list