[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 added a comment.

In D142008#4067013 <https://reviews.llvm.org/D142008#4067013>, @JonChesterfield wrote:

> We have provisional consensus that this is a bug in HSA. It shouldn't be calling dlopen on various processes it finds in the address space and a patch to stop it doing that is in progress.

That's good, but note that it can't load any shared library that's compiled with offloading (maybe even any library that's using openmp?) otherwise this can still be triggered.

With this in mind should I continue with this patch?

In D142008#4065506 <https://reviews.llvm.org/D142008#4065506>, @jhuber6 wrote:

> Do we know which module is calling `__tgt_register_lib`? Is it HSA forcing us to do this twice on the same executable?

See my previous replies, its dlopen executing the library init functions (`__attribute((constructor))`) of the two shared libraries,

> The descriptors all have unique pointers I'm guessing?

I would think that they point to static data embedded in the shared libraries, so yes, but I didn't verify.

> I'm guessing whatever is causing this also calls `__tgt_unregister_lib`?

May or may not in general whatever called `dlopen` can also call `dlclose` and if that was the last reference to the shared library then it will unload calling `__tgt_unregister_lib` in the library destructor.

In D142008#4065692 <https://reviews.llvm.org/D142008#4065692>, @JonChesterfield wrote:

> In D142008#4065590 <https://reviews.llvm.org/D142008#4065590>, @jdoerfert wrote:
>
>> Instead of this local atomic and thread Id stuff, why don't we lock the entire thing? One thread at a time can register or unregister. It's not like we want to make this part super fast.
>
> Lock sounds good here. I can't picture a use case for trying to do lots of register/unregister calls from lots of threads. If that deadlocks immediately (which it might, it's not clear to me whether the control flow here can go back in and try to re-take the same lock) that's useful information to have.

It will deadlock with a non-recursive lock, essentially that's what was originally happening with `call_once`.


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

https://reviews.llvm.org/D142008