From b5a44e3a3393bf683acbc02d599bcb6a074f95ef Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Tue, 3 Dec 2024 19:33:51 +0000 Subject: [PATCH] Address some post-commit review from #56660 Some more questions still to be answered, but this should address the immediate actionable review items. --- .../extras/CompilerDevTools/src/CompilerDevTools.jl | 12 ++---------- Compiler/src/abstractinterpretation.jl | 7 ++++--- Compiler/src/stmtinfo.jl | 2 +- src/builtins.c | 8 +++++--- 4 files changed, 12 insertions(+), 17 deletions(-) diff --git a/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl b/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl index cd3f7b7b4bdac..5d0df5ccaa4e4 100644 --- a/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl +++ b/Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl @@ -34,19 +34,11 @@ import Core.OptimizedGenerics.CompilerPlugins: typeinf, typeinf_edge Compiler.typeinf_edge(interp, mi.def, mi.specTypes, Core.svec(), parent_frame, false, false) end -# TODO: This needs special compiler support to properly case split for multiple -# method matches, etc. -@noinline function mi_for_tt(tt, world=Base.tls_world_age()) - interp = SplitCacheInterp(; world) - match, _ = Compiler.findsup(tt, Compiler.method_table(interp)) - Base.specialize_method(match) -end - function with_new_compiler(f, args...) - tt = Base.signature_type(f, typeof(args)) + mi = @ccall jl_method_lookup(Any[f, args...]::Ptr{Any}, (1+length(args))::Csize_t, Base.tls_world_age()::Csize_t)::Ref{Core.MethodInstance} world = Base.tls_world_age() new_compiler_ci = Core.OptimizedGenerics.CompilerPlugins.typeinf( - SplitCacheOwner(), mi_for_tt(tt), Compiler.SOURCE_MODE_ABI + SplitCacheOwner(), mi, Compiler.SOURCE_MODE_ABI ) invoke(f, new_compiler_ci, args...) end diff --git a/Compiler/src/abstractinterpretation.jl b/Compiler/src/abstractinterpretation.jl index 24daaf1e6f626..b398c1bdc4295 100644 --- a/Compiler/src/abstractinterpretation.jl +++ b/Compiler/src/abstractinterpretation.jl @@ -2223,11 +2223,12 @@ function abstract_invoke(interp::AbstractInterpreter, arginfo::ArgInfo, si::Stmt if isa(method_or_ci, CodeInstance) our_world = sv.world.this argtype = argtypes_to_type(pushfirst!(argtype_tail(argtypes, 4), ft)) - sig = method_or_ci.def.specTypes + specsig = method_or_ci.def.specTypes + defdef = method_or_ci.def.def exct = method_or_ci.exctype if !hasintersect(argtype, sig) return Future(CallMeta(Bottom, TypeError, EFFECTS_THROWS, NoCallInfo())) - elseif !(argtype <: sig) + elseif !(argtype <: sig) || (isa(defdef, Method) && !(argtype <: defdef.sig)) exct = Union{exct, TypeError} end callee_valid_range = WorldRange(method_or_ci.min_world, method_or_ci.max_world) @@ -2257,7 +2258,7 @@ function abstract_invoke(interp::AbstractInterpreter, arginfo::ArgInfo, si::Stmt # Fall through to generic invoke handling end else - widenconst(types) >: Union{Method, CodeInstance} && return Future(CallMeta(Any, Any, Effects(), NoCallInfo())) + hasintersect(widenconst(types), Union{Method, CodeInstance}) && return Future(CallMeta(Any, Any, Effects(), NoCallInfo())) (types, isexact, isconcrete, istype) = instanceof_tfunc(argtype_by_index(argtypes, 3), false) isexact || return Future(CallMeta(Any, Any, Effects(), NoCallInfo())) unwrapped = unwrap_unionall(types) diff --git a/Compiler/src/stmtinfo.jl b/Compiler/src/stmtinfo.jl index 9f0f1f38d4c8a..4f55f068e9456 100644 --- a/Compiler/src/stmtinfo.jl +++ b/Compiler/src/stmtinfo.jl @@ -277,7 +277,7 @@ struct InvokeCICallInfo <: CallInfo edge::CodeInstance end add_edges_impl(edges::Vector{Any}, info::InvokeCICallInfo) = - add_one_edge!(edges, info.edge) + add_inlining_edge!(edges, info.edge) """ info::InvokeCallInfo diff --git a/src/builtins.c b/src/builtins.c index 3f555da9d2a83..a47145ce225a3 100644 --- a/src/builtins.c +++ b/src/builtins.c @@ -1590,7 +1590,9 @@ JL_CALLABLE(jl_f_invoke) } else if (jl_is_code_instance(argtypes)) { jl_code_instance_t *codeinst = (jl_code_instance_t*)args[1]; jl_callptr_t invoke = jl_atomic_load_acquire(&codeinst->invoke); - if (jl_tuple1_isa(args[0], &args[2], nargs - 2, (jl_datatype_t*)codeinst->def->specTypes)) { + // N.B.: specTypes need not be a subtype of the method signature. We need to check both. + if (!jl_tuple1_isa(args[0], &args[2], nargs - 2, (jl_datatype_t*)codeinst->def->specTypes) || + (jl_is_method(codeinst->def->def.value) && !jl_tuple1_isa(args[0], &args[2], nargs - 2, (jl_datatype_t*)codeinst->def->def.method->sig))) { jl_type_error("invoke: argument type error", codeinst->def->specTypes, arg_tuple(args[0], &args[2], nargs - 2)); } if (jl_atomic_load_relaxed(&codeinst->min_world) > jl_current_task->world_age || @@ -1604,10 +1606,10 @@ JL_CALLABLE(jl_f_invoke) if (invoke) { return invoke(args[0], &args[2], nargs - 2, codeinst); } else { - if (codeinst->owner != jl_nothing || !jl_is_method(codeinst->def->def.value)) { + if (codeinst->owner != jl_nothing) { jl_error("Failed to invoke or compile external codeinst"); } - return jl_gf_invoke_by_method(codeinst->def->def.method, args[0], &args[2], nargs - 1); + return jl_invoke(args[0], &args[2], nargs - 1, codeinst->def); } } if (!jl_is_tuple_type(jl_unwrap_unionall(argtypes)))