-
Notifications
You must be signed in to change notification settings - Fork 58
Enable users to map from GV to Julia value #746
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #746 +/- ##
==========================================
- Coverage 75.28% 74.49% -0.79%
==========================================
Files 24 24
Lines 3670 3682 +12
==========================================
- Hits 2763 2743 -20
- Misses 907 939 +32 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
|
||
| @tracepoint "IR generation" begin | ||
| ir, compiled = irgen(job) | ||
| ir, compiled, gv_to_value = irgen(job) |
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.
I am open to a better name
| if haskey(asm[2], :gv_to_value) | ||
| # TODO: Serializer cannot handle Core.IntrinsicFunction | ||
| # We kinda want Julia to serialize the values in `gv_to_value` in the pkgimg and us just having to store an index | ||
| # for now we just empty them out | ||
| # We would need to remove the initializers from LLVM IR as well to be correct, and then link these in at runtime | ||
| empty!(asm[2].gv_to_value) |
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.
We should likely just give up on caching this for now if !isempty(gv_to_value)
For proper caching, we would want to create an IdDict in the current parent module and instead of saving values we would want to save "offsets/keys" into that IdDict so that Julia performs the pkgimg reolcation for us.
| return list | ||
| end | ||
| 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.
😱 For backwards compatibility we need to support the internal arraylist_t format.
| if ptr in gvalues | ||
| gv_to_value[LLVM.name(gv)] = ptr | ||
| end | ||
| LLVM.initializer!(gv, nothing) |
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.
So we don't necessarily need to delete the initializer here, but it makes the code a bit more consistent.
Two fold motivation.
Currently, we perform the JIT favourite trick on all globals, we inline the runtime pointers, this makes the resulting code non-relocatable. A concrete example of that would be something like:
The memory location of
:xchanges upon session restart (as do other things like Types, Singletons), and for objects like this Julia performs a simple pointer comparison. So we can actually support this operation on the GPU.Yet, we cannot cache the resulting kernel, or if we do, we will perform the wrong computation. So what we must do instead is to initialize the global after loading it from cache.
Now that is a tricky affair (backend dependent), we can do it on LLVM IR, I believe AMDGPU could also do it, the CPU backends can use something like
JITDylib, but I don't think we can do it on the obj file from Nvidia, and I don't about other backends.So we should likely mark compilations that use globals with runtime pointers as ineligible for caching.
The second motivation stems from Enzyme where we would like to have a mapping from global to Julia object, and it can be hard to figure out which globals are Julia objects and which ones are not.