From 15b265e6764fb5d57777a28f9053a2793f726f40 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Mon, 15 Mar 2021 10:55:35 -0400 Subject: [PATCH] Fix opaque closure codegen ABI (#40001) At the moment our ABI for specfun opaque closures is that the opaque closure itself remains a GC tracked pointer out of which the function itself implicitly loads the environment and the world age. In the future we may want to improve this ABI, but it seems fine for now. However, this ABI wasn't quite implemented correctly after we've started turning on inference of OpaqueClosure where the first argument of the MethodInstance's linfo actually stores the type of the closure environment rather than the closure itself. Fix this by manually forcing the signature determination code to just treat that argument as `Any`, though of course in the future we may want to do something fancier. --- src/codegen.cpp | 51 ++++++++++++++++++++++++++++++------------ test/opaque_closure.jl | 15 +++++++++++++ 2 files changed, 52 insertions(+), 14 deletions(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index ea7fb688e10819..33572e58b24127 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -1118,7 +1118,7 @@ GlobalVariable *JuliaVariable::realize(jl_codectx_t &ctx) { } static Type *julia_type_to_llvm(jl_codectx_t &ctx, jl_value_t *jt, bool *isboxed = NULL); -static jl_returninfo_t get_specsig_function(jl_codectx_t &ctx, Module *M, StringRef name, jl_value_t *sig, jl_value_t *jlrettype); +static jl_returninfo_t get_specsig_function(jl_codectx_t &ctx, Module *M, StringRef name, jl_value_t *sig, jl_value_t *jlrettype, bool is_opaque_closure); static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval = -1); static Value *global_binding_pointer(jl_codectx_t &ctx, jl_module_t *m, jl_sym_t *s, jl_binding_t **pbnd, bool assign); @@ -3305,7 +3305,8 @@ static jl_cgval_t emit_call_specfun_other(jl_codectx_t &ctx, jl_method_instance_ jl_cgval_t *argv, size_t nargs, jl_returninfo_t::CallingConv *cc, unsigned *return_roots, jl_value_t *inferred_retty) { // emit specialized call site - jl_returninfo_t returninfo = get_specsig_function(ctx, jl_Module, specFunctionObject, mi->specTypes, jlretty); + bool is_opaque_closure = jl_is_method(mi->def.value) && mi->def.method->is_for_opaque_closure; + jl_returninfo_t returninfo = get_specsig_function(ctx, jl_Module, specFunctionObject, mi->specTypes, jlretty, is_opaque_closure); FunctionType *cft = returninfo.decl->getFunctionType(); *cc = returninfo.cc; *return_roots = returninfo.return_roots; @@ -3340,7 +3341,8 @@ static jl_cgval_t emit_call_specfun_other(jl_codectx_t &ctx, jl_method_instance_ } for (size_t i = 0; i < nargs; i++) { - jl_value_t *jt = jl_nth_slot_type(mi->specTypes, i); + jl_value_t *jt = (is_opaque_closure && i == 0) ? (jl_value_t*)jl_any_type : + jl_nth_slot_type(mi->specTypes, i); if (is_uniquerep_Type(jt)) continue; bool isboxed = deserves_argbox(jt); @@ -4622,7 +4624,7 @@ static jl_cgval_t emit_expr(jl_codectx_t &ctx, jl_value_t *expr, ssize_t ssaval) Function *specptr = closure_m->getFunction(closure_decls.specFunctionObject); if (specptr) { jl_returninfo_t returninfo = get_specsig_function(ctx, jl_Module, - closure_decls.specFunctionObject, li->specTypes, ub.constant); + closure_decls.specFunctionObject, li->specTypes, ub.constant, true); fptr = mark_julia_type(ctx, returninfo.decl, false, jl_voidpointer_type); } else { fptr = mark_julia_type(ctx, @@ -5237,10 +5239,11 @@ static Function* gen_cfun_wrapper( retval = mark_julia_type(ctx, ret, true, astrt); } else { + bool is_opaque_closure = jl_is_method(lam->def.value) && lam->def.method->is_for_opaque_closure; assert(calltype == 3); // emit a specsig call StringRef protoname = jl_ExecutionEngine->getFunctionAtAddress((uintptr_t)callptr, codeinst); - jl_returninfo_t returninfo = get_specsig_function(ctx, M, protoname, lam->specTypes, astrt); + jl_returninfo_t returninfo = get_specsig_function(ctx, M, protoname, lam->specTypes, astrt, is_opaque_closure); FunctionType *cft = returninfo.decl->getFunctionType(); jlfunc_sret = (returninfo.cc == jl_returninfo_t::SRet); @@ -5265,7 +5268,8 @@ static Function* gen_cfun_wrapper( // figure out how to repack the arguments jl_cgval_t &inputarg = inputargs[i]; Value *arg; - jl_value_t *spect = jl_nth_slot_type(lam->specTypes, i); + jl_value_t *spect = (i == 0 && is_opaque_closure) ? (jl_value_t*)jl_any_type : + jl_nth_slot_type(lam->specTypes, i); bool isboxed = deserves_argbox(spect); Type *T = isboxed ? T_prjlvalue : julia_type_to_llvm(ctx, spect); if (is_uniquerep_Type(spect)) { @@ -5683,8 +5687,10 @@ static Function *gen_invoke_wrapper(jl_method_instance_t *lam, jl_value_t *jlret idx++; } + bool is_opaque_closure = jl_is_method(lam->def.value) && lam->def.method->is_for_opaque_closure; for (size_t i = 0; i < jl_nparams(lam->specTypes) && idx < nfargs; ++i) { - jl_value_t *ty = jl_nth_slot_type(lam->specTypes, i); + jl_value_t *ty = ((i == 0) && is_opaque_closure) ? (jl_value_t*)jl_any_type : + jl_nth_slot_type(lam->specTypes, i); bool isboxed = deserves_argbox(ty); Type *lty = isboxed ? T_prjlvalue : julia_type_to_llvm(ctx, ty); if (type_is_ghost(lty) || is_uniquerep_Type(ty)) @@ -5754,7 +5760,7 @@ static Function *gen_invoke_wrapper(jl_method_instance_t *lam, jl_value_t *jlret return w; } -static jl_returninfo_t get_specsig_function(jl_codectx_t &ctx, Module *M, StringRef name, jl_value_t *sig, jl_value_t *jlrettype) +static jl_returninfo_t get_specsig_function(jl_codectx_t &ctx, Module *M, StringRef name, jl_value_t *sig, jl_value_t *jlrettype, bool is_opaque_closure) { jl_returninfo_t props = {}; SmallVector fsig; @@ -5833,6 +5839,9 @@ static jl_returninfo_t get_specsig_function(jl_codectx_t &ctx, Module *M, String for (size_t i = 0; i < jl_nparams(sig); i++) { jl_value_t *jt = jl_tparam(sig, i); + if (i == 0 && is_opaque_closure) { + jt = (jl_value_t*)jl_any_type; + } if (is_uniquerep_Type(jt)) continue; bool isboxed = deserves_argbox(jt); @@ -6076,7 +6085,7 @@ static std::pair, jl_llvm_functions_t> Function *f = NULL; bool has_sret = false; if (specsig) { // assumes !va and !needsparams - returninfo = get_specsig_function(ctx, M, declarations.specFunctionObject, lam->specTypes, jlrettype); + returninfo = get_specsig_function(ctx, M, declarations.specFunctionObject, lam->specTypes, jlrettype, ctx.is_opaque_closure); f = returninfo.decl; has_sret = (returninfo.cc == jl_returninfo_t::SRet || returninfo.cc == jl_returninfo_t::Union); jl_init_function(f); @@ -6460,7 +6469,8 @@ static std::pair, jl_llvm_functions_t> AI++; // skip return_roots slot for (i = 0; i < nreq; i++) { jl_sym_t *s = (jl_sym_t*)jl_array_ptr_ref(src->slotnames, i); - jl_value_t *argType = jl_nth_slot_type(lam->specTypes, i); + jl_value_t *argType = (i == 0 && ctx.is_opaque_closure) ? (jl_value_t*)jl_any_type : + jl_nth_slot_type(lam->specTypes, i); bool isboxed = deserves_argbox(argType); Type *llvmArgType = isboxed ? T_prjlvalue : julia_type_to_llvm(ctx, argType); if (s == unused_sym) { @@ -6508,11 +6518,24 @@ static std::pair, jl_llvm_functions_t> // If this is an opaque closure, implicitly load the env and switch // the world age. if (i == 0 && ctx.is_opaque_closure) { - jl_cgval_t closure_world = emit_getfield_knownidx(ctx, theArg, 2, (jl_datatype_t*)argType); + // Load closure world + Value *argaddr = emit_bitcast(ctx, maybe_decay_tracked(ctx, data_pointer(ctx, theArg)), T_pint8); + Value *worldaddr = ctx.builder.CreateInBoundsGEP( + T_int8, argaddr, + ConstantInt::get(T_size, offsetof(jl_opaque_closure_t, world))); + + jl_cgval_t closure_world = typed_load(ctx, worldaddr, NULL, (jl_value_t*)jl_long_type, + theArg.tbaa, nullptr, false, sizeof(size_t)); emit_unbox(ctx, T_size, closure_world, (jl_value_t*)jl_long_type, ctx.world_age_field, tbaa_gcframe); - theArg = convert_julia_type(ctx, - emit_getfield_knownidx(ctx, theArg, 0, (jl_datatype_t*)argType), - vi.value.typ); + + // Load closure env + Value *envaddr = ctx.builder.CreateInBoundsGEP( + T_int8, argaddr, + ConstantInt::get(T_size, offsetof(jl_opaque_closure_t, captures))); + + jl_cgval_t closure_env = typed_load(ctx, envaddr, NULL, (jl_value_t*)jl_any_type, + theArg.tbaa, nullptr, false, sizeof(void*)); + theArg = convert_julia_type(ctx, closure_env, vi.value.typ); } if (vi.boxroot == NULL) { diff --git a/test/opaque_closure.jl b/test/opaque_closure.jl index e74d54820163cd..b8d8c100b575c6 100644 --- a/test/opaque_closure.jl +++ b/test/opaque_closure.jl @@ -189,3 +189,18 @@ function oc_varargs_constprop() return Val{oc(1,2,3)}() end Base.return_types(oc_varargs_constprop, Tuple{}) == Any[Val{6}] + +# OpaqueClosure ABI +f_oc_noinline(x) = @opaque function (y) + @Base._noinline_meta + x + y +end + +let oc = Base.inferencebarrier(f_oc_noinline(1)) + @test oc(2) == 3 +end + +function f_oc_noinline_call(x, y) + return f_oc_noinline(x)(y) +end +@test f_oc_noinline_call(1, 2) == 3