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 5, 2024
1 parent 5835c3b commit d3361ca
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 22 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
9 changes: 5 additions & 4 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)
if !hasintersect(argtype, specsig)
return Future(CallMeta(Bottom, TypeError, EFFECTS_THROWS, NoCallInfo()))
elseif !(argtype <: sig)
elseif !(argtype <: specsig) || (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
4 changes: 2 additions & 2 deletions base/docs/basedocs.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2064,8 +2064,8 @@ to `invoke(f, method.sig, args...)`.
The `argtypes` argument may be a `CodeInstance`, bypassing both method lookup and specialization.
The semantics of this invocation are similar to a function pointer call of the `CodeInstance`'s
`invoke` pointer. It is an error to invoke a `CodeInstance` with arguments that do not match its
parent MethodInstance or from a world age not included in the `min_world`/`max_world` range.
It is undefined behavior to invoke a CodeInstance whose behavior does not match the constraints
parent `MethodInstance` or from a world age not included in the `min_world`/`max_world` range.
It is undefined behavior to invoke a `CodeInstance` whose behavior does not match the constraints
specified in its fields. For some code instances with `owner !== nothing` (i.e. those generated
by external compilers), it may be an error to invoke them after passing through precompilation.
This is an advanced interface intended for use with external compiler plugins.
Expand Down
2 changes: 1 addition & 1 deletion base/optimized_generics.jl
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ end
Implements a pair of functions `typeinf`/`typeinf_edge`. When the optimizer sees
a call to `typeinf`, it has license to instead call `typeinf_edge`, supplying the
current inference stack in `parent_frame` (but otherwise supplying the arguments
to `typeinf`). typeinf_edge will return the `CodeInstance` that `typeinf` would
to `typeinf`). `typeinf_edge` will return the `CodeInstance` that `typeinf` would
have returned at runtime. The optimizer may perform a non-IPO replacement of
the call to `typeinf` by the result of `typeinf_edge`. In addition, the IPO-safe
fields of the `CodeInstance` may be propagated in IPO mode.
Expand Down
10 changes: 6 additions & 4 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -1590,8 +1590,10 @@ 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)) {
jl_type_error("invoke: argument type error", codeinst->def->specTypes, arg_tuple(args[0], &args[2], nargs - 2));
// 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 - 1, (jl_datatype_t*)codeinst->def->specTypes) ||
(jl_is_method(codeinst->def->def.value) && !jl_tuple1_isa(args[0], &args[2], nargs - 1, (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 - 1));
}
if (jl_atomic_load_relaxed(&codeinst->min_world) > jl_current_task->world_age ||
jl_current_task->world_age > jl_atomic_load_relaxed(&codeinst->max_world)) {
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 d3361ca

Please sign in to comment.