Skip to content

Commit

Permalink
staticdata: Insert backedges recursively (#57212)
Browse files Browse the repository at this point in the history
In the new edges scheme, when we perform constant propagation, we create
a dummy CodeInstance that is not inserted into the specializations
cache, but instead simply serves as a container for all edges that were
encountered during the constant propagation. These CodeInstances are not
part of either the internal or external edges list collected during
pkgimage generation. As such, while we were verifying edges recursively
and would thus catch an invalidation prior to package image reload, we
were failing to also insert backedges for these CodeInstance. We were
thus failing to invalidate such methods if the method was redefined
after re-load of the package image. Fix that by moving the storing of
the backedges to the end of the validation code, so that it too happens
recursively.
  • Loading branch information
Keno authored Jan 31, 2025
1 parent 3952c2c commit dbe19e4
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 22 deletions.
39 changes: 17 additions & 22 deletions base/staticdata.jl
Original file line number Diff line number Diff line change
Expand Up @@ -39,26 +39,16 @@ function _insert_backedges(edges::Vector{Any}, stack::Vector{CodeInstance}, visi
verify_method_graph(codeinst, stack, visiting)
minvalid = codeinst.min_world
maxvalid = codeinst.max_world
if maxvalid minvalid
if get_world_counter() == maxvalid
# if this callee is still valid, add all the backedges
Base.Compiler.store_backedges(codeinst, codeinst.edges)
end
if get_world_counter() == maxvalid
maxvalid = typemax(UInt)
@atomic :monotonic codeinst.max_world = maxvalid
end
if external
caller = get_ci_mi(codeinst)
@assert isdefined(codeinst, :inferred) # See #53586, #53109
inferred = @ccall jl_rettype_inferred(
codeinst.owner::Any, caller::Any, minvalid::UInt, maxvalid::UInt)::Any
if inferred !== nothing
# We already got a code instance for this world age range from
# somewhere else - we don't need this one.
else
@ccall jl_mi_cache_insert(caller::Any, codeinst::Any)::Cvoid
end
if maxvalid minvalid && external
caller = get_ci_mi(codeinst)
@assert isdefined(codeinst, :inferred) # See #53586, #53109
inferred = @ccall jl_rettype_inferred(
codeinst.owner::Any, caller::Any, minvalid::UInt, maxvalid::UInt)::Any
if inferred !== nothing
# We already got a code instance for this world age range from
# somewhere else - we don't need this one.
else
@ccall jl_mi_cache_insert(caller::Any, codeinst::Any)::Cvoid
end
end
end
Expand Down Expand Up @@ -196,9 +186,14 @@ function verify_method(codeinst::CodeInstance, stack::Vector{CodeInstance}, visi
while length(stack) depth
child = pop!(stack)
if maxworld 0
@atomic :monotonic child.min_world = minworld
@atomic :monotonic child.min_world = minworld
end
if maxworld == current_world
Base.Compiler.store_backedges(child, child.edges)
@atomic :monotonic child.max_world = typemax(UInt)
else
@atomic :monotonic child.max_world = maxworld
end
@atomic :monotonic child.max_world = maxworld
@assert visiting[child] == length(stack) + 1
delete!(visiting, child)
invalidations = _jl_debug_method_invalidation[]
Expand Down
33 changes: 33 additions & 0 deletions test/precompile.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2222,4 +2222,37 @@ precompile_test_harness("No package module") do load_path
String(take!(io)))
end

precompile_test_harness("Constprop CodeInstance invalidation") do load_path
write(joinpath(load_path, "DefineTheMethod.jl"),
"""
module DefineTheMethod
export the_method
the_method_val(::Val{x}) where {x} = x
the_method_val(::Val{1}) = 0xdeadbeef
the_method_val(::Val{2}) = 2
the_method_val(::Val{3}) = 3
the_method_val(::Val{4}) = 4
the_method_val(::Val{5}) = 5
Base.@constprop :aggressive the_method(x) = the_method_val(Val{x}())
the_method(2)
end
""")
Base.compilecache(Base.PkgId("DefineTheMethod"))
write(joinpath(load_path, "CallTheMethod.jl"),
"""
module CallTheMethod
using DefineTheMethod
call_the_method() = the_method(1)
call_the_method()
end
""")
Base.compilecache(Base.PkgId("CallTheMethod"))
@eval using DefineTheMethod
@eval using CallTheMethod
@eval DefineTheMethod.the_method_val(::Val{1}) = Int(0)
invokelatest() do
@test Int(0) == CallTheMethod.call_the_method()
end
end

finish_precompile_test!()

0 comments on commit dbe19e4

Please sign in to comment.