Skip to content

Commit

Permalink
Support adding backedges with invokesig
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
timholy committed Aug 22, 2022
1 parent 80e50b5 commit 7080f68
Show file tree
Hide file tree
Showing 14 changed files with 430 additions and 107 deletions.
2 changes: 2 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ Compiler/Runtime improvements
`@nospecialize`-d call sites and avoiding excessive compilation. ([#44512])
* All the previous usages of `@pure`-macro in `Base` has been replaced with the preferred
`Base.@assume_effects`-based annotations. ([#44776])
* `invoke(f, invokesig, args...)` calls to a less-specific method than would normally be chosen
for `f(args...)` are no longer spuriously invalidated when loading package precompile files. ([#46010])

Command-line option changes
---------------------------
Expand Down
9 changes: 8 additions & 1 deletion base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -801,6 +801,13 @@ function collect_const_args(argtypes::Vector{Any})
end for i = 2:length(argtypes) ]
end

function invoke_signature(invokesig::Vector{Any})
unwrapconst(x) = isa(x, Const) ? x.val : x

f, argtyps = unwrapconst(invokesig[2]), unwrapconst(invokesig[3])
return Tuple{typeof(f), unwrap_unionall(argtyps).parameters...}
end

function concrete_eval_call(interp::AbstractInterpreter,
@nospecialize(f), result::MethodCallResult, arginfo::ArgInfo, sv::InferenceState)
concrete_eval_eligible(interp, f, result, arginfo, sv) || return nothing
Expand Down Expand Up @@ -1631,7 +1638,7 @@ function abstract_invoke(interp::AbstractInterpreter, (; fargs, argtypes)::ArgIn
ti = tienv[1]; env = tienv[2]::SimpleVector
result = abstract_call_method(interp, method, ti, env, false, sv)
(; rt, edge, effects) = result
edge !== nothing && add_backedge!(edge::MethodInstance, sv)
edge !== nothing && add_backedge!(edge::MethodInstance, sv, argtypes)
match = MethodMatch(ti, env, method, argtype <: method.sig)
res = nothing
sig = match.spec_types
Expand Down
5 changes: 4 additions & 1 deletion base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -479,12 +479,15 @@ function add_cycle_backedge!(frame::InferenceState, caller::InferenceState, curr
end

# temporarily accumulate our edges to later add as backedges in the callee
function add_backedge!(li::MethodInstance, caller::InferenceState)
function add_backedge!(li::MethodInstance, caller::InferenceState, invokesig::Union{Nothing,Vector{Any}}=nothing)
isa(caller.linfo.def, Method) || return # don't add backedges to toplevel exprs
edges = caller.stmt_edges[caller.currpc]
if edges === nothing
edges = caller.stmt_edges[caller.currpc] = []
end
if invokesig !== nothing
push!(edges, invoke_signature(invokesig))
end
push!(edges, li)
return nothing
end
Expand Down
4 changes: 4 additions & 0 deletions base/compiler/optimize.jl
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ intersect!(et::EdgeTracker, range::WorldRange) =
et.valid_worlds[] = intersect(et.valid_worlds[], range)

push!(et::EdgeTracker, mi::MethodInstance) = push!(et.edges, mi)
function add_edge!(et::EdgeTracker, @nospecialize(invokesig), mi::MethodInstance)
invokesig === nothing && return push!(et.edges, mi)
push!(et.edges, invokesig, mi)
end
function push!(et::EdgeTracker, ci::CodeInstance)
intersect!(et, WorldRange(min_world(li), max_world(li)))
push!(et, ci.def)
Expand Down
31 changes: 17 additions & 14 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,21 @@ pass to apply its own inlining policy decisions.
struct DelayedInliningSpec
match::Union{MethodMatch, InferenceResult}
argtypes::Vector{Any}
invokesig # either nothing or a signature (signature is for an `invoke` call)
end
DelayedInliningSpec(match, argtypes) = DelayedInliningSpec(match, argtypes, nothing)

struct InliningTodo
# The MethodInstance to be inlined
mi::MethodInstance
spec::Union{ResolvedInliningSpec, DelayedInliningSpec}
end

InliningTodo(mi::MethodInstance, match::MethodMatch, argtypes::Vector{Any}) =
InliningTodo(mi, DelayedInliningSpec(match, argtypes))
InliningTodo(mi::MethodInstance, match::MethodMatch, argtypes::Vector{Any}, invokesig=nothing) =
InliningTodo(mi, DelayedInliningSpec(match, argtypes, invokesig))

InliningTodo(result::InferenceResult, argtypes::Vector{Any}) =
InliningTodo(result.linfo, DelayedInliningSpec(result, argtypes))
InliningTodo(result::InferenceResult, argtypes::Vector{Any}, invokesig=nothing) =
InliningTodo(result.linfo, DelayedInliningSpec(result, argtypes, invokesig))

struct ConstantCase
val::Any
Expand Down Expand Up @@ -810,15 +812,15 @@ end

function resolve_todo(todo::InliningTodo, state::InliningState, flag::UInt8)
mi = todo.mi
(; match, argtypes) = todo.spec::DelayedInliningSpec
(; match, argtypes, invokesig) = todo.spec::DelayedInliningSpec
et = state.et

#XXX: update_valid_age!(min_valid[1], max_valid[1], sv)
if isa(match, InferenceResult)
inferred_src = match.src
if isa(inferred_src, ConstAPI)
# use constant calling convention
et !== nothing && push!(et, mi)
et !== nothing && add_edge!(et, invokesig, mi)
return ConstantCase(quoted(inferred_src.val))
else
src = inferred_src # ::Union{Nothing,CodeInfo} for NativeInterpreter
Expand All @@ -829,7 +831,7 @@ function resolve_todo(todo::InliningTodo, state::InliningState, flag::UInt8)
if code isa CodeInstance
if use_const_api(code)
# in this case function can be inlined to a constant
et !== nothing && push!(et, mi)
et !== nothing && add_edge!(et, invokesig, mi)
return ConstantCase(quoted(code.rettype_const))
else
src = @atomic :monotonic code.inferred
Expand All @@ -851,7 +853,7 @@ function resolve_todo(todo::InliningTodo, state::InliningState, flag::UInt8)

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

et !== nothing && push!(et, mi)
et !== nothing && add_edge!(et, invokesig, mi)
return InliningTodo(mi, retrieve_ir_for_inlining(mi, src), effects)
end

Expand All @@ -873,7 +875,7 @@ function validate_sparams(sparams::SimpleVector)
return true
end

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

todo = InliningTodo(mi, match, argtypes)
todo = InliningTodo(mi, match, argtypes, invokesig)
# If we don't have caches here, delay resolving this MethodInstance
# until the batch inlining step (or an external post-processing pass)
state.mi_cache === nothing && return todo
Expand Down Expand Up @@ -1100,17 +1102,18 @@ function inline_invoke!(
if isa(result, ConcreteResult)
item = concrete_result_item(result, state)
else
invokesig = invoke_signature(sig.argtypes)
argtypes = invoke_rewrite(sig.argtypes)
if isa(result, ConstPropResult)
(; mi) = item = InliningTodo(result.result, argtypes)
(; mi) = item = InliningTodo(result.result, argtypes, invokesig)
validate_sparams(mi.sparam_vals) || return nothing
if argtypes_to_type(argtypes) <: mi.def.sig
state.mi_cache !== nothing && (item = resolve_todo(item, state, flag))
handle_single_case!(ir, idx, stmt, item, todo, state.params, true)
return nothing
end
end
item = analyze_method!(match, argtypes, flag, state)
item = analyze_method!(match, argtypes, invokesig, flag, state)
end
handle_single_case!(ir, idx, stmt, item, todo, state.params, true)
return nothing
Expand Down Expand Up @@ -1328,7 +1331,7 @@ function handle_match!(
# during abstract interpretation: for the purpose of inlining, we can just skip
# processing this dispatch candidate
_any(case->case.sig === spec_types, cases) && return true
item = analyze_method!(match, argtypes, flag, state)
item = analyze_method!(match, argtypes, nothing, flag, state)
item === nothing && return false
push!(cases, InliningCase(spec_types, item))
return true
Expand Down Expand Up @@ -1475,7 +1478,7 @@ function assemble_inline_todo!(ir::IRCode, state::InliningState)
if isa(result, ConcreteResult)
item = concrete_result_item(result, state)
else
item = analyze_method!(info.match, sig.argtypes, flag, state)
item = analyze_method!(info.match, sig.argtypes, nothing, flag, state)
end
handle_single_case!(ir, idx, stmt, item, todo, state.params)
end
Expand Down
13 changes: 5 additions & 8 deletions base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,9 @@ function CodeInstance(
const_flags = 0x00
end
end
relocatability = isa(inferred_result, Vector{UInt8}) ? inferred_result[end] : UInt8(0)
relocatability = isa(inferred_result, Vector{UInt8}) ? inferred_result[end] :
inferred_result === nothing ? UInt8(1) : UInt8(0)
# relocatability = isa(inferred_result, Vector{UInt8}) ? inferred_result[end] : UInt8(0)
return CodeInstance(result.linfo,
widenconst(result_type), rettype_const, inferred_result,
const_flags, first(valid_worlds), last(valid_worlds),
Expand Down Expand Up @@ -561,17 +563,12 @@ function store_backedges(frame::InferenceResult, edges::Vector{Any})
end

function store_backedges(caller::MethodInstance, edges::Vector{Any})
i = 1
while i <= length(edges)
to = edges[i]
for (typ, to) in BackedgeIterator(edges)
if isa(to, MethodInstance)
ccall(:jl_method_instance_add_backedge, Cvoid, (Any, Any), to, caller)
i += 1
ccall(:jl_method_instance_add_backedge, Cvoid, (Any, Any, Any), to, typ, caller)
else
typeassert(to, Core.MethodTable)
typ = edges[i + 1]
ccall(:jl_method_table_add_backedge, Cvoid, (Any, Any, Any), to, typ, caller)
i += 2
end
end
end
Expand Down
59 changes: 59 additions & 0 deletions base/compiler/utilities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,65 @@ Check if `method` is declared as `Base.@constprop :none`.
"""
is_no_constprop(method::Union{Method,CodeInfo}) = method.constprop == 0x02

#############
# backedges #
#############

"""
BackedgeIterator(mi::MethodInstance)
BackedgeIterator(backedges::Vector{Any})
Return an iterator over a list of backedges, which may be extracted
from `mi`. Iteration returns `(sig, caller)` elements, which will be one of
the following:
- `(nothing, caller::MethodInstance)`: a call made by ordinary inferrable dispatch
- `(invokesig, caller::MethodInstance)`: a call made by `invoke(f, invokesig, args...)`
- `(specsig, mt::MethodTable)`: an abstract call
# Examples
```julia
julia> callme(x) = x+1
callme (generic function with 1 method)
julia> callyou(x) = callme(x)
callyou (generic function with 1 method)
julia> callyou(2.0)
3.0
julia> mi = first(which(callme, (Any,)).specializations)
MethodInstance for callme(::Float64)
julia> @eval Core.Compiler for (sig, caller) in BackedgeIterator(Main.mi)
println(sig)
println(caller)
end
nothing
callyou(Float64) from callyou(Any)
```
"""
struct BackedgeIterator
backedges::Vector{Any}
end

const empty_backedge_iter = BackedgeIterator(Any[])

function BackedgeIterator(mi::MethodInstance)
isdefined(mi, :backedges) || return empty_backedge_iter
return BackedgeIterator(mi.backedges)
end

function iterate(iter::BackedgeIterator, i::Int=1)
backedges = iter.backedges
i > length(backedges) && return nothing
item = backedges[i]
isa(item, MethodInstance) && return (nothing, item), i+1 # regular dispatch
isa(item, Core.MethodTable) && return (backedges[i+1], item), i+2 # abstract dispatch
return (item, backedges[i+1]::MethodInstance), i+2 # `invoke` calls
end

#########
# types #
#########
Expand Down
Loading

0 comments on commit 7080f68

Please sign in to comment.