From 438ed911c80bfce18ab3be20334fded4d4e1bb10 Mon Sep 17 00:00:00 2001 From: Keno Fischer Date: Sun, 28 Jun 2020 20:06:11 -0400 Subject: [PATCH] Remove superflous method lookup in inlining This code used to only do one method lookup, but in 8ca6e8d got changed to one method lookup per union split, to better match what inference does, only doing the big method lookup in the case where we want to inline a non-dispatchtuple signature. I'd like to go one step further and have inference forward all method lookup results to the optimizer. However, as a result the fallback to the big method lookup is problematic, because that same method lookup is not performed during inference. Luckily, we don't actually need it, we already know which method will get inlined, so all we need to do is to recompute the signature and type parameters, which is a simple call to type intersection. This code path isn't hit very often, but regardless of any future refactors it is also a performance improvement, since it was already doing this intersection internally, but this way gets to skip all the additional work of the method lookup. As a side node, there's an additional question of whether inference should be doing this one big method lookup or multiple. However, Jameson points out that this would currently pessimize some important cases, because of the following: ``` julia> Core.Compiler.switchtupleunion(Tuple{Union{Int,Float64},Union{Int,Float64}}) 4-element Array{Any,1}: Tuple{Float64,Float64} Tuple{Int64,Float64} Tuple{Float64,Int64} Tuple{Int64,Int64} julia> Core.Compiler.switchtupleunion(typeintersect(Tuple{Union{Int,Float64},Union{Int,Float64}}, Tuple{T,T} where T)) 1-element Array{Any,1}: Tuple{T,T} where T<:Union{Float64, Int64} ``` and potentially some other cases. Regardless, either way is fine for my planned refactor as long as the method lookups during inference match those during inlining. --- base/compiler/ssair/inlining.jl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/base/compiler/ssair/inlining.jl b/base/compiler/ssair/inlining.jl index 491d228aae9ff..37f885fb93f60 100644 --- a/base/compiler/ssair/inlining.jl +++ b/base/compiler/ssair/inlining.jl @@ -1094,13 +1094,13 @@ function assemble_inline_todo!(ir::IRCode, sv::OptimizationState) # we inline, even if the signature is not a dispatch tuple 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, min_valid, max_valid) = - matching_methods(sig.atype, sv.matching_methods_cache, sv.params.MAX_METHODS, sv.world) + method = only_method + (metharg, methsp) = ccall(:jl_type_intersection_with_env, Any, (Any, Any), + sig.atype, method.sig)::SimpleVector + else @assert length(meth) == 1 - update_valid_age!(min_valid, max_valid, sv) + (metharg, methsp, method) = (meth[1][1]::Type, meth[1][2]::SimpleVector, meth[1][3]::Method) end - (metharg, methsp, method) = (meth[1][1]::Type, meth[1][2]::SimpleVector, meth[1][3]::Method) fully_covered = true case = analyze_method!(idx, sig, metharg, methsp, method, stmt, sv, false, nothing, calltype)