Skip to content
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

add entry point to construct an OpaqueClosure from pre-optimized IRCode #44197

Merged
merged 7 commits into from
Apr 28, 2022
Merged

add entry point to construct an OpaqueClosure from pre-optimized IRCode #44197

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4176f00
add entry point to construct an OpaqueClosure from pre-optimized IRCode
JeffBezanson Feb 16, 2022
c49662c
Merge branch 'master' into jb/ircode2oc
aviatesk Mar 25, 2022
c48e638
update `jl_new_codeinst` signature
aviatesk Mar 25, 2022
c5fae67
Merge branch 'master' into jb/ircode2oc
aviatesk Apr 5, 2022
691d310
fixes to OpaqueClosure argument count handling and MethodError display
JeffBezanson Apr 8, 2022
7aa4a7d
Merge branch 'master' into jb/ircode2oc
aviatesk Apr 12, 2022
6c22265
more test coverage
aviatesk Apr 12, 2022
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
8 changes: 4 additions & 4 deletions src/method.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ extern jl_value_t *jl_builtin_getfield;
extern jl_value_t *jl_builtin_tuple;

jl_method_t *jl_make_opaque_closure_method(jl_module_t *module, jl_value_t *name,
jl_value_t *nargs, jl_value_t *functionloc, jl_code_info_t *ci, int isva);
int nargs, jl_value_t *functionloc, jl_code_info_t *ci, int isva);

static void check_c_types(const char *where, jl_value_t *rt, jl_value_t *at)
{
Expand Down Expand Up @@ -102,7 +102,7 @@ static jl_value_t *resolve_globals(jl_value_t *expr, jl_module_t *module, jl_sve
if (!jl_is_code_info(ci)) {
jl_error("opaque_closure_method: lambda should be a CodeInfo");
}
jl_method_t *m = jl_make_opaque_closure_method(module, name, nargs, functionloc, (jl_code_info_t*)ci, isva);
jl_method_t *m = jl_make_opaque_closure_method(module, name, jl_unbox_long(nargs), functionloc, (jl_code_info_t*)ci, isva);
return (jl_value_t*)m;
}
if (e->head == jl_cfunction_sym) {
Expand Down Expand Up @@ -781,7 +781,7 @@ JL_DLLEXPORT jl_method_t *jl_new_method_uninit(jl_module_t *module)
// method definition ----------------------------------------------------------

jl_method_t *jl_make_opaque_closure_method(jl_module_t *module, jl_value_t *name,
jl_value_t *nargs, jl_value_t *functionloc, jl_code_info_t *ci, int isva)
int nargs, jl_value_t *functionloc, jl_code_info_t *ci, int isva)
{
jl_method_t *m = jl_new_method_uninit(module);
JL_GC_PUSH1(&m);
Expand All @@ -795,7 +795,7 @@ jl_method_t *jl_make_opaque_closure_method(jl_module_t *module, jl_value_t *name
assert(jl_is_symbol(name));
m->name = (jl_sym_t*)name;
}
m->nargs = jl_unbox_long(nargs) + 1;
m->nargs = nargs + 1;
assert(jl_is_linenode(functionloc));
jl_value_t *file = jl_linenode_file(functionloc);
m->file = jl_is_symbol(file) ? (jl_sym_t*)file : jl_empty_sym;
Expand Down
80 changes: 66 additions & 14 deletions src/opaque_closure.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,23 @@ JL_DLLEXPORT int jl_is_valid_oc_argtype(jl_tupletype_t *argt, jl_method_t *sourc
return 1;
}

jl_opaque_closure_t *jl_new_opaque_closure(jl_tupletype_t *argt, jl_value_t *rt_lb, jl_value_t *rt_ub,
jl_value_t *source_, jl_value_t **env, size_t nenv)
static jl_value_t *prepend_type(jl_value_t *t0, jl_tupletype_t *t)
{
jl_svec_t *sig_args = NULL;
JL_GC_PUSH1(&sig_args);
size_t nsig = 1 + jl_svec_len(t->parameters);
sig_args = jl_alloc_svec_uninit(nsig);
jl_svecset(sig_args, 0, t0);
for (size_t i = 0; i < nsig-1; ++i) {
jl_svecset(sig_args, 1+i, jl_tparam(t, i));
}
jl_value_t *sigtype = (jl_value_t*)jl_apply_tuple_type_v(jl_svec_data(sig_args), nsig);
JL_GC_POP();
return sigtype;
}

static jl_opaque_closure_t *new_opaque_closure(jl_tupletype_t *argt, jl_value_t *rt_lb, jl_value_t *rt_ub,
jl_value_t *source_, jl_value_t *captures)
{
if (!jl_is_tuple_type((jl_value_t*)argt)) {
jl_error("OpaqueClosure argument tuple must be a tuple type");
Expand All @@ -40,26 +55,19 @@ jl_opaque_closure_t *jl_new_opaque_closure(jl_tupletype_t *argt, jl_value_t *rt_
}
if (jl_nparams(argt) + 1 - jl_is_va_tuple(argt) < source->nargs - source->isva)
jl_error("Argument type tuple has too few required arguments for method");
jl_task_t *ct = jl_current_task;
jl_value_t *sigtype = NULL;
JL_GC_PUSH1(&sigtype);
sigtype = prepend_type(jl_typeof(captures), argt);

jl_value_t *oc_type JL_ALWAYS_LEAFTYPE;
oc_type = jl_apply_type2((jl_value_t*)jl_opaque_closure_type, (jl_value_t*)argt, rt_ub);
JL_GC_PROMISE_ROOTED(oc_type);
jl_value_t *captures = NULL, *sigtype = NULL;
jl_svec_t *sig_args = NULL;
JL_GC_PUSH3(&captures, &sigtype, &sig_args);
captures = jl_f_tuple(NULL, env, nenv);

size_t nsig = 1 + jl_svec_len(argt->parameters);
sig_args = jl_alloc_svec_uninit(nsig);
jl_svecset(sig_args, 0, jl_typeof(captures));
for (size_t i = 0; i < nsig-1; ++i) {
jl_svecset(sig_args, 1+i, jl_tparam(argt, i));
}
sigtype = (jl_value_t*)jl_apply_tuple_type_v(jl_svec_data(sig_args), nsig);
jl_method_instance_t *mi = jl_specializations_get_linfo(source, sigtype, jl_emptysvec);
size_t world = jl_atomic_load_acquire(&jl_world_counter);
jl_code_instance_t *ci = jl_compile_method_internal(mi, world);

jl_task_t *ct = jl_current_task;
jl_opaque_closure_t *oc = (jl_opaque_closure_t*)jl_gc_alloc(ct->ptls, sizeof(jl_opaque_closure_t), oc_type);
JL_GC_POP();
oc->source = source;
Expand All @@ -82,6 +90,50 @@ jl_opaque_closure_t *jl_new_opaque_closure(jl_tupletype_t *argt, jl_value_t *rt_
return oc;
}

jl_opaque_closure_t *jl_new_opaque_closure(jl_tupletype_t *argt, jl_value_t *rt_lb, jl_value_t *rt_ub,
jl_value_t *source_, jl_value_t **env, size_t nenv)
{
jl_value_t *captures = jl_f_tuple(NULL, env, nenv);
JL_GC_PUSH1(&captures);
jl_opaque_closure_t *oc = new_opaque_closure(argt, rt_lb, rt_ub, source_, captures);
JL_GC_POP();
return oc;
}

jl_method_t *jl_make_opaque_closure_method(jl_module_t *module, jl_value_t *name,
int nargs, jl_value_t *functionloc, jl_code_info_t *ci, int isva);

JL_DLLEXPORT jl_code_instance_t *jl_new_codeinst(
jl_method_instance_t *mi, jl_value_t *rettype,
jl_value_t *inferred_const, jl_value_t *inferred,
int32_t const_flags, size_t min_world, size_t max_world,
uint8_t ipo_effects, uint8_t effects, uint8_t relocatability);

JL_DLLEXPORT void jl_mi_cache_insert(jl_method_instance_t *mi JL_ROOTING_ARGUMENT,
jl_code_instance_t *ci JL_ROOTED_ARGUMENT JL_MAYBE_UNROOTED);

JL_DLLEXPORT jl_opaque_closure_t *jl_new_opaque_closure_from_code_info(jl_tupletype_t *argt, jl_value_t *rt_lb, jl_value_t *rt_ub,
jl_module_t *mod, jl_code_info_t *ci, int lineno, jl_value_t *file, int nargs, int isva, jl_value_t *env)
{
if (!ci->inferred)
jl_error("CodeInfo must already be inferred");
jl_value_t *root = NULL, *sigtype = NULL;
jl_code_instance_t *inst = NULL;
JL_GC_PUSH3(&root, &sigtype, &inst);
root = jl_box_long(lineno);
root = jl_new_struct(jl_linenumbernode_type, root, file);
root = (jl_value_t*)jl_make_opaque_closure_method(mod, jl_nothing, nargs, root, ci, isva);

sigtype = prepend_type(jl_typeof(env), argt);
jl_method_instance_t *mi = jl_specializations_get_linfo((jl_method_t*)root, sigtype, jl_emptysvec);
inst = jl_new_codeinst(mi, rt_ub, NULL, (jl_value_t*)ci, 0, ((jl_method_t*)root)->primary_world, -1, 0, 0, 0);
jl_mi_cache_insert(mi, inst);

jl_opaque_closure_t *oc = new_opaque_closure(argt, rt_lb, rt_ub, root, env);
JL_GC_POP();
return oc;
}

JL_CALLABLE(jl_new_opaque_closure_jlcall)
{
if (nargs < 4)
Expand Down
23 changes: 23 additions & 0 deletions test/opaque_closure.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,3 +230,26 @@ call_global_opaque_closure() = GLOBAL_OPAQUE_CLOSURE()
let oc = @opaque a->sin(a)
@test length(code_typed(oc, (Int,))) == 1
end

# constructing an opaque closure from IRCode
using Core.Compiler: IRCode
using Core: CodeInfo

function OC(ir::IRCode, env...)
Copy link
Member

Choose a reason for hiding this comment

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

This code seems to segfault in certain cases, e.g. I found:

julia> function kernel(n)
           r = 0
           for i = 1:n
               r += sum(sincos(i))
           end
           return r
       end
kernel (generic function with 1 method)

julia> let ci = first(@code_typed kernel(42))
           ir = Core.Compiler.inflate_ir(ci)
           OC(ir)
       end

signal (11): Segmentation fault: 11
in expression starting at REPL[5]:1
resolve_globals at /Users/aviatesk/julia/julia3/src/method.c:48
resolve_globals at /Users/aviatesk/julia/julia3/src/method.c:54
jl_method_set_source at /Users/aviatesk/julia/julia3/src/method.c:729
jl_make_opaque_closure_method at /Users/aviatesk/julia/julia3/src/method.c:804
jl_new_opaque_closure_from_code_info at /Users/aviatesk/julia/julia3/src/opaque_closure.c:126
OC at ./REPL[3]:11
unknown function (ip: 0x1169ac119)
_jl_invoke at /Users/aviatesk/julia/julia3/src/gf.c:0 [inlined]
ijl_apply_generic at /Users/aviatesk/julia/julia3/src/gf.c:2549
jl_apply at /Users/aviatesk/julia/julia3/src/./julia.h:1833 [inlined]
do_call at /Users/aviatesk/julia/julia3/src/interpreter.c:126
eval_stmt_value at /Users/aviatesk/julia/julia3/src/interpreter.c:166 [inlined]
eval_body at /Users/aviatesk/julia/julia3/src/interpreter.c:594
jl_interpret_toplevel_thunk at /Users/aviatesk/julia/julia3/src/interpreter.c:750
jl_toplevel_eval_flex at /Users/aviatesk/julia/julia3/src/toplevel.c:909
jl_toplevel_eval_flex at /Users/aviatesk/julia/julia3/src/toplevel.c:853
ijl_toplevel_eval at /Users/aviatesk/julia/julia3/src/toplevel.c:918 [inlined]
ijl_toplevel_eval_in at /Users/aviatesk/julia/julia3/src/toplevel.c:968
eval at ./boot.jl:370 [inlined]
eval_user_input at /Users/aviatesk/julia/julia3/usr/share/julia/stdlib/v1.9/REPL/src/REPL.jl:151
repl_backend_loop at /Users/aviatesk/julia/julia3/usr/share/julia/stdlib/v1.9/REPL/src/REPL.jl:246
start_repl_backend at /Users/aviatesk/julia/julia3/usr/share/julia/stdlib/v1.9/REPL/src/REPL.jl:231
#run_repl#47 at /Users/aviatesk/julia/julia3/usr/share/julia/stdlib/v1.9/REPL/src/REPL.jl:368
run_repl at /Users/aviatesk/julia/julia3/usr/share/julia/stdlib/v1.9/REPL/src/REPL.jl:355
jfptr_run_repl_64436 at /Users/aviatesk/julia/julia3/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/aviatesk/julia/julia3/src/gf.c:0 [inlined]
ijl_apply_generic at /Users/aviatesk/julia/julia3/src/gf.c:2549
#965 at ./client.jl:419
jfptr_YY.965_28005 at /Users/aviatesk/julia/julia3/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/aviatesk/julia/julia3/src/gf.c:0 [inlined]
ijl_apply_generic at /Users/aviatesk/julia/julia3/src/gf.c:2549
jl_apply at /Users/aviatesk/julia/julia3/src/./julia.h:1833 [inlined]
jl_f__call_latest at /Users/aviatesk/julia/julia3/src/builtins.c:769
#invokelatest#2 at ./essentials.jl:729 [inlined]
invokelatest at ./essentials.jl:727 [inlined]
run_main_repl at ./client.jl:404
exec_options at ./client.jl:318
_start at ./client.jl:518
jfptr__start_61570 at /Users/aviatesk/julia/julia3/usr/lib/julia/sys.dylib (unknown line)
_jl_invoke at /Users/aviatesk/julia/julia3/src/gf.c:0 [inlined]
ijl_apply_generic at /Users/aviatesk/julia/julia3/src/gf.c:2549
jl_apply at /Users/aviatesk/julia/julia3/src/./julia.h:1833 [inlined]
true_main at /Users/aviatesk/julia/julia3/src/jlapi.c:567
jl_repl_entrypoint at /Users/aviatesk/julia/julia3/src/jlapi.c:711
Allocations: 2882378 (Pool: 2880618; Big: 1760); GC: 3

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, resolve_globals just needs to handle ReturnNode with an undefined field, which only occurs in optimized IR. Or even better, we could pass a flag through to prevent it from calling resolve_globals in this case, since that should only be needed by the front end.

src = ccall(:jl_new_code_info_uninit, Ref{CodeInfo}, ());
src.slotflags = UInt8[]
nargs = length(ir.argtypes)
Copy link
Member

Choose a reason for hiding this comment

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

length(ir.argtypes) doesn't necessarily correspond to the # of arguments of method from which this ir is constructed. IIUC that information disappeared within CodeInfo/IRCode. I think this entry point should take nargs::Int manually?

src.slotnames = fill(:none, nargs)
Core.Compiler.replace_code_newstyle!(src, ir, nargs);
Core.Compiler.widen_all_consts!(src);
src.inferred = true
# NOTE: we need ir.argtypes[1] == typeof(env)

ccall(:jl_new_opaque_closure_from_code_info, Any, (Any, Any, Any, Any, Any, Cint, Any, Cint, Cint, Any),
Tuple{ir.argtypes[2:end]...}, Union{}, Any, @__MODULE__, src, 0, nothing, nargs-1, false, env)
Copy link
Member

Choose a reason for hiding this comment

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

IIRC we had said that we did want to separate the method construction from the actual opaque closure creation. I don't need it immediately, but I think it would be perfectly sensible to construct several opaque closures with the same inferred code by different envs (after all, otherwise, you could have just constproped env at inference time.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed, just starting with the minimal API surface.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually it's a bit tricky, since with this there are two kinds of methods, one that can be specialized as normal, and one that is already inferred. If the same OpaqueClosure constructor can accept both, it needs some additional checks.

Copy link
Member

Choose a reason for hiding this comment

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

I think we are always permitted to specialize methods in order to do non-observable optimizations (e.g. the normal inference pipeline) on anything we compile, or we couldn't really pass them to codegen either (which also runs various compilation and inference stages). Only the other syntax form has the ability to create partial specializations at a call-site and optimize the env on those (while preserving the opaque characteristic).

end

let ci = code_typed(+, (Int, Int))[1][1]
ir = Core.Compiler.inflate_ir(ci)
@test OC(ir)(40, 2) == 42
end