Skip to content

Commit

Permalink
Remove superflous method lookup in inlining
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Keno committed Jun 29, 2020
1 parent ab520c7 commit 438ed91
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 438ed91

Please sign in to comment.