Skip to content

Commit

Permalink
allow external absint to hold custom data in codeinst.inferred
Browse files Browse the repository at this point in the history
It has been possible for external abstract interpreters to keep custom
data in `codeinst.inferred` (together /w overloading `inlining_policy`).
After #52233, when such external absint uses
`InternalCodeCache`, this data is passed to `jl_ir_flag_inferred`,
leading to segfaults in assertion builds.

This commit resolves the issue by omitting `jl_ir_flag_inferred` checks
when the `cache_owner` is external. Nonetheless, a better resolution
might be necessary. It suggests that the current design of `code_owner`
and `InternalCodeCache` for the external cache system is somewhat flawed.
A conceivable approach could involve:
- Adding a layer similar to `inlining_policy` in `CC.get(::WorldView{InternalCodeCache})`
  to enable safe redirection of custom data to the native interpreter's
  implementation.
- Prohibiting custom data in the `inferred` field and directing such
  data to be kept in `analysis_results`.
  • Loading branch information
aviatesk committed Feb 12, 2024
1 parent 29a58d5 commit 0ec75bb
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 9 deletions.
9 changes: 3 additions & 6 deletions base/compiler/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -373,20 +373,17 @@ end
function NativeInterpreter(world::UInt = get_world_counter();
inf_params::InferenceParams = InferenceParams(),
opt_params::OptimizationParams = OptimizationParams())
curr_world = get_world_counter()
# Sometimes the caller is lazy and passes typemax(UInt).
# we cap it to the current world age for correctness
if world == typemax(UInt)
world = get_world_counter()
world = curr_world
end

# If they didn't pass typemax(UInt) but passed something more subtly
# incorrect, fail out loudly.
@assert world <= get_world_counter()

@assert world <= curr_world
method_table = CachedMethodTable(InternalMethodTable(world))

inf_cache = Vector{InferenceResult}() # Initially empty cache

return NativeInterpreter(world, method_table, inf_cache, inf_params, opt_params)
end

Expand Down
7 changes: 5 additions & 2 deletions src/gf.c
Original file line number Diff line number Diff line change
Expand Up @@ -444,8 +444,11 @@ STATIC_INLINE jl_value_t *_jl_rettype_inferred(jl_value_t *owner, jl_method_inst
if (jl_atomic_load_relaxed(&codeinst->min_world) <= min_world &&
max_world <= jl_atomic_load_relaxed(&codeinst->max_world) &&
jl_egal(codeinst->owner, owner)) {
jl_value_t *code = jl_atomic_load_relaxed(&codeinst->inferred);
if (code && (code == jl_nothing || jl_ir_flag_inferred(code)))
jl_value_t *inferred = jl_atomic_load_relaxed(&codeinst->inferred);
if (inferred && ((inferred == jl_nothing) || (
// allow whatever code instance external abstract interpreter produced
// since `jl_ir_flag_inferred` is specific to the native interpreter
codeinst->owner != jl_nothing || jl_ir_flag_inferred(inferred))))
return (jl_value_t*)codeinst;
}
codeinst = jl_atomic_load_relaxed(&codeinst->next);
Expand Down
33 changes: 32 additions & 1 deletion test/compiler/AbstractInterpreter.jl
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,6 @@ end
Core.eval(Core.Compiler, quote f(;a=1) = a end)
@test_throws MethodError Core.Compiler.f(;b=2)


# Custom lookup function
# ======================

Expand Down Expand Up @@ -469,3 +468,35 @@ let # generate cache
@test occursin("j_sin_", s)
@test !occursin("j_cos_", s)
end

# custom inferred data
# ====================

@newinterp CustomDataInterp
struct CustomDataInterpToken end
CC.cache_owner(::CustomDataInterp) = CustomDataInterpToken()
struct CustomData
inferred
CustomData(@nospecialize inferred) = new(inferred)
end
function CC.transform_result_for_cache(interp::CustomDataInterp,
mi::Core.MethodInstance, valid_worlds::CC.WorldRange, result::CC.InferenceResult)
inferred_result = @invoke CC.transform_result_for_cache(interp::CC.AbstractInterpreter,
mi::Core.MethodInstance, valid_worlds::CC.WorldRange, result::CC.InferenceResult)
return CustomData(inferred_result)
end
function CC.inlining_policy(interp::CustomDataInterp, @nospecialize(src),
@nospecialize(info::CC.CallInfo), stmt_flag::UInt32)
if src isa CustomData
src = src.inferred
end
return @invoke CC.inlining_policy(interp::CC.AbstractInterpreter, src::Any,
info::CC.CallInfo, stmt_flag::UInt32)
end
let src = code_typed((Int,); interp=CustomDataInterp()) do x
return sin(x) + cos(x)
end |> only |> first
@test count(isinvoke(:sin), src.code) == 1
@test count(isinvoke(:cos), src.code) == 1
@test count(isinvoke(:+), src.code) == 0
end

0 comments on commit 0ec75bb

Please sign in to comment.