-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
remove more global state from codegen #36260
Conversation
52a9119
to
b5cff71
Compare
Not sure if exactly the same bug as https://build.julialang.org/#/builders/48/builds/527 but I was getting an inconsistent error like that during builds and "solved" it (temporarily worked around it) by adding a '\0' to the end of the numbered function names. It looks like somewhere explicit-length strings are not getting converted correctly to null terminated string where required for native function names. |
b5cff71
to
cb993f3
Compare
Yes, it is that bug, and is caused by the unpredictability of the ordering of running finalizers. It is being fixed here. |
I'll plan to merge this tomorrow. Though review would be good (@JeffBezanson @Keno or @maleadt perhaps). |
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.
As discussed this morning, needs to be adjusted to avoid breaking the GPU stack (at least for the moment).
…unctions We already had prepare_global/prepare_call functions. This simply lets us use that to allocate these on-demand instead of keeping around a global copy of them to template from.
cb993f3
to
26de664
Compare
Annoyed by https://build.julialang.org/#/builders/48/builds/527, I decided to spend some time removing more global state from codegen and wrapping it into our jl_codegen_params_t context.
While we still have some work to do to remove the global use of many Type objects, and to remove the global counter (we'll need to figure out a different mechanism when merging modules to identify unique values), this is another step in that direction.