Skip to content

Make cfunction optmization in codegen valid #19801

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.

Sign up for GitHub

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

Closed
yuyichao wants to merge 1 commit into from
Closed

Make cfunction optmization in codegen valid #19801

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions base/inference.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,10 @@ function abstract_call(f::ANY, fargs, argtypes::Vector{Any}, vtypes::VarTable, s
end
end
end
if istopfunction(tm, f, :cfunction) && length(fargs) == 4
# This can be skipped if codegen cannot inline the call
update_valid_age!(sv.params.world, sv.params.world, sv)
end

atype = argtypes_to_type(argtypes)
t = pure_eval_call(f, argtypes, atype, vtypes, sv)
Expand Down
2 changes: 1 addition & 1 deletion src/ccall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1635,7 +1635,7 @@ static jl_cgval_t emit_ccall(jl_value_t **args, size_t nargs, jl_codectx_t *ctx)
frt = jl_tparam0(frt);
Value *llvmf = NULL;
JL_TRY {
llvmf = jl_cfunction_object((jl_function_t*)f, frt, (jl_tupletype_t*)fargt);
llvmf = jl_cfunction_object((jl_function_t*)f, frt, (jl_tupletype_t*)fargt, ctx->world);
Copy link
Copy Markdown
Member

@vtjnash vtjnash Jan 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can also check here that min_age == max_age, to see if the backedge is right

}
JL_CATCH {
llvmf = NULL;
Expand Down
15 changes: 9 additions & 6 deletions src/codegen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1225,17 +1225,19 @@ jl_generic_fptr_t jl_generate_fptr(jl_method_instance_t *li, void *_F, size_t wo
return fptr;
}

static Function *jl_cfunction_object(jl_function_t *f, jl_value_t *rt, jl_tupletype_t *argt);
static Function *jl_cfunction_object(jl_function_t *f, jl_value_t *rt, jl_tupletype_t *argt, size_t world);
// get the address of a C-callable entry point for a function
extern "C" JL_DLLEXPORT
void *jl_function_ptr(jl_function_t *f, jl_value_t *rt, jl_value_t *argt)
{
jl_ptls_t ptls = jl_get_ptls_states();
JL_GC_PUSH1(&argt);
if (jl_is_tuple(argt)) {
// TODO: maybe deprecation warning, better checking
argt = (jl_value_t*)jl_apply_tuple_type_v((jl_value_t**)jl_data_ptr(argt), jl_nfields(argt));
}
Function *llvmf = jl_cfunction_object(f, rt, (jl_tupletype_t*)argt);
Function *llvmf = jl_cfunction_object(f, rt, (jl_tupletype_t*)argt,
ptls->world_age);
Copy link
Copy Markdown
Member

@vtjnash vtjnash Jan 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm. Is this actually right? Now that I think back on some of my original work on cfunction, I think I may have contemplated they should always appear to operate in the newest world. And that it would update the JIT code as needed to make that true. Clearly I forgot about that plan as I finished up the PR though last year. But it's a new year here now!

Copy link
Copy Markdown
Contributor Author

@yuyichao yuyichao Jan 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's very important to make sure that we can still do the same cfunction optimization that we are doing now, i.e. cfunction on known types/functions should return a constant (or relocated) pointer. How can you do it if cfunction always returns the function in the latest world?

JL_GC_POP();
return (void*)getAddressForFunction(llvmf);
}
Expand All @@ -1255,8 +1257,10 @@ void *jl_function_ptr_by_llvm_name(char *name) {
extern "C" JL_DLLEXPORT
void jl_extern_c(jl_function_t *f, jl_value_t *rt, jl_value_t *argt, char *name)
{
jl_ptls_t ptls = jl_get_ptls_states();
assert(jl_is_tuple_type(argt));
Function *llvmf = jl_cfunction_object(f, rt, (jl_tupletype_t*)argt);
Function *llvmf = jl_cfunction_object(f, rt, (jl_tupletype_t*)argt,
ptls->world_age);
if (llvmf) {
// force eager emission of the function (llvm 3.3 gets confused otherwise and tries to do recursive compilation)
uint64_t Addr = getAddressForFunction(llvmf);
Expand Down Expand Up @@ -3739,7 +3743,7 @@ static Function *gen_cfun_wrapper(jl_function_t *ff, jl_value_t *jlrettype, jl_t
ctx.f = cw;
ctx.linfo = lam;
ctx.code = NULL;
ctx.world = jl_world_counter;
ctx.world = world;
ctx.sret = false;
ctx.spvals_ptr = NULL;
ctx.params = &jl_default_cgparams;
Expand Down Expand Up @@ -4000,7 +4004,7 @@ const struct jl_typemap_info cfunction_cache = {
// Get the LLVM Function* for the C-callable entry point for a certain function
// and argument types.
// here argt does not include the leading function type argument
static Function *jl_cfunction_object(jl_function_t *ff, jl_value_t *declrt, jl_tupletype_t *argt)
static Function *jl_cfunction_object(jl_function_t *ff, jl_value_t *declrt, jl_tupletype_t *argt, size_t world)
{
// validate and unpack the arguments
JL_TYPECHK(cfunction, type, declrt);
Expand Down Expand Up @@ -4051,7 +4055,6 @@ static Function *jl_cfunction_object(jl_function_t *ff, jl_value_t *declrt, jl_t
cfunc_sig = (jl_value_t*)jl_apply_tuple_type((jl_svec_t*)cfunc_sig);

// check the cache
size_t world = jl_world_counter;
if (jl_cfunction_list.unknown != jl_nothing) {
jl_typemap_entry_t *sf = jl_typemap_assoc_by_type(jl_cfunction_list, (jl_tupletype_t*)cfunc_sig, NULL, 1, 0, /*offs*/0, world);
if (sf) {
Expand Down
23 changes: 23 additions & 0 deletions test/misc.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,29 @@ let
@test c_18711 == 1
end

g19801(a) = a
f19801() = cfunction(g19801, Int, (Int,))
f19801_dynamic = f19801
# Run a single function twice to make sure the function is recompiled
function test19801(first, ptr0=f19801())
if first
@test ccall(ptr0, Int, (Int,), 1) == 1
@eval g19801(a::Int) = a + 1
# Check that both static and dynamic version returns the old pointer
@test f19801() == ptr0
@test f19801_dynamic() == ptr0
@test ccall(f19801(), Int, (Int,), 1) == 1
else
@test ccall(ptr0, Int, (Int,), 1) == 1
@test f19801() != ptr0
@test f19801_dynamic() == f19801()
@test ccall(f19801(), Int, (Int,), 1) == 2
end
return ptr0
end
ptr19801 = test19801(true)
test19801(false, ptr19801)

let
old_have_color = Base.have_color
try
Expand Down