From f2505c1adb58c4e54879b8bdc1367041d11c844b Mon Sep 17 00:00:00 2001 From: Jeff Bezanson Date: Tue, 23 Jun 2020 17:36:20 -0400 Subject: [PATCH] fix and de-dup cached calls to `methods_by_ftype` in compiler --- base/compiler/abstractinterpretation.jl | 41 +++++++++++++++---------- base/compiler/ssair/inlining.jl | 27 ++++++---------- 2 files changed, 35 insertions(+), 33 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 861758576aedd..b766c043dc514 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -16,8 +16,25 @@ const _REF_NAME = Ref.body.name call_result_unused(frame::InferenceState, pc::LineNum=frame.currpc) = isexpr(frame.src.code[frame.currpc], :call) && isempty(frame.ssavalue_uses[pc]) +function matching_methods(@nospecialize(atype), cache::IdDict{Any, Tuple{Any, UInt, UInt}}, max_methods::Int, world::UInt) + box = Core.Box(atype) + return get!(cache, atype) do + _min_val = UInt[typemin(UInt)] + _max_val = UInt[typemax(UInt)] + ms = _methods_by_ftype(box.contents, max_methods, world, _min_val, _max_val) + return ms, _min_val[1], _max_val[1] + end +end + +function matching_methods(@nospecialize(atype), cache::IdDict{Any, Tuple{Any, UInt, UInt}}, max_methods::Int, world::UInt, min_valid::Vector{UInt}, max_valid::Vector{UInt}) + ms, minvalid, maxvalid = matching_methods(atype, cache, max_methods, world) + min_valid[1] = max(min_valid[1], minvalid) + max_valid[1] = min(max_valid[1], maxvalid) + return ms +end + function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f), argtypes::Vector{Any}, @nospecialize(atype), sv::InferenceState, - max_methods = InferenceParams(interp).MAX_METHODS) + max_methods::Int = InferenceParams(interp).MAX_METHODS) atype_params = unwrap_unionall(atype).parameters ft = unwrap_unionall(atype_params[1]) # TODO: ccall jl_method_table_for here isa(ft, DataType) || return Any # the function being called is unknown. can't properly handle this backedge right now @@ -42,22 +59,14 @@ function abstract_call_gf_by_type(interp::AbstractInterpreter, @nospecialize(f), splitsigs = switchtupleunion(atype) applicable = Any[] for sig_n in splitsigs - (xapplicable, min_valid[1], max_valid[1]) = - get!(sv.matching_methods_cache, sig_n) do - ms = _methods_by_ftype(sig_n, max_methods, - get_world_counter(interp), min_valid, max_valid) - return (ms, min_valid[1], max_valid[1]) - end + xapplicable = matching_methods(sig_n, sv.matching_methods_cache, max_methods, + get_world_counter(interp), min_valid, max_valid) xapplicable === false && return Any append!(applicable, xapplicable) end else - (applicable, min_valid[1], max_valid[1]) = - get!(sv.matching_methods_cache, atype) do - ms = _methods_by_ftype(atype, max_methods, - get_world_counter(interp), min_valid, max_valid) - return (ms, min_valid[1], max_valid[1]) - end + applicable = matching_methods(atype, sv.matching_methods_cache, max_methods, + get_world_counter(interp), min_valid, max_valid) if applicable === false # this means too many methods matched # (assume this will always be true, so we don't compute / update valid age in this case) @@ -595,7 +604,7 @@ end # do apply(af, fargs...), where af is a function value function abstract_apply(interp::AbstractInterpreter, @nospecialize(itft), @nospecialize(aft), aargtypes::Vector{Any}, vtypes::VarTable, sv::InferenceState, - max_methods = InferenceParams(interp).MAX_METHODS) + max_methods::Int = InferenceParams(interp).MAX_METHODS) aftw = widenconst(aft) if !isa(aft, Const) && (!isType(aftw) || has_free_typevars(aftw)) if !isconcretetype(aftw) || (aftw <: Builtin) @@ -694,7 +703,7 @@ function argtype_tail(argtypes::Vector{Any}, i::Int) end # call where the function is known exactly -function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f), fargs::Union{Nothing,Vector{Any}}, argtypes::Vector{Any}, vtypes::VarTable, sv::InferenceState, max_methods = InferenceParams(interp).MAX_METHODS) +function abstract_call_known(interp::AbstractInterpreter, @nospecialize(f), fargs::Union{Nothing,Vector{Any}}, argtypes::Vector{Any}, vtypes::VarTable, sv::InferenceState, max_methods::Int = InferenceParams(interp).MAX_METHODS) la = length(argtypes) if isa(f, Builtin) @@ -911,7 +920,7 @@ end # call where the function is any lattice element function abstract_call(interp::AbstractInterpreter, fargs::Union{Nothing,Vector{Any}}, argtypes::Vector{Any}, - vtypes::VarTable, sv::InferenceState, max_methods = InferenceParams(interp).MAX_METHODS) + vtypes::VarTable, sv::InferenceState, max_methods::Int = InferenceParams(interp).MAX_METHODS) #print("call ", e.args[1], argtypes, "\n\n") ft = argtypes[1] if isa(ft, Const) diff --git a/base/compiler/ssair/inlining.jl b/base/compiler/ssair/inlining.jl index d5986c34bee1f..491d228aae9ff 100644 --- a/base/compiler/ssair/inlining.jl +++ b/base/compiler/ssair/inlining.jl @@ -1041,18 +1041,13 @@ function assemble_inline_todo!(ir::IRCode, sv::OptimizationState) local fully_covered = true for atype in splits # Regular case: Retrieve matching methods from cache (or compute them) - (meth, min_valid, max_valid) = get(sv.matching_methods_cache, atype) do - # World age does not need to be taken into account in the cache - # because it is forwarded from type inference through `sv.params` - # in the case that the cache is nonempty, so it should be unchanged - # The max number of methods should be the same as in inference most - # of the time, and should not affect correctness otherwise. - min_val = UInt[typemin(UInt)] - max_val = UInt[typemax(UInt)] - ms = _methods_by_ftype(atype, sv.params.MAX_METHODS, - sv.world, min_val, max_val) - return (ms, min_val[1], max_val[1]) - end + # World age does not need to be taken into account in the cache + # because it is forwarded from type inference through `sv.params` + # in the case that the cache is nonempty, so it should be unchanged + # The max number of methods should be the same as in inference most + # of the time, and should not affect correctness otherwise. + (meth, min_valid, max_valid) = + matching_methods(atype, sv.matching_methods_cache, sv.params.MAX_METHODS, sv.world) if meth === false # Too many applicable methods too_many = true @@ -1100,12 +1095,10 @@ function assemble_inline_todo!(ir::IRCode, sv::OptimizationState) if signature_fully_covered && length(cases) == 0 && only_method isa Method if length(splits) > 1 # get match information for a single overall match instead of union splits - meth = get(sv.matching_methods_cache, sig.atype) do - ms = _methods_by_ftype(sig.atype, sv.params.MAX_METHODS, - sv.world, UInt[typemin(UInt)], UInt[typemin(UInt)]) - return ms - end + (meth, min_valid, max_valid) = + matching_methods(sig.atype, sv.matching_methods_cache, sv.params.MAX_METHODS, sv.world) @assert length(meth) == 1 + update_valid_age!(min_valid, max_valid, sv) end (metharg, methsp, method) = (meth[1][1]::Type, meth[1][2]::SimpleVector, meth[1][3]::Method) fully_covered = true