make ccallable work with system images and precompilation
#35574
Conversation
cfff392
to
8c5313d
Compare
src/dump.c
Outdated
| @@ -3266,6 +3276,12 @@ static jl_value_t *_jl_restore_incremental(ios_t *f, jl_array_t *mod_array) | |||
| arraylist_free(tracee_list); | |||
| free(tracee_list); | |||
| } | |||
| for(int i = 0; i < ccallable_list.len; i++) { | |||
There was a problem hiding this comment.
| for(int i = 0; i < ccallable_list.len; i++) { | |
| for (int i = 0; i < ccallable_list.len; i++) { |
src/jitlayers.cpp
Outdated
| for (i = 1; i < nargs; i++) { | ||
| jl_value_t *ati = jl_tparam(sigt, i); | ||
| if (jl_is_abstract_ref_type(ati)) { | ||
| ati = jl_tparam0(ati); | ||
| if (jl_is_typevar(ati)) | ||
| jl_error("@ccallable: argument type Ref should have an element type, not Ref{<:T}"); | ||
| } | ||
| if (jl_is_pointer(ati) && jl_is_typevar(jl_tparam0(ati))) | ||
| jl_error("@ccallable: argument type Ptr should have an element type, Ptr{<:T}"); | ||
| } |
There was a problem hiding this comment.
Ah, I see this design is just handling all of the Ref declared signatures incorrectly. It seems part also of why they can't appropriately be attached to a single Method.
There was a problem hiding this comment.
Yeah, I would be fine with not supporting Ref at all here --- no julia values, no by-reference, only simple types you'd write directly in C code. That would make a lot of this easier.
| { | ||
| // validate arguments. try to do as many checks as possible here to avoid | ||
| // throwing errors later during codegen. | ||
| JL_TYPECHK(@ccallable, type, declrt); |
There was a problem hiding this comment.
Since this function is named ccallable, I don't think the @ is really a part of the name.
There was a problem hiding this comment.
The user-facing API is @ccallable. I'll add an underscore to ccallable to make that clear.
|
Review comments hopefully addressed now. For now I'm just going to be strict about what types are allowed, and later we can see if that can be leveraged to simplify the compilation flow. |
Co-authored-by: Jameson Nash <[email protected]>
|
Also fixes #34076 I guess? |
fixes #35014
The approach I took is to add a field to
Methodstoring the needed return type and signature if a C entry point is requested for that method.jl_compile_extern_cis called to inform the JIT during loading, or after loading a .ji or system image. The signatures are also added to the data passed tojl_create_nativeto tell it to include aliases in the system image.A possible simplification is to require the argument types to be specific enough to be specialized. Currently, we allow
@ccallable f(x::Any) = ..., in which case we need to generate a wrapper accepting any value and doing dynamic dispatch. If we required specific types, e.g.f(x::Cint), then every ccallable would be associated with a single MethodInstance, and we'd just generate the alias whenever the instance is compiled.It's not clear whether this feature is needed in the JIT, or just system images (i.e. only when you want to link a C program with direct calls against sys.so). The JIT case takes a bunch of the code here.
A separate problem to fix in the near future is that I believe ccallable and possibly also cfunction are always using jl_apply_generic when compiled into a system image, since
gen_cfun_wrapperdoesn't use the new mechanism for looking up call targets (there is already a TODO comment about the recursive compilation done there).