Skip to content

Commit 7080f68

Browse files
committed
Support adding backedges with invokesig
This allows a MethodInstance to store dispatch tuples as well as other MethodInstances among their backedges. The motivation is to properly handle invalidation with `invoke`, which allows calling a less-specific method and thus runs afoul of our standard mechanisms to detect "outdated" dispatch. Additionally: - provide backedge-iterators for both C and Julia that abstracts the specific storage mechanism. - fix a bug in the CodeInstance `relocatability` field, where methods that only return a constant (and hence store `nothing` for `inferred`) were deemed non-relocatable. - fix a bug in which #43990 should have checked that the method had not been deleted. Tests passed formerly simply because we weren't caching external CodeInstances that inferred down to a `Const`; fixing that exposed the bug. This bug has been exposed since merging #43990 for non-`Const` inference, and would affect Revise etc.
1 parent 80e50b5 commit 7080f68

File tree

14 files changed

+430
-107
lines changed

14 files changed

+430
-107
lines changed

NEWS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ Compiler/Runtime improvements
3535
`@nospecialize`-d call sites and avoiding excessive compilation. ([#44512])
3636
* All the previous usages of `@pure`-macro in `Base` has been replaced with the preferred
3737
`Base.@assume_effects`-based annotations. ([#44776])
38+
* `invoke(f, invokesig, args...)` calls to a less-specific method than would normally be chosen
39+
for `f(args...)` are no longer spuriously invalidated when loading package precompile files. ([#46010])
3840

3941
Command-line option changes
4042
---------------------------

base/compiler/abstractinterpretation.jl

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,13 @@ function collect_const_args(argtypes::Vector{Any})
801801
end for i = 2:length(argtypes) ]
802802
end
803803

804+
function invoke_signature(invokesig::Vector{Any})
805+
unwrapconst(x) = isa(x, Const) ? x.val : x
806+
807+
f, argtyps = unwrapconst(invokesig[2]), unwrapconst(invokesig[3])
808+
return Tuple{typeof(f), unwrap_unionall(argtyps).parameters...}
809+
end
810+
804811
function concrete_eval_call(interp::AbstractInterpreter,
805812
@nospecialize(f), result::MethodCallResult, arginfo::ArgInfo, sv::InferenceState)
806813
concrete_eval_eligible(interp, f, result, arginfo, sv) || return nothing
@@ -1631,7 +1638,7 @@ function abstract_invoke(interp::AbstractInterpreter, (; fargs, argtypes)::ArgIn
16311638
ti = tienv[1]; env = tienv[2]::SimpleVector
16321639
result = abstract_call_method(interp, method, ti, env, false, sv)
16331640
(; rt, edge, effects) = result
1634-
edge !== nothing && add_backedge!(edge::MethodInstance, sv)
1641+
edge !== nothing && add_backedge!(edge::MethodInstance, sv, argtypes)
16351642
match = MethodMatch(ti, env, method, argtype <: method.sig)
16361643
res = nothing
16371644
sig = match.spec_types

base/compiler/inferencestate.jl

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -479,12 +479,15 @@ function add_cycle_backedge!(frame::InferenceState, caller::InferenceState, curr
479479
end
480480

481481
# temporarily accumulate our edges to later add as backedges in the callee
482-
function add_backedge!(li::MethodInstance, caller::InferenceState)
482+
function add_backedge!(li::MethodInstance, caller::InferenceState, invokesig::Union{Nothing,Vector{Any}}=nothing)
483483
isa(caller.linfo.def, Method) || return # don't add backedges to toplevel exprs
484484
edges = caller.stmt_edges[caller.currpc]
485485
if edges === nothing
486486
edges = caller.stmt_edges[caller.currpc] = []
487487
end
488+
if invokesig !== nothing
489+
push!(edges, invoke_signature(invokesig))
490+
end
488491
push!(edges, li)
489492
return nothing
490493
end

base/compiler/optimize.jl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ intersect!(et::EdgeTracker, range::WorldRange) =
6262
et.valid_worlds[] = intersect(et.valid_worlds[], range)
6363

6464
push!(et::EdgeTracker, mi::MethodInstance) = push!(et.edges, mi)
65+
function add_edge!(et::EdgeTracker, @nospecialize(invokesig), mi::MethodInstance)
66+
invokesig === nothing && return push!(et.edges, mi)
67+
push!(et.edges, invokesig, mi)
68+
end
6569
function push!(et::EdgeTracker, ci::CodeInstance)
6670
intersect!(et, WorldRange(min_world(li), max_world(li)))
6771
push!(et, ci.def)

base/compiler/ssair/inlining.jl

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,19 +29,21 @@ pass to apply its own inlining policy decisions.
2929
struct DelayedInliningSpec
3030
match::Union{MethodMatch, InferenceResult}
3131
argtypes::Vector{Any}
32+
invokesig # either nothing or a signature (signature is for an `invoke` call)
3233
end
34+
DelayedInliningSpec(match, argtypes) = DelayedInliningSpec(match, argtypes, nothing)
3335

3436
struct InliningTodo
3537
# The MethodInstance to be inlined
3638
mi::MethodInstance
3739
spec::Union{ResolvedInliningSpec, DelayedInliningSpec}
3840
end
3941

40-
InliningTodo(mi::MethodInstance, match::MethodMatch, argtypes::Vector{Any}) =
41-
InliningTodo(mi, DelayedInliningSpec(match, argtypes))
42+
InliningTodo(mi::MethodInstance, match::MethodMatch, argtypes::Vector{Any}, invokesig=nothing) =
43+
InliningTodo(mi, DelayedInliningSpec(match, argtypes, invokesig))
4244

43-
InliningTodo(result::InferenceResult, argtypes::Vector{Any}) =
44-
InliningTodo(result.linfo, DelayedInliningSpec(result, argtypes))
45+
InliningTodo(result::InferenceResult, argtypes::Vector{Any}, invokesig=nothing) =
46+
InliningTodo(result.linfo, DelayedInliningSpec(result, argtypes, invokesig))
4547

4648
struct ConstantCase
4749
val::Any
@@ -810,15 +812,15 @@ end
810812

811813
function resolve_todo(todo::InliningTodo, state::InliningState, flag::UInt8)
812814
mi = todo.mi
813-
(; match, argtypes) = todo.spec::DelayedInliningSpec
815+
(; match, argtypes, invokesig) = todo.spec::DelayedInliningSpec
814816
et = state.et
815817

816818
#XXX: update_valid_age!(min_valid[1], max_valid[1], sv)
817819
if isa(match, InferenceResult)
818820
inferred_src = match.src
819821
if isa(inferred_src, ConstAPI)
820822
# use constant calling convention
821-
et !== nothing && push!(et, mi)
823+
et !== nothing && add_edge!(et, invokesig, mi)
822824
return ConstantCase(quoted(inferred_src.val))
823825
else
824826
src = inferred_src # ::Union{Nothing,CodeInfo} for NativeInterpreter
@@ -829,7 +831,7 @@ function resolve_todo(todo::InliningTodo, state::InliningState, flag::UInt8)
829831
if code isa CodeInstance
830832
if use_const_api(code)
831833
# in this case function can be inlined to a constant
832-
et !== nothing && push!(et, mi)
834+
et !== nothing && add_edge!(et, invokesig, mi)
833835
return ConstantCase(quoted(code.rettype_const))
834836
else
835837
src = @atomic :monotonic code.inferred
@@ -851,7 +853,7 @@ function resolve_todo(todo::InliningTodo, state::InliningState, flag::UInt8)
851853

852854
src === nothing && return compileable_specialization(et, match, effects)
853855

854-
et !== nothing && push!(et, mi)
856+
et !== nothing && add_edge!(et, invokesig, mi)
855857
return InliningTodo(mi, retrieve_ir_for_inlining(mi, src), effects)
856858
end
857859

@@ -873,7 +875,7 @@ function validate_sparams(sparams::SimpleVector)
873875
return true
874876
end
875877

876-
function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
878+
function analyze_method!(match::MethodMatch, argtypes::Vector{Any}, invokesig,
877879
flag::UInt8, state::InliningState)
878880
method = match.method
879881
spec_types = match.spec_types
@@ -905,7 +907,7 @@ function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
905907
mi = specialize_method(match; preexisting=true) # Union{Nothing, MethodInstance}
906908
isa(mi, MethodInstance) || return compileable_specialization(et, match, Effects())
907909

908-
todo = InliningTodo(mi, match, argtypes)
910+
todo = InliningTodo(mi, match, argtypes, invokesig)
909911
# If we don't have caches here, delay resolving this MethodInstance
910912
# until the batch inlining step (or an external post-processing pass)
911913
state.mi_cache === nothing && return todo
@@ -1100,17 +1102,18 @@ function inline_invoke!(
11001102
if isa(result, ConcreteResult)
11011103
item = concrete_result_item(result, state)
11021104
else
1105+
invokesig = invoke_signature(sig.argtypes)
11031106
argtypes = invoke_rewrite(sig.argtypes)
11041107
if isa(result, ConstPropResult)
1105-
(; mi) = item = InliningTodo(result.result, argtypes)
1108+
(; mi) = item = InliningTodo(result.result, argtypes, invokesig)
11061109
validate_sparams(mi.sparam_vals) || return nothing
11071110
if argtypes_to_type(argtypes) <: mi.def.sig
11081111
state.mi_cache !== nothing && (item = resolve_todo(item, state, flag))
11091112
handle_single_case!(ir, idx, stmt, item, todo, state.params, true)
11101113
return nothing
11111114
end
11121115
end
1113-
item = analyze_method!(match, argtypes, flag, state)
1116+
item = analyze_method!(match, argtypes, invokesig, flag, state)
11141117
end
11151118
handle_single_case!(ir, idx, stmt, item, todo, state.params, true)
11161119
return nothing
@@ -1328,7 +1331,7 @@ function handle_match!(
13281331
# during abstract interpretation: for the purpose of inlining, we can just skip
13291332
# processing this dispatch candidate
13301333
_any(case->case.sig === spec_types, cases) && return true
1331-
item = analyze_method!(match, argtypes, flag, state)
1334+
item = analyze_method!(match, argtypes, nothing, flag, state)
13321335
item === nothing && return false
13331336
push!(cases, InliningCase(spec_types, item))
13341337
return true
@@ -1475,7 +1478,7 @@ function assemble_inline_todo!(ir::IRCode, state::InliningState)
14751478
if isa(result, ConcreteResult)
14761479
item = concrete_result_item(result, state)
14771480
else
1478-
item = analyze_method!(info.match, sig.argtypes, flag, state)
1481+
item = analyze_method!(info.match, sig.argtypes, nothing, flag, state)
14791482
end
14801483
handle_single_case!(ir, idx, stmt, item, todo, state.params)
14811484
end

base/compiler/typeinfer.jl

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,9 @@ function CodeInstance(
312312
const_flags = 0x00
313313
end
314314
end
315-
relocatability = isa(inferred_result, Vector{UInt8}) ? inferred_result[end] : UInt8(0)
315+
relocatability = isa(inferred_result, Vector{UInt8}) ? inferred_result[end] :
316+
inferred_result === nothing ? UInt8(1) : UInt8(0)
317+
# relocatability = isa(inferred_result, Vector{UInt8}) ? inferred_result[end] : UInt8(0)
316318
return CodeInstance(result.linfo,
317319
widenconst(result_type), rettype_const, inferred_result,
318320
const_flags, first(valid_worlds), last(valid_worlds),
@@ -561,17 +563,12 @@ function store_backedges(frame::InferenceResult, edges::Vector{Any})
561563
end
562564

563565
function store_backedges(caller::MethodInstance, edges::Vector{Any})
564-
i = 1
565-
while i <= length(edges)
566-
to = edges[i]
566+
for (typ, to) in BackedgeIterator(edges)
567567
if isa(to, MethodInstance)
568-
ccall(:jl_method_instance_add_backedge, Cvoid, (Any, Any), to, caller)
569-
i += 1
568+
ccall(:jl_method_instance_add_backedge, Cvoid, (Any, Any, Any), to, typ, caller)
570569
else
571570
typeassert(to, Core.MethodTable)
572-
typ = edges[i + 1]
573571
ccall(:jl_method_table_add_backedge, Cvoid, (Any, Any, Any), to, typ, caller)
574-
i += 2
575572
end
576573
end
577574
end

base/compiler/utilities.jl

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,65 @@ Check if `method` is declared as `Base.@constprop :none`.
223223
"""
224224
is_no_constprop(method::Union{Method,CodeInfo}) = method.constprop == 0x02
225225

226+
#############
227+
# backedges #
228+
#############
229+
230+
"""
231+
BackedgeIterator(mi::MethodInstance)
232+
BackedgeIterator(backedges::Vector{Any})
233+
234+
Return an iterator over a list of backedges, which may be extracted
235+
from `mi`. Iteration returns `(sig, caller)` elements, which will be one of
236+
the following:
237+
238+
- `(nothing, caller::MethodInstance)`: a call made by ordinary inferrable dispatch
239+
- `(invokesig, caller::MethodInstance)`: a call made by `invoke(f, invokesig, args...)`
240+
- `(specsig, mt::MethodTable)`: an abstract call
241+
242+
# Examples
243+
244+
```julia
245+
julia> callme(x) = x+1
246+
callme (generic function with 1 method)
247+
248+
julia> callyou(x) = callme(x)
249+
callyou (generic function with 1 method)
250+
251+
julia> callyou(2.0)
252+
3.0
253+
254+
julia> mi = first(which(callme, (Any,)).specializations)
255+
MethodInstance for callme(::Float64)
256+
257+
julia> @eval Core.Compiler for (sig, caller) in BackedgeIterator(Main.mi)
258+
println(sig)
259+
println(caller)
260+
end
261+
nothing
262+
callyou(Float64) from callyou(Any)
263+
```
264+
"""
265+
struct BackedgeIterator
266+
backedges::Vector{Any}
267+
end
268+
269+
const empty_backedge_iter = BackedgeIterator(Any[])
270+
271+
function BackedgeIterator(mi::MethodInstance)
272+
isdefined(mi, :backedges) || return empty_backedge_iter
273+
return BackedgeIterator(mi.backedges)
274+
end
275+
276+
function iterate(iter::BackedgeIterator, i::Int=1)
277+
backedges = iter.backedges
278+
i > length(backedges) && return nothing
279+
item = backedges[i]
280+
isa(item, MethodInstance) && return (nothing, item), i+1 # regular dispatch
281+
isa(item, Core.MethodTable) && return (backedges[i+1], item), i+2 # abstract dispatch
282+
return (item, backedges[i+1]::MethodInstance), i+2 # `invoke` calls
283+
end
284+
226285
#########
227286
# types #
228287
#########

0 commit comments

Comments
 (0)