-
Notifications
You must be signed in to change notification settings - Fork 58
Fix deferred_codegen registration #711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Your PR requires formatting changes to meet the project's style guidelines. Click here to view the suggested changes.diff --git a/src/GPUCompiler.jl b/src/GPUCompiler.jl
index ec33e9a..8f2cf75 100644
--- a/src/GPUCompiler.jl
+++ b/src/GPUCompiler.jl
@@ -66,7 +66,7 @@ function __init__()
global compile_cache = dir
Tracy.@register_tracepoints()
- register_deferred_codegen()
+ return register_deferred_codegen()
end
end # module
diff --git a/src/driver.jl b/src/driver.jl
index 8dbff91..7b0c318 100644
--- a/src/driver.jl
+++ b/src/driver.jl
@@ -154,13 +154,15 @@ end
# Called from __init__
# On 1.11+ this is needed due to a Julia bug that drops the pointer when code-coverage is enabled.
function register_deferred_codegen()
- @dispose jljit=JuliaOJIT() begin
+ @dispose jljit = JuliaOJIT() begin
jd = JITDylib(jljit)
address = LLVM.API.LLVMOrcJITTargetAddress(
- reinterpret(UInt, @cfunction(deferred_codegen, Ptr{Cvoid}, (Ptr{Cvoid},))))
+ reinterpret(UInt, @cfunction(deferred_codegen, Ptr{Cvoid}, (Ptr{Cvoid},)))
+ )
flags = LLVM.API.LLVMJITSymbolFlags(
- LLVM.API.LLVMJITSymbolGenericFlagsExported, 0)
+ LLVM.API.LLVMJITSymbolGenericFlagsExported, 0
+ )
name = mangle(jljit, "deferred_codegen")
symbol = LLVM.API.LLVMJITEvaluatedSymbol(address, flags)
map = if LLVM.version() >= v"15" |
|
With this change: Before this change: |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #711 +/- ##
==========================================
+ Coverage 75.27% 75.43% +0.15%
==========================================
Files 24 24
Lines 3523 3537 +14
==========================================
+ Hits 2652 2668 +16
+ Misses 871 869 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Gabriel Baraldi <[email protected]>
9ee6efb to
5e2a80d
Compare
| LLVM.define(jd, mu) | ||
| addr = lookup(jljit, "deferred_codegen") | ||
| @assert addr != C_NULL "Failed to register deferred_codegen" | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we ccall it here just to check if it's working? I guess the address is enough but not sure. I guess the test is enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I think the ccall would not add more information, and if failed lead to a more confusing error.
|
This isn't pretty, but it's also not very different from what was there before. |
According to @gbaraldi this was running into issues on 1.11 when code-coverage was enable (since Base dropped the registration of the ccallable)
and on 1.12 this is going to be broken by JuliaLang/julia#56987