Skip to content

Commit

Permalink
inference: Don't confuse frames in different interpreters (#53627)
Browse files Browse the repository at this point in the history
Diffractor's abstract interpreter sometimes needs to do side queries
using the native interpreter. These are pushed onto the regular
inference callstack in anticipation of a future where compiler plugins
may want to recurse from the native interpreter back into the Diffractor
abstract interpreter. However, this introduced a subtle challenge: When
the native interpreter is looking at a frame that is currently on the
inference stack, it would treat them as the same, incorrectly merging
inference across the two abstract interpreters (which have different
semantics and may not be confused). The caches for the two abstract
interpreters were already different, so once things are inferred,
there's no problem (likely because things were already inferred on the
native interpreter), but if not, this could cause subtle and hard to
debug problems.
  • Loading branch information
Keno authored Mar 8, 2024
1 parent 321fb2c commit 4dcf357
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 1 deletion.
4 changes: 4 additions & 0 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -689,6 +689,10 @@ function edge_matches_sv(interp::AbstractInterpreter, frame::AbsIntState,
if callee_method2 !== inf_method2
return false
end
if isa(frame, InferenceState) && cache_owner(frame.interp) !== cache_owner(interp)
# Don't assume that frames in different interpreters are the same
return false
end
if !hardlimit || InferenceParams(interp).ignore_recursion_hardlimit
# if this is a soft limit,
# also inspect the parent of this edge,
Expand Down
11 changes: 11 additions & 0 deletions base/compiler/inferencestate.jl
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,16 @@ function empty_backedges!(frame::InferenceState, currpc::Int=frame.currpc)
end

function print_callstack(sv::InferenceState)
print("=================== Callstack: ==================\n")
idx = 0
while sv !== nothing
print("[")
print(idx)
if !isa(sv.interp, NativeInterpreter)
print(", ")
print(typeof(sv.interp))
end
print("] ")
print(sv.linfo)
is_cached(sv) || print(" [uncached]")
println()
Expand All @@ -740,7 +749,9 @@ function print_callstack(sv::InferenceState)
println()
end
sv = sv.parent
idx += 1
end
print("================= End callstack ==================\n")
end

function narguments(sv::InferenceState, include_va::Bool=true)
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ function merge_call_chain!(interp::AbstractInterpreter, parent::InferenceState,
end

function is_same_frame(interp::AbstractInterpreter, mi::MethodInstance, frame::InferenceState)
return mi === frame_instance(frame)
return mi === frame_instance(frame) && cache_owner(interp) === cache_owner(frame.interp)
end

function poison_callstack!(infstate::InferenceState, topmost::InferenceState)
Expand Down
6 changes: 6 additions & 0 deletions base/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2888,6 +2888,12 @@ show(io::IO, ::Core.Compiler.NativeInterpreter) =
show(io::IO, cache::Core.Compiler.CachedMethodTable) =
print(io, typeof(cache), "(", Core.Compiler.length(cache.cache), " entries)")

function show(io::IO, limited::Core.Compiler.LimitedAccuracy)
print(io, "Core.Compiler.LimitedAccuracy(")
show(io, limited.typ)
print(io, ", #= ", Core.Compiler.length(limited.causes), " cause(s) =#)")
end

function dump(io::IOContext, x::SimpleVector, n::Int, indent)
if isempty(x)
print(io, "empty SimpleVector")
Expand Down

0 comments on commit 4dcf357

Please sign in to comment.