-
-
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
runtime versions of Intrinsics.reinterpret #13485
Conversation
…to have "reinterpret" do runtime error checking (fix #12832)
…other than libLLVMSupport.a for APInt support)
@@ -0,0 +1,486 @@ | |||
#include "llvm-version.h" |
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.
need license
switch (f) { | ||
case ccall: return emit_ccall(args, nargs, ctx); | ||
case cglobal: return emit_cglobal(args, nargs, ctx); | ||
case llvmcall: return emit_llvmcall(args, nargs, ctx); | ||
|
||
HANDLE(pointerref,2) | ||
#if 0 |
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.
what's this about?
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.
you can flip this switch and codegen will only use runtime intrinsics, never inline / optimized :O
it's a useful mode for showing that the runtime versions pass all of the julia tests, but its rather slow (it ends up doing too much re-boxing due to the current codegen design).
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 see, that makes sense. Is this worthy of a named option define? and/or an explanatory comment...
reduces runtime lookup effort. but the real reason is that it is a workaround because of the replacing of Core.Inference
runtime versions of Intrinsics.reinterpret
okay if I commit https://gist.github.com/tkelman/0475c5c9c86ada3e7b36 ? MSVC is strange. |
jl_value_t *jl_static_eval(jl_value_t *ex, void *ctx_, jl_module_t *mod, | ||
jl_value_t *sp, jl_expr_t *ast, int sparams, int allow_alloc) | ||
{ | ||
return NULL; |
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.
Seems like it should be possible to always include a useful version of this function.
Cool! It looks like |
Long term, yes. Or make them all Function with this as the fptr and the id in the env (no linfo). It's all a bit dependent on what happens in the jb/functions branch |
Fix compilation under MSVC after #13485
this implements runtime versions of all of the julia intrinsics (so that functions that use them don't need to be compiled to be run). it additionally includes a
Core.Intrinsics.intrinsic_call
invoke point for calling any of these runtime intrinsics. and it adds the name of the intrinsic tojl_
printing.additionally, It implements full error checking for the reinterpret/box/unbox intrinsic (fixes #12832), falling back to the the runtime version when necessary.
finally, it re-organizes some of the julia codegen architecture so that it is possible to link a version of libjulia that does not include a copy of LLVM (although I don't recommend trying, since the runtime currently can't get very far without the JIT).
(marked as wip simply because I'm still working on this branch, not because it couldn't be merged)