Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use tagged CodeInstances for AbstractInterpreter caching #52233

Merged
merged 3 commits into from
Feb 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions base/boot.jl
Original file line number Diff line number Diff line change
Expand Up @@ -483,13 +483,13 @@ eval(Core, quote
end)

function CodeInstance(
mi::MethodInstance, @nospecialize(rettype), @nospecialize(exctype), @nospecialize(inferred_const),
mi::MethodInstance, owner, @nospecialize(rettype), @nospecialize(exctype), @nospecialize(inferred_const),
@nospecialize(inferred), const_flags::Int32, min_world::UInt, max_world::UInt,
ipo_effects::UInt32, effects::UInt32, @nospecialize(analysis_results),
relocatability::UInt8)
return ccall(:jl_new_codeinst, Ref{CodeInstance},
(Any, Any, Any, Any, Any, Int32, UInt, UInt, UInt32, UInt32, Any, UInt8),
mi, rettype, exctype, inferred_const, inferred, const_flags, min_world, max_world,
(Any, Any, Any, Any, Any, Any, Int32, UInt, UInt, UInt32, UInt32, Any, UInt8),
mi, owner, rettype, exctype, inferred_const, inferred, const_flags, min_world, max_world,
ipo_effects, effects, analysis_results,
relocatability)
end
Expand Down
17 changes: 12 additions & 5 deletions base/compiler/cicache.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ Internally, each `MethodInstance` keep a unique global cache of code instances
that have been created for the given method instance, stratified by world age
ranges. This struct abstracts over access to this cache.
"""
struct InternalCodeCache end
struct InternalCodeCache
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm thinking it might be a good idea to rename CC.InternalCodeCache to either CC.CodeCache or CC.GlobalCodeCache, because, following this PR, it's expected that almost all external abstract interpreters will start using the caching system provided by CC.InternalCodeCache and then "internal" sounds a bit too private. Additionally, it might also be more reasonable to rename CC.GLOBAL_CI_CACHE to something along the lines of CC.NATIVE_CODE_CACHE.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Upon further thought, now I even think that we don't need the code_cache(interp::AbstractInterpreter) API requirement anymore? Since now we can define the default implementation as

code_cache(interp::AbstractInterpreter) = 
    WorldView(InternalCodeCache(cache_owner(interp)), WorldRange(get_inference_world(interp)))

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think on d5c5248? Happy to revert it if you don't like it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks like a great simplification.

owner::Any # `jl_egal` is used for comparison
end

function setindex!(cache::InternalCodeCache, ci::CodeInstance, mi::MethodInstance)
@assert ci.owner === cache.owner
vchuravy marked this conversation as resolved.
Show resolved Hide resolved
ccall(:jl_mi_cache_insert, Cvoid, (Any, Any), mi, ci)
return cache
end

const GLOBAL_CI_CACHE = InternalCodeCache()

struct WorldRange
min_world::UInt
max_world::UInt
Expand Down Expand Up @@ -49,11 +50,11 @@ WorldView(wvc::WorldView, wr::WorldRange) = WorldView(wvc.cache, wr)
WorldView(wvc::WorldView, args...) = WorldView(wvc.cache, args...)

function haskey(wvc::WorldView{InternalCodeCache}, mi::MethodInstance)
return ccall(:jl_rettype_inferred, Any, (Any, UInt, UInt), mi, first(wvc.worlds), last(wvc.worlds)) !== nothing
return ccall(:jl_rettype_inferred, Any, (Any, Any, UInt, UInt), wvc.cache.owner, mi, first(wvc.worlds), last(wvc.worlds)) !== nothing
end

function get(wvc::WorldView{InternalCodeCache}, mi::MethodInstance, default)
r = ccall(:jl_rettype_inferred, Any, (Any, UInt, UInt), mi, first(wvc.worlds), last(wvc.worlds))
r = ccall(:jl_rettype_inferred, Any, (Any, Any, UInt, UInt), wvc.cache.owner, mi, first(wvc.worlds), last(wvc.worlds))
if r === nothing
return default
end
Expand All @@ -70,3 +71,9 @@ function setindex!(wvc::WorldView{InternalCodeCache}, ci::CodeInstance, mi::Meth
setindex!(wvc.cache, ci, mi)
return wvc
end

function code_cache(interp::AbstractInterpreter)
cache = InternalCodeCache(cache_owner(interp))
worlds = WorldRange(get_inference_world(interp))
return WorldView(cache, worlds)
end
2 changes: 1 addition & 1 deletion base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ function CodeInstance(interp::AbstractInterpreter, result::InferenceResult,
end
end
# relocatability = isa(inferred_result, String) ? inferred_result[end] : UInt8(0)
return CodeInstance(result.linfo,
return CodeInstance(result.linfo, cache_owner(interp),
widenconst(result_type), widenconst(result.exc_result), rettype_const, inferred_result,
const_flags, first(valid_worlds), last(valid_worlds),
# TODO: Actually do something with non-IPO effects
Expand Down
4 changes: 2 additions & 2 deletions base/compiler/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ the following methods to satisfy the `AbstractInterpreter` API requirement:
- `OptimizationParams(interp::NewInterpreter)` - return an `OptimizationParams` instance
- `get_inference_world(interp::NewInterpreter)` - return the world age for this interpreter
- `get_inference_cache(interp::NewInterpreter)` - return the local inference cache
- `code_cache(interp::NewInterpreter)` - return the global inference cache
- `cache_owner(interp::NewInterpreter)` - return the owner of any new cache entries
"""
:(AbstractInterpreter)

Expand Down Expand Up @@ -404,7 +404,7 @@ InferenceParams(interp::NativeInterpreter) = interp.inf_params
OptimizationParams(interp::NativeInterpreter) = interp.opt_params
get_inference_world(interp::NativeInterpreter) = interp.world
get_inference_cache(interp::NativeInterpreter) = interp.inf_cache
code_cache(interp::NativeInterpreter) = WorldView(GLOBAL_CI_CACHE, get_inference_world(interp))
cache_owner(interp::NativeInterpreter) = nothing

"""
already_inferred_quick_test(::AbstractInterpreter, ::MethodInstance)
Expand Down
19 changes: 0 additions & 19 deletions base/compiler/utilities.jl
Original file line number Diff line number Diff line change
Expand Up @@ -321,25 +321,6 @@ function iterate(iter::BackedgeIterator, i::Int=1)
return BackedgePair(item, backedges[i+1]::MethodInstance), i+2 # `invoke` calls
end

"""
add_invalidation_callback!(callback, mi::MethodInstance)

Register `callback` to be triggered upon the invalidation of `mi`.
`callback` should a function taking two arguments, `callback(replaced::MethodInstance, max_world::UInt32)`,
and it will be recursively invoked on `MethodInstance`s within the invalidation graph.
"""
function add_invalidation_callback!(@nospecialize(callback), mi::MethodInstance)
if !isdefined(mi, :callbacks)
callbacks = mi.callbacks = Any[callback]
else
callbacks = mi.callbacks::Vector{Any}
if !any(@nospecialize(cb)->cb===callback, callbacks)
push!(callbacks, callback)
end
end
return callbacks
end

#########
# types #
#########
Expand Down
9 changes: 9 additions & 0 deletions base/reflection.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1332,6 +1332,15 @@ function method_instances(@nospecialize(f), @nospecialize(t), world::UInt)
return results
end

function method_instance(@nospecialize(f), @nospecialize(t);
world=Base.get_world_counter(), method_table=nothing)
tt = signature_type(f, t)
mi = ccall(:jl_method_lookup_by_tt, Any,
(Any, Csize_t, Any),
tt, world, method_table)
return mi::Union{Nothing, MethodInstance}
end

default_debug_info_kind() = unsafe_load(cglobal(:jl_default_debug_info_kind, Cint))

# this type mirrors jl_cgparams_t (documented in julia.h)
Expand Down
4 changes: 4 additions & 0 deletions doc/src/devdocs/ast.md
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,10 @@ for important details on how to modify these fields safely.

The `MethodInstance` that this cache entry is derived from.

* `owner`

A token that represents the owner of this `CodeInstance`. Will use `jl_egal` to match.


* `rettype`/`rettype_const`

Expand Down
1 change: 1 addition & 0 deletions doc/src/devdocs/locks.md
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ MethodInstance/CodeInstance updates : Method->writelock, codegen lock
> * specTypes
> * sparam_vals
> * def
> * owner

> * These are set by `jl_type_infer` (while holding codegen lock):
> * cache
Expand Down
1 change: 1 addition & 0 deletions src/builtins.c
Original file line number Diff line number Diff line change
Expand Up @@ -2378,6 +2378,7 @@ jl_fptr_args_t jl_get_builtin_fptr(jl_datatype_t *dt)
jl_typemap_entry_t *entry = (jl_typemap_entry_t*)jl_atomic_load_relaxed(&dt->name->mt->defs);
jl_method_instance_t *mi = jl_atomic_load_relaxed(&entry->func.method->unspecialized);
jl_code_instance_t *ci = jl_atomic_load_relaxed(&mi->cache);
assert(ci->owner == jl_nothing);
return jl_atomic_load_relaxed(&ci->specptr.fptr1);
}

Expand Down
Loading