search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.3k stars 11.69k forks source link

Is PassRegistry still not thread-safe? #94154

Open falhumai96 opened 4 months ago

falhumai96 commented 4 months ago

https://llvm.org/doxygen/classllvm_1_1PassRegistry.html#details

I'm looking at the details for this class, and I saw a note saying that this class is not thread-safe. However, looking at the class declaration in the headr file in private member fields and also looking at the .CPP file for the implementation, I noticed that all methods are being locked by a reader-writer lock. So, is the note outdated or does it still hold that statement?

The .CPP implementation for LLVM 19: https://llvm.org/doxygen/PassRegistry_8cpp_source.html for reference.

Also, speaking of which, I have a question regarding LLVM initialization (which internally they call PassRegistry). I understand that llvm initialization functions (e.g. initializeNativeTarget) must be called before any other llvm functions (e.g. JIT related code) and I understand that they can be called multiple times by the calling clients, but are the initialization functions thread-safe themselves? Meaning, can two threads call these functions safely, or are they only allowed to be called by one parent thread (e.g. the main thread)?

falhumai96 commented 4 months ago

Just to add to the thread, this comment was added by this commit when the mutex lock was removed and replaced with pImpl managed static: https://github.com/llvm/llvm-project/commit/a74fa15f320fe999a682eb81fd902478aea3351b, which the note made sense (I believe the whole PassRegistry needed to be locked to ensure synchronization, but this situation seems to be reverted back (changing back pImpl to RW locks): https://github.com/llvm/llvm-project/commit/0921f6bdb85e4e6ae15f2c0bbd41a45e4f8f66dd). So, my intuition says that this comment might outdated but I could be mistaken.