-
-
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
Improve codegen for Core.throw_methoderror
and Core.current_scope
#55803
Conversation
I'm not sure why we don't require these addresses to be available for all builtins, but this mildly improves our codegen for these. ```julia julia> foo() = Core.current_scope() ``` Before: ```llvm define nonnull ptr @julia_foo_2488() #0 { top: %0 = call ptr @jl_get_builtin_fptr(ptr nonnull @"+Core.#current_scope#2491.jit") %Builtin_ret = call nonnull ptr %0(ptr nonnull @"jl_global#2492.jit", ptr null, i32 0) ret ptr %Builtin_ret } ``` After: ```llvm define nonnull ptr @julia_foo_1066() #0 { top: %jl_f_current_scope_ret = call nonnull ptr @jl_f_current_scope(ptr null, ptr null, i32 0) ret ptr %jl_f_current_scope_ret } ```
@vtjnash Do you know why we don't just make |
We certainly could. Just hasn't been done by anyone yet |
We could also do better to inline that pointer, since we don't need to call |
Which CodeInstance? |
The one in |
Ah right - but if we make |
Current scope should just be codegened to lower to just getting the scope out of the task. The fact it's a builtin is slightly annoying but it's fine. Lines 6789 to 6793 in 0073917
|
Core.throw_methoderror
and Core.current_scope
Core.throw_methoderror
and Core.current_scope
Done 👍 |
09174f1
to
12e4999
Compare
This slightly improves our (LLVM) codegen for
Core.throw_methoderror
andCore.current_scope
Before:
After: