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.

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

Conversation

JeffBezanson
Copy link
Member

No description provided.

# 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).

@KristofferC KristofferC added the backport 1.8 Change should be backported to release-1.8 label Feb 16, 2022
@KristofferC KristofferC mentioned this pull request Feb 24, 2022
47 tasks
@KristofferC KristofferC mentioned this pull request Mar 23, 2022
22 tasks
@aviatesk
Copy link
Member

Updated to take in #43800.

function OC(ir::IRCode, env...)
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?

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.

@KristofferC KristofferC mentioned this pull request Mar 29, 2022
67 tasks
@aviatesk aviatesk mentioned this pull request Apr 12, 2022
3 tasks
@vtjnash vtjnash removed the backport 1.8 Change should be backported to release-1.8 label Apr 12, 2022
@vtjnash
Copy link
Member

vtjnash commented Apr 12, 2022

I am not certain how this got the backport label, since it implements a new feature

@Keno Keno merged commit 2049baa into master Apr 28, 2022
@Keno Keno deleted the jb/ircode2oc branch April 28, 2022 22:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants