Skip to content

Commit

Permalink
Address some post-commit review from #56660
Browse files Browse the repository at this point in the history
Some more questions still to be answered, but this should address the
immediate actionable review items.
  • Loading branch information
Keno committed Dec 4, 2024
1 parent 2590e67 commit b5a44e3
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 17 deletions.
12 changes: 2 additions & 10 deletions Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions Compiler/src/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion Compiler/src/stmtinfo.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 5 additions & 3 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand All @@ -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)))
Expand Down

0 comments on commit b5a44e3

Please sign in to comment.