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

OptAnalyzer: fix #643, ignore reports from callee on throw path #645

Merged
merged 1 commit into from
Jul 3, 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
7 changes: 2 additions & 5 deletions src/abstractinterpret/abstractanalyzer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,6 @@ end
a global cache maintained by `AbstractAnalyzer`. That means,
`codeinf::CodeInstance = Core.Compiler.code_cache(analyzer::AbstractAnalyzer)[mi::MethodInstance])`
is expected to have its field `codeinf.inferred::CachedAnalysisResult`.

[`InferenceErrorReport`](@ref)s found within already-analyzed `result::InferenceResult`
can be accessed with `get_cached_reports(analyzer, result)`.
"""
struct CachedAnalysisResult
src
Expand Down Expand Up @@ -130,7 +127,7 @@ mutable struct AnalyzerState

# the temporal stash to keep track of the context of caller inference/optimization and
# the caller itself, to which reconstructed cached reports will be appended
cache_target::Union{Nothing,Pair{Symbol,InferenceResult}}
cache_target::Union{Nothing,Pair{Symbol,InferenceState}}

## abstract toplevel execution ##

Expand Down Expand Up @@ -179,7 +176,7 @@ function AnalyzerState(world::UInt = get_world_counter();
#=opt_params::OptimizationParams=# opt_params,
#=results::IdDict{InferenceResult,AnyAnalysisResult}=# results,
#=report_stash::Vector{InferenceErrorReport}=# report_stash,
#=cache_target::Union{Nothing,InferenceResult}=# nothing,
#=cache_target::Union{Nothing,Pair{Symbol,InferenceState}}=# nothing,
#=concretized::BitVector=# concretized,
#=toplevelmod::Module=# toplevelmod,
#=global_slots::Dict{Int,Symbol}=# global_slots,
Expand Down
42 changes: 21 additions & 21 deletions src/abstractinterpret/typeinfer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,20 @@ function collect_callee_reports!(analyzer::AbstractAnalyzer, sv::InferenceState)
end
empty!(reports)
end
return nothing
end

function collect_cached_callee_reports!(analyzer::AbstractAnalyzer, reports::Vector{InferenceErrorReport},
caller::InferenceState, origin_mi::MethodInstance)
for cached in reports
restored = add_cached_report!(analyzer, caller.result, cached)
@static if JET_DEV_MODE
actual, expected = first(restored.vst).linfo, origin_mi
@assert actual === expected "invalid local cache restoration, expected $expected but got $actual"
end
stash_report!(analyzer, restored) # should be updated in `abstract_call_method_with_const_args`
end
return nothing
end

function CC.abstract_call_method(analyzer::AbstractAnalyzer,
Expand All @@ -26,7 +40,7 @@ end
function CC.const_prop_call(analyzer::AbstractAnalyzer,
mi::MethodInstance, result::MethodCallResult, arginfo::ArgInfo, sv::InferenceState,
concrete_eval_result::Union{Nothing,CC.ConstCallResults})
set_cache_target!(analyzer, :const_prop_call => sv.result)
set_cache_target!(analyzer, :const_prop_call => sv)
const_result = @invoke CC.const_prop_call(analyzer::AbstractInterpreter,
mi::MethodInstance, result::MethodCallResult, arginfo::ArgInfo, sv::InferenceState,
concrete_eval_result::Union{Nothing,CC.ConstCallResults})
Expand Down Expand Up @@ -165,7 +179,7 @@ AnalysisCache(wvc::WorldView{<:AbstractAnalyzerView}) = AnalysisCache(wvc.cache.
CC.haskey(wvc::WorldView{<:AbstractAnalyzerView}, mi::MethodInstance) = haskey(AnalysisCache(wvc), mi)

function CC.typeinf_edge(analyzer::AbstractAnalyzer, method::Method, @nospecialize(atype), sparams::SimpleVector, caller::InferenceState)
set_cache_target!(analyzer, :typeinf_edge => caller.result)
set_cache_target!(analyzer, :typeinf_edge => caller)
ret = @invoke CC.typeinf_edge(analyzer::AbstractInterpreter, method::Method, atype::Any, sparams::SimpleVector, caller::InferenceState)
@assert get_cache_target(analyzer) === nothing "invalid JET analysis state"
return ret
Expand All @@ -184,20 +198,13 @@ function CC.get(wvc::WorldView{<:AbstractAnalyzerView}, mi::MethodInstance, defa
# enable report cache reconstruction without the information
# XXX move this logic into `typeinf_edge`?
cache_target = get_cache_target(analyzer)
if isa(cache_target, Pair{Symbol,InferenceResult})
if cache_target !== nothing
context, caller = cache_target
if context === :typeinf_edge
if isa(codeinst, CodeInstance)
# cache hit, now we need to append cached reports associated with this `MethodInstance`
inferred = (@atomic :monotonic codeinst.inferred)::CachedAnalysisResult
for cached in inferred.reports
restored = add_cached_report!(analyzer, caller, cached)
@static if JET_DEV_MODE
actual, expected = first(restored.vst).linfo, mi
@assert actual === expected "invalid global cache restoration, expected $expected but got $actual"
end
stash_report!(analyzer, restored) # should be updated in `abstract_call` (after exiting `typeinf_edge`)
end
collect_cached_callee_reports!(analyzer, inferred.reports, caller, mi)
end
end
set_cache_target!(analyzer, nothing)
Expand Down Expand Up @@ -251,7 +258,7 @@ function (callback::JETCallback)(replaced::MethodInstance, max_world::UInt32,
delete!(callback.analysis_cache, replaced)
if isdefined(replaced, :backedges)
for item in replaced.backedges
isa(item, MethodInstance) || continue # might be `Type` object representing an `invoke` signature
item isa MethodInstance || continue # might be `Type` object representing an `invoke` signature
mi = item
mi in seen && continue # otherwise fail into an infinite loop
callback(mi, max_world, seen)
Expand Down Expand Up @@ -291,16 +298,9 @@ function CC.cache_lookup(𝕃ᵢ::CC.AbstractLattice, mi::MethodInstance, given_
# with the extended lattice elements, here we should throw-away the error reports
# that are collected during the previous non-constant abstract-interpretation
# (see the `CC.typeinf(::AbstractAnalyzer, ::InferenceState)` overload)
filter_lineages!(analyzer, caller, mi)
filter_lineages!(analyzer, caller.result, mi)

for cached in get_cached_reports(analyzer, inf_result)
restored = add_cached_report!(analyzer, caller, cached)
@static if JET_DEV_MODE
actual, expected = first(restored.vst).linfo, mi
@assert actual === expected "invalid local cache restoration, expected $expected but got $actual"
end
stash_report!(analyzer, restored) # should be updated in `abstract_call_method_with_const_args`
end
collect_cached_callee_reports!(analyzer, get_cached_reports(analyzer, inf_result), caller, mi)
end
return inf_result
end
Expand Down
17 changes: 17 additions & 0 deletions src/analyzers/optanalyzer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,23 @@ function CC.const_prop_call(analyzer::OptAnalyzer,
nothing::Nothing)
end

function collect_callee_reports!(analyzer::OptAnalyzer, sv::InferenceState)
if analyzer.skip_unoptimized_throw_blocks && CC.is_stmt_throw_block(CC.get_curr_ssaflag(sv))
empty!(get_report_stash(analyzer))
return nothing
end
@invoke collect_callee_reports!(analyzer::AbstractAnalyzer, sv::InferenceState)
return nothing
end
function collect_cached_callee_reports!(analyzer::OptAnalyzer, reports::Vector{InferenceErrorReport},
caller::InferenceState, origin_mi::MethodInstance)
if !(analyzer.skip_unoptimized_throw_blocks && CC.is_stmt_throw_block(CC.get_curr_ssaflag(caller)))
@invoke collect_cached_callee_reports!(analyzer::AbstractAnalyzer, reports::Vector{InferenceErrorReport},
caller::InferenceState, origin_mi::MethodInstance)
end
return nothing
end

# TODO better to work only `CC.finish!`
function CC.finish(frame::InferenceState, analyzer::OptAnalyzer)
ret = @invoke CC.finish(frame::InferenceState, analyzer::AbstractAnalyzer)
Expand Down
10 changes: 10 additions & 0 deletions test/analyzers/test_optanalyzer.jl
Original file line number Diff line number Diff line change
Expand Up @@ -325,4 +325,14 @@ const issue560μ = zeros(Issue560Vec3, 2, 3, 4, 5)
issue560f(μ) = reinterpret(reshape, Float64, μ)
@test_opt issue560f(issue560μ)

f_issue643_1(x) = throw("$x <- dynamic dispatches from this interpolation should be ignored")
@noinline _f_issue643_2(x) = "$x <- dynamic dispatches from this interpolation should be ignored"
f_issue643_2(x) = throw(_f_issue643_2(x))
test_opt(f_issue643_1, (Any,))
test_opt(f_issue643_1, (Any,)) # check cached case
test_opt(f_issue643_2, (Any,))
test_opt(f_issue643_2, (Any,)) # check cached case
# the dynamic dispatch should be reported if the method is analyzed standalone
@test !isempty(get_reports(report_opt(_f_issue643_2, (Any,))))

end # module test_optanalyzer
Loading