Skip to content

Commit

Permalink
inference: fixes cache lookup with extended lattice elements
Browse files Browse the repository at this point in the history
The previous local cache lookup system wasn't working well for caches
with extended lattice elements that are transformed from the caller
context to the callee context by `matching_cache_argtypes`. To address
this, the current commit moves the call to `matching_cache_argtypes`
right before `cache_lookup`, instead of within the `InferenceResult`
constructor.

Note that this adjustment leads to `given_argtypes` being allocated even
when there's a cache hit, potentially affecting performance negatively.
I'm looking to further optimize `matching_cache_argtypes` in an upcoming
commit.
  • Loading branch information
aviatesk committed Apr 6, 2024
1 parent 273d91e commit fabc166
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 5 deletions.
7 changes: 4 additions & 3 deletions base/compiler/abstractinterpretation.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1248,7 +1248,9 @@ function const_prop_call(interp::AbstractInterpreter,
concrete_eval_result::Union{Nothing, ConstCallResults}=nothing)
inf_cache = get_inference_cache(interp)
𝕃ᡒ = typeinf_lattice(interp)
inf_result = cache_lookup(𝕃ᡒ, mi, arginfo.argtypes, inf_cache)
argtypes = has_conditional(𝕃ᡒ, sv) ? ConditionalArgtypes(arginfo, sv) : SimpleArgtypes(arginfo.argtypes)
given_argtypes, overridden_by_const = matching_cache_argtypes(𝕃ᡒ, mi, argtypes)
inf_result = cache_lookup(𝕃ᡒ, mi, given_argtypes, inf_cache)
if inf_result !== nothing
# found the cache for this constant prop'
if inf_result.result === nothing
Expand All @@ -1259,8 +1261,7 @@ function const_prop_call(interp::AbstractInterpreter,
return return_cached_result(interp, inf_result, sv)
end
# perform fresh constant prop'
argtypes = has_conditional(𝕃ᡒ, sv) ? ConditionalArgtypes(arginfo, sv) : SimpleArgtypes(arginfo.argtypes)
inf_result = InferenceResult(mi, argtypes, typeinf_lattice(interp))
inf_result = InferenceResult(mi, given_argtypes, overridden_by_const)
if !any(inf_result.overridden_by_const)
add_remark!(interp, sv, "[constprop] Could not handle constant info in matching_cache_argtypes")
return nothing
Expand Down
2 changes: 0 additions & 2 deletions base/compiler/types.jl
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,6 @@ mutable struct InferenceResult
end
InferenceResult(mi::MethodInstance, 𝕃::AbstractLattice=fallback_lattice) =
InferenceResult(mi, matching_cache_argtypes(𝕃, mi)...)
InferenceResult(mi::MethodInstance, argtypes::ForwardableArgtypes, 𝕃::AbstractLattice=fallback_lattice) =
InferenceResult(mi, matching_cache_argtypes(𝕃, mi, argtypes)...)

function stack_analysis_result!(inf_result::InferenceResult, @nospecialize(result))
return inf_result.analysis_results = AnalysisResults(result, inf_result.analysis_results)
Expand Down
22 changes: 22 additions & 0 deletions test/compiler/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5696,3 +5696,25 @@ end
# We want to make sure that both this returns `Tuple` and that
# it doesn't infinite loop inside inference.
@test Core.Compiler.return_type(gen_infinite_loop_ssa, Tuple{}) === Tuple

# inference local cache lookup with extended lattice elements that may be transformed
# by `matching_cache_argtypes`
@newinterp CachedConditionalInterp
Base.@constprop :aggressive function func_cached_conditional(x, y)
if x
@noinline sin(y)
else
0.0
end
end;
function test_func_cached_conditional(y)
y₁ = func_cached_conditional(isa(y, Float64), y)
yβ‚‚ = func_cached_conditional(isa(y, Float64), y)
return y₁, yβ‚‚
end;
let interp = CachedConditionalInterp();
@test Base.infer_return_type(test_func_cached_conditional, (Any,); interp) == Tuple{Float64, Float64}
@test count(interp.inf_cache) do result
result.linfo.def.name === :func_cached_conditional
end == 1
end

0 comments on commit fabc166

Please sign in to comment.