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

In the analysis performed by `OptAnalyzer`, when the
`skip_unoptimized_throw_blocks` configuration is enabled, dynamic
dispatch on `throw` code path is supposed to be ignored. However,
cached reports of non-inlined callees on the `throw` code path were not
ignored, leading to confusion (#643). This commit addresses the issue by
overloading the propagation of reports from callees in abstract
interpretation.

While the issue has been resolved, the resulting code is quite hacky,
and improvements to these interfaces are certainly desirable.

src/abstractinterpret/abstractanalyzer.jl

@@ -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
@@ -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 ##
 
@@ -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,

src/abstractinterpret/typeinfer.jl

@@ -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,
@@ -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})
@@ -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
@@ -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)
@@ -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)
@@ -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

src/analyzers/optanalyzer.jl

@@ -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)

test/analyzers/test_optanalyzer.jl

@@ -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