Skip to content

Commit 96f93a3

Browse files
Kenostevengj
authored andcommitted
Address some post-commit review from #56660 (#56747)
Some more questions still to be answered, but this should address the immediate actionable review items.
1 parent 91725a5 commit 96f93a3

File tree

6 files changed

+17
-22
lines changed

6 files changed

+17
-22
lines changed

Compiler/extras/CompilerDevTools/src/CompilerDevTools.jl

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,11 @@ import Core.OptimizedGenerics.CompilerPlugins: typeinf, typeinf_edge
3434
Compiler.typeinf_edge(interp, mi.def, mi.specTypes, Core.svec(), parent_frame, false, false)
3535
end
3636

37-
# TODO: This needs special compiler support to properly case split for multiple
38-
# method matches, etc.
39-
@noinline function mi_for_tt(tt, world=Base.tls_world_age())
40-
interp = SplitCacheInterp(; world)
41-
match, _ = Compiler.findsup(tt, Compiler.method_table(interp))
42-
Base.specialize_method(match)
43-
end
44-
4537
function with_new_compiler(f, args...)
46-
tt = Base.signature_type(f, typeof(args))
38+
mi = @ccall jl_method_lookup(Any[f, args...]::Ptr{Any}, (1+length(args))::Csize_t, Base.tls_world_age()::Csize_t)::Ref{Core.MethodInstance}
4739
world = Base.tls_world_age()
4840
new_compiler_ci = Core.OptimizedGenerics.CompilerPlugins.typeinf(
49-
SplitCacheOwner(), mi_for_tt(tt), Compiler.SOURCE_MODE_ABI
41+
SplitCacheOwner(), mi, Compiler.SOURCE_MODE_ABI
5042
)
5143
invoke(f, new_compiler_ci, args...)
5244
end

Compiler/src/abstractinterpretation.jl

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2223,11 +2223,12 @@ function abstract_invoke(interp::AbstractInterpreter, arginfo::ArgInfo, si::Stmt
22232223
if isa(method_or_ci, CodeInstance)
22242224
our_world = sv.world.this
22252225
argtype = argtypes_to_type(pushfirst!(argtype_tail(argtypes, 4), ft))
2226-
sig = method_or_ci.def.specTypes
2226+
specsig = method_or_ci.def.specTypes
2227+
defdef = method_or_ci.def.def
22272228
exct = method_or_ci.exctype
2228-
if !hasintersect(argtype, sig)
2229+
if !hasintersect(argtype, specsig)
22292230
return Future(CallMeta(Bottom, TypeError, EFFECTS_THROWS, NoCallInfo()))
2230-
elseif !(argtype <: sig)
2231+
elseif !(argtype <: specsig) || (isa(defdef, Method) && !(argtype <: defdef.sig))
22312232
exct = Union{exct, TypeError}
22322233
end
22332234
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
22572258
# Fall through to generic invoke handling
22582259
end
22592260
else
2260-
widenconst(types) >: Union{Method, CodeInstance} && return Future(CallMeta(Any, Any, Effects(), NoCallInfo()))
2261+
hasintersect(widenconst(types), Union{Method, CodeInstance}) && return Future(CallMeta(Any, Any, Effects(), NoCallInfo()))
22612262
(types, isexact, isconcrete, istype) = instanceof_tfunc(argtype_by_index(argtypes, 3), false)
22622263
isexact || return Future(CallMeta(Any, Any, Effects(), NoCallInfo()))
22632264
unwrapped = unwrap_unionall(types)

Compiler/src/stmtinfo.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,7 @@ struct InvokeCICallInfo <: CallInfo
277277
edge::CodeInstance
278278
end
279279
add_edges_impl(edges::Vector{Any}, info::InvokeCICallInfo) =
280-
add_one_edge!(edges, info.edge)
280+
add_inlining_edge!(edges, info.edge)
281281

282282
"""
283283
info::InvokeCallInfo

base/docs/basedocs.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2064,8 +2064,8 @@ to `invoke(f, method.sig, args...)`.
20642064
The `argtypes` argument may be a `CodeInstance`, bypassing both method lookup and specialization.
20652065
The semantics of this invocation are similar to a function pointer call of the `CodeInstance`'s
20662066
`invoke` pointer. It is an error to invoke a `CodeInstance` with arguments that do not match its
2067-
parent MethodInstance or from a world age not included in the `min_world`/`max_world` range.
2068-
It is undefined behavior to invoke a CodeInstance whose behavior does not match the constraints
2067+
parent `MethodInstance` or from a world age not included in the `min_world`/`max_world` range.
2068+
It is undefined behavior to invoke a `CodeInstance` whose behavior does not match the constraints
20692069
specified in its fields. For some code instances with `owner !== nothing` (i.e. those generated
20702070
by external compilers), it may be an error to invoke them after passing through precompilation.
20712071
This is an advanced interface intended for use with external compiler plugins.

base/optimized_generics.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ end
6161
Implements a pair of functions `typeinf`/`typeinf_edge`. When the optimizer sees
6262
a call to `typeinf`, it has license to instead call `typeinf_edge`, supplying the
6363
current inference stack in `parent_frame` (but otherwise supplying the arguments
64-
to `typeinf`). typeinf_edge will return the `CodeInstance` that `typeinf` would
64+
to `typeinf`). `typeinf_edge` will return the `CodeInstance` that `typeinf` would
6565
have returned at runtime. The optimizer may perform a non-IPO replacement of
6666
the call to `typeinf` by the result of `typeinf_edge`. In addition, the IPO-safe
6767
fields of the `CodeInstance` may be propagated in IPO mode.

src/builtins.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1590,8 +1590,10 @@ JL_CALLABLE(jl_f_invoke)
15901590
} else if (jl_is_code_instance(argtypes)) {
15911591
jl_code_instance_t *codeinst = (jl_code_instance_t*)args[1];
15921592
jl_callptr_t invoke = jl_atomic_load_acquire(&codeinst->invoke);
1593-
if (jl_tuple1_isa(args[0], &args[2], nargs - 2, (jl_datatype_t*)codeinst->def->specTypes)) {
1594-
jl_type_error("invoke: argument type error", codeinst->def->specTypes, arg_tuple(args[0], &args[2], nargs - 2));
1593+
// N.B.: specTypes need not be a subtype of the method signature. We need to check both.
1594+
if (!jl_tuple1_isa(args[0], &args[2], nargs - 1, (jl_datatype_t*)codeinst->def->specTypes) ||
1595+
(jl_is_method(codeinst->def->def.value) && !jl_tuple1_isa(args[0], &args[2], nargs - 1, (jl_datatype_t*)codeinst->def->def.method->sig))) {
1596+
jl_type_error("invoke: argument type error", codeinst->def->specTypes, arg_tuple(args[0], &args[2], nargs - 1));
15951597
}
15961598
if (jl_atomic_load_relaxed(&codeinst->min_world) > jl_current_task->world_age ||
15971599
jl_current_task->world_age > jl_atomic_load_relaxed(&codeinst->max_world)) {
@@ -1604,10 +1606,10 @@ JL_CALLABLE(jl_f_invoke)
16041606
if (invoke) {
16051607
return invoke(args[0], &args[2], nargs - 2, codeinst);
16061608
} else {
1607-
if (codeinst->owner != jl_nothing || !jl_is_method(codeinst->def->def.value)) {
1609+
if (codeinst->owner != jl_nothing) {
16081610
jl_error("Failed to invoke or compile external codeinst");
16091611
}
1610-
return jl_gf_invoke_by_method(codeinst->def->def.method, args[0], &args[2], nargs - 1);
1612+
return jl_invoke(args[0], &args[2], nargs - 1, codeinst->def);
16111613
}
16121614
}
16131615
if (!jl_is_tuple_type(jl_unwrap_unionall(argtypes)))

0 commit comments

Comments
 (0)