Conversation
|
For the remaining issues: Ref llvm/llvm-project@0bfa0bc and llvm/llvm-project@76d5a79#diff-889679ec233591bf43379efae9b68a4e901c003f4c3f5872c6e15bf261bc5572 |
|
For the eh frame registration issue, it seems that the ehframe register plugin doesn't have any customization hooks anymore so maybe we need to create our own plugin instead? For the ThreadSafeContext issue, I currently worked around it by patching llvm. The usage patter is more restricted with the new API and I'm not 100% sure yet if the new API covers all the needs here. |
30b9363 to
ca8ce88
Compare
c53adce to
2961412
Compare
|
EHFrame registrer should be fixed now and the only remaining issue is the API change for ThreadSafeContext. I'm convinced that it is not possible to support the new API as-is without getting rid of The comment suggests that |
|
Here's the LLVM patch I used to get julia and LLVM.jl to build/load JuliaLang/llvm-project#51 . |
|
Actually I take that back. The |
|
The early release of thread safe context in add_output is now fixed so this should be everything for dealing with C++ API change. The implementation could probably be cleaner with C++20 co_routine since this is more-or-less a manual implementation of it... |
22dec88 to
bdc92a9
Compare
|
I moved all the change that doesn't require a version check to other PR's. (#59965 is included due to otherwise merge conflict). I assume the first few changes should be reasonably clear and uncontroversial (those are part of the original PR). I can split that out if needed as well. |
We already need to manually create a C compatible function pointer and heap allocated data, might as well do so directly without another indirection through std::function. Noticed the callback during #59946. It does not need to be an non static `extern "C"` function and could even be a lambda.
e691651 to
2a87b5d
Compare
Split out of #59946 to make it hopefully easier to review.
Turn `add_output` into a manual co-routine so that we can get out of the caller scope mid-call to release the threadsafecontext lock early.
LLVM 21 removes `ThreadSafeContext::getLock()`, `::Lock`, and `::getContext()`, replacing them with callback-based `ThreadSafeContext::withContextDo(callback)`. Add a `withContextDo` compatibility shim that works on both LLVM 20 (acquires lock + calls callback) and LLVM 21+ (delegates to the new API). Update all callsites to use scoped locking through this shim instead of manual lock/unlock. Key changes: - `jl_codegen_output_t` no longer holds a persistent `tsctx_lock`. All context access goes through `withContextDo`, with debug asserts ensuring `get_context()`/`get_module()` are only called inside the callback scope. - `addOutput()` and `R->replace()` are called outside `withContextDo` to respect the lock hierarchy (TSC locks must be released before dispatching materialization units). - `jl_create_ts_module` and `compileModule` use the free-function shim. - `disasm.cpp` replaces `std::optional<ThreadSafeContext::Lock>` with the callback pattern. Supersedes #59946. Co-Authored-By: yuyichao <yyc1992@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
LLVM 21 removes `ThreadSafeContext::getLock()`, `::Lock`, and `::getContext()`, replacing them with callback-based `ThreadSafeContext::withContextDo(callback)`. Add a `withContextDo` compatibility shim that works on both LLVM 20 (acquires lock + calls callback) and LLVM 21+ (delegates to the new API). Update all callsites to use scoped locking through this shim instead of manual lock/unlock. Key changes: - `jl_codegen_output_t` no longer holds a persistent `tsctx_lock`. All context access goes through `withContextDo`, with debug asserts ensuring `get_context()`/`get_module()` are only called inside the callback scope. - `addOutput()` and `R->replace()` are called outside `withContextDo` to respect the lock hierarchy (TSC locks must be released before dispatching materialization units). - `jl_create_ts_module` and `compileModule` use the free-function shim. - `disasm.cpp` replaces `std::optional<ThreadSafeContext::Lock>` with the callback pattern. Supersedes #59946. Co-Authored-By: yuyichao <yyc1992@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
LLVM 21 removes `ThreadSafeContext::getLock()`, `::Lock`, and `::getContext()`, replacing them with callback-based `ThreadSafeContext::withContextDo(callback)`. Add a `withContextDo` compatibility shim that works on both LLVM 20 (acquires lock + calls callback) and LLVM 21+ (delegates to the new API). Update all callsites to use scoped locking through this shim instead of manual lock/unlock. Key changes: - `jl_codegen_output_t` no longer holds a persistent `tsctx_lock`. All context access goes through `withContextDo`, with debug asserts ensuring `get_context()`/`get_module()` are only called inside the callback scope. - `addOutput()` and `R->replace()` are called outside `withContextDo` to respect the lock hierarchy (TSC locks must be released before dispatching materialization units). - `jl_create_ts_module` and `compileModule` use the free-function shim. - `disasm.cpp` replaces `std::optional<ThreadSafeContext::Lock>` with the callback pattern. Supersedes #59946. Co-Authored-By: yuyichao <yyc1992@gmail.com> Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
## Summary - Add `withContextDo` compatibility shim for `ThreadSafeContext` API changes in LLVM 21 (which removes `getLock()`, `Lock`, and `getContext()` in favor of callback-based `withContextDo`) - Replace all manual lock/unlock patterns with scoped `withContextDo` callbacks across `jitlayers.h`, `jitlayers.cpp`, `aotcompile.cpp`, and `disasm.cpp` - Add `JLEHFrameRegistrationPlugin` for LLVM 21+ (which removes `jitlink::EHFrameRegistrar`), using JITLink allocation actions for EH frame registration - Debug asserts ensure `get_context()`/`get_module()` are only called inside `withContextDo` scope - Lock hierarchy respected: TSC locks released before `addOutput()`/`R->replace()` which can trigger materialization Supersedes #59946 and #60358, forward-porting those changes to current master. ## Test plan - [x] Builds and passes smoke test on LLVM 20 - [x] Builds and passes smoke test on LLVM 21 (tested with libLLVM 21.1.2 JLL) - [ ] CI This pull request was written with the assistance of generative AI (Claude). 🤖 Generated with [Claude Code](https://claude.com/claude-code) There is quite a few whitespace changes so ignoring it is quite helpful --------- Co-authored-by: yuyichao <yyc1992@gmail.com> Co-authored-by: Sam Schweigel <sam.schweigel@juliahub.com>
|
Were all changes here incorporated into other PRs, or is there anything missing? |
Remaining issues are the removal of EHFrameRegistrar class and manual locking interface for ThreadSafeContext. Some other deprecation warnings are also left as is for now.