Use opaque closure for tasks

[tkf]

To make fetch(::Task) inferrable and also to optimize tasks better in general, it might be a good idea to use opaque closures with @spawn etc. Now we are at a very early phase of 1.9-DEV, it might be a good idea to start using opaque closures now and let possible bugs surface before the next feature freeze. (But this PR does not make fetch(::Task) inferrable on its own.)

Actually, it looks like we need to fix at least one bug before even start using opaque closures in @spawn since codegen for opaque closures does not work with sysimage yet. I get an error like

Sysimage built. Summary:
Total ───────  81.970892 seconds
Base: ───────  32.638817 seconds 39.8176%
Stdlibs: ────  49.330319 seconds 60.1803%
julia: /cache/build/default-amdci5-0/julialang/julia-master/usr/include/llvm/Support/Casting.h:104: static bool llvm::isa_impl_cl<To, const From*>::doit(const From*) [with To = llvm::GlobalVariable; From = llvm::GlobalValue]: Assertion `Val && "isa<> used on a null pointer"' failed.

signal (6): Aborted
in expression starting at none:0
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x7f67e9dd540e)
__assert_fail at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
doit at /cache/build/default-amdci5-0/julialang/julia-master/usr/include/llvm/Support/Casting.h:104 [inlined]
doit at /cache/build/default-amdci5-0/julialang/julia-master/usr/include/llvm/Support/Casting.h:131 [inlined]
doit at /cache/build/default-amdci5-0/julialang/julia-master/usr/include/llvm/Support/Casting.h:122 [inlined]
isa<llvm::GlobalVariable, llvm::GlobalValue*> at /cache/build/default-amdci5-0/julialang/julia-master/usr/include/llvm/Support/Casting.h:143 [inlined]
cast<llvm::GlobalVariable, llvm::GlobalValue> at /cache/build/default-amdci5-0/julialang/julia-master/usr/include/llvm/Support/Casting.h:269 [inlined]
jl_create_native_impl at /cache/build/default-amdci5-0/julialang/julia-master/src/aotcompile.cpp:369
jl_precompile at /cache/build/default-amdci5-0/julialang/julia-master/src/precompile.c:402 [inlined]

--- https://buildkite.com/julialang/julia-master/builds/8961#8ba39922-d862-4ac8-95d1-ecd0178c9819/193-847

This is due to the lookup of the global clone->getNamedValue(global) in

julia/src/aotcompile.cpp

Lines 366 to 373 in 7b39515

	// now get references to the globals in the merged module
	// and set them to be internalized and initialized at startup
	for (auto &global : gvars) {
	GlobalVariable *G = cast<GlobalVariable>(clone->getNamedValue(global));
	G->setInitializer(ConstantPointerNull::get(cast<PointerType>(G->getValueType())));
	G->setLinkage(GlobalVariable::InternalLinkage);
	data->jl_sysimg_gvars.push_back(G);
	}

failed and returned a NULL.

I tried to fix this in d30e90a but I'm not sure if this is the right way to do it. I just simply tried to treat the symptom I found with rr.

@Keno Are opaque closures ready for this? Also, is my patch d30e90a OK?

[tkf]

Hmm... I get another error in doctest. I've never seen this in my local build

    JULIA usr/lib/julia/sys-o.a
Generating REPL precompile statements... 36/36
Executing precompile statements... 1442/2053
signal (11): Segmentation fault
in expression starting at none:0
_ZNK4llvm5Value7getNameEv at /cache/build/default-amdci5-6/julialang/julia-master/usr/bin/../lib/libLLVM-13jl.so (unknown line)
get_pointer_to_constant at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:1390
stringConstPtr at /cache/build/default-amdci5-6/julialang/julia-master/src/cgutils.cpp:78
emit_type_error at /cache/build/default-amdci5-6/julialang/julia-master/src/cgutils.cpp:1135
emit_typecheck at /cache/build/default-amdci5-6/julialang/julia-master/src/cgutils.cpp:1327
emit_builtin_call at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:2810
emit_call at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:3831
emit_expr at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:4731
emit_ssaval_assign at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:4306
emit_stmtpos at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:4553 [inlined]
emit_function at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:7404
get_oc_function at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:4585 [inlined]
emit_expr at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:4911
emit_ssaval_assign at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:4306
emit_stmtpos at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:4553 [inlined]
emit_function at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:7404
jl_emit_code at /cache/build/default-amdci5-6/julialang/julia-master/src/codegen.cpp:7787
jl_create_native_impl at /cache/build/default-amdci5-6/julialang/julia-master/src/aotcompile.cpp:314
jl_precompile at /cache/build/default-amdci5-6/julialang/julia-master/src/precompile.c:402 [inlined]
jl_write_compiler_output at /cache/build/default-amdci5-6/julialang/julia-master/src/precompile.c:33
ijl_atexit_hook at /cache/build/default-amdci5-6/julialang/julia-master/src/init.c:207
jl_repl_entrypoint at /cache/build/default-amdci5-6/julialang/julia-master/src/jlapi.c:707
main at /cache/build/default-amdci5-6/julialang/julia-master/cli/loader_exe.c:59
__libc_start_main at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
_start at /cache/build/default-amdci5-6/julialang/julia-master/usr/bin/julia (unknown line)
Allocations: 157101283 (Pool: 157025822; Big: 75461); GC: 156
Segmentation fault

https://buildkite.com/julialang/julia-master/builds/8966#57455b5b-5bad-4675-ad45-2de736b70e1e/160-818

[tkf]

I just noticed

julia/src/dump.c

Lines 725 to 727 in 7b39515

	if (jl_is_method(mi->def.value) && mi->def.method->is_for_opaque_closure) {
	jl_error("unimplemented: serialization of MethodInstances for OpaqueClosure");
	}

which makes it impossible to precompile Revise with this branch presumably because Revise precompiles some functions containing @async or @spawn.

So, maybe we should only do the fix for the sysimage and postpone what I actually wanted to do in this PR.

[vtjnash]

fetch(::Task) is already inferrable. What does this PR bring except problems with serializing and reflecting on them?

[vtjnash]

jl_merge_module is mostly a legacy hack. If I understand the purpose of #44176 correctly, we should be able to delete this code and handle them like a normal method now.

[JeffBezanson]

Right, I don't see why we need opaque closures to make fetch inferrable. If inference is going to look at the function inside the task, it doesn't matter what kind of function it is.

[tkf]

Hmm... yeah, I don't remember why I didn't think about it.