-
-
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
use interpreter when possible in --compile=no
mode
#24473
Conversation
265ae2b
to
50e8e83
Compare
make `JULIACODEGEN=none` builds work again
50e8e83
to
4a91985
Compare
@@ -5581,8 +5581,6 @@ let x5 = UnionField5(nothing, Int8(3)) | |||
@test x5 == x5copy | |||
@test object_id(x5) === object_id(x5copy) | |||
@test hash(x5) === hash(x5copy) | |||
@test pointer_from_objref(x5) === pointer_from_objref(x5) | |||
@test pointer_from_objref(x5) !== pointer_from_objref(x5copy) |
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.
The second test is always valid. Why isn't the first one working? It should be even easier for the interpreter than codegen.
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.
The first test does work, but I removed it since we had talked about disallowing pointer_from_objref
on immutables. In the second test, deepcopy
returns its argument for isbits objects, and the compiler re-boxed it but the interpreter didn't.
757c83f
to
4288b0e
Compare
@Keno There seems to be some preprocessor issue on win32. |
Yeah, I saw, will fix. |
I've pushed those fixes as a separate commit here in case you have local work. Should make rebasing easier (or just fetch and cherry-pick onto your branch before pushing). |
I can't reproduce the win32 failure locally. Works fine for me (under wine at least). Guess I'll spin up a VM and try there as well. |
base/error.jl
Outdated
deleteat!(bt, 1:3) | ||
unshift!(bt2) | ||
else | ||
deleteat!(bt, 1:2) |
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.
@JeffBezanson this is what's causing the win32 failure. On win32 the caller of this function is the topmost frame returned from backtrace_from_here.
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.
Ok, so to summarize my findings, on win32, RtlCaptureContext get's the caller's caller's ebp/eip |
@JeffBezanson I'll leave it to you how you want to resolve that. Basically win32 is not getting a frame for jl_backtrace_from here. |
- `isdefined` on static parameter - stack overflow when interpreting a block with many exception handlers (as in some tests) due to never popping jmp_bufs - fix backtraces for non-top-level interpreted frames - remove invalid `pointer_from_objref(immutable)` test, where interpreter and compiler had different boxing behavior
Makes changing the interpreter state struct less error prone
b6b8a9f
to
f60d438
Compare
!strncmp(fname, "jlcall_", 7) || // jlcall abi 1 from JIT wrapping a specsig method | ||
!strncmp(fname, "jlsysw_", 7)); // jlcall abi 1 from sysimg wrapping a specsig method | ||
return JL_API_GENERIC; | ||
} |
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.
This isn't static data, it's compiler.cpp data. Why did this move?
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 builds that exclude codegen. It's such a simple function we ought to be able to use it without llvm. I put it here because this file uses it.
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 compiles that don't have a JIT, why would they care what the JIT would do if it was present?
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.
Fair enough; my only goal here is to solve a linking problem. Another solution might be to separate reading and writing staticdata, since all builds need to read it but only builds with codegen need to write it. That would be more disruptive though.
@@ -1563,7 +1547,9 @@ void *jl_get_llvmf_decl(jl_method_instance_t *linfo, size_t world, bool getwrapp | |||
} | |||
|
|||
// compile this normally | |||
jl_llvm_functions_t decls = jl_compile_for_dispatch(&linfo, world); | |||
if (linfo->inferred == NULL) | |||
linfo->inferred = jl_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.
It's generally invalid to change the ->inferred
field, only jl_set_method_inferred
should be doing that.
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.
Would it be better to handle this in jl_compile_linfo
? It's not clear to me why jl_compile_linfo
does goto locked_out
if li->inferred
is NULL.
Also make
JULIACODEGEN=none
builds work again.With this,
./julia --compile=no
gives a fun low-latency version of julia. It will run everything in a (very slow) interpreter except code withforeigncall
(or llvmcall). Next steps are to merge #23973, support some common ccalls in the interpreter (in practice, many call known run-time system functions), and work out ways to interpret some functions automatically (e.g. interpret functions with no loops until they're called some number of times).