Skip to content

Commit 86fc8b6

Browse files
committed
inlining: make union splitting account for uncovered call (#48455)
#44421 changed the union-splitting to skip generating unnecessary fallback dynamic dispatch call when there is any fully covered call. But it turned out that this is only valid when there is any fully covered call in matches for all signatures that inference split, and it is invalid if there is any union split signature against which any uncovered call is found. Consider the following example: # case 1 # def nosplit(::Any) = [...] nosplit(::Int) = [...] # call nosplit(a::Any) split1: a::Any ┬ nosplit(a::Int) └ nosplit(a::Any) # fully covers split1 # case 2 # def convert(::Type{T}, ::T) = T # call convert(::Type{Union{Bool,Tuple{Int,String}}}, a::Union{Bool,Tuple{Int,Any}}) split1: a::Bool ─ convert(::Type{Bool}, ::Bool) # fully covers split1 split2: a::Tuple{Int,Any} ─ convert(::Type{Tuple{Int,String}}, ::Tuple{Int,String}) # NOT fully covers split2 #44421 allows us to optimize the the first case, but handles the second case wrongly. This commit fixes it up while still optimizing the first case. fix #48397.
1 parent 9a67956 commit 86fc8b6

File tree

2 files changed

+26
-7
lines changed

2 files changed

+26
-7
lines changed

base/compiler/ssair/inlining.jl

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,14 +1348,13 @@ function compute_inlining_cases(@nospecialize(info::CallInfo), flag::UInt8, sig:
13481348
nunion === nothing && return nothing
13491349
cases = InliningCase[]
13501350
argtypes = sig.argtypes
1351-
local any_fully_covered = false
13521351
local handled_all_cases::Bool = true
13531352
local revisit_idx = nothing
13541353
local only_method = nothing
13551354
local meth::MethodLookupResult
13561355
local all_result_count = 0
13571356
local joint_effects::Effects = EFFECTS_TOTAL
1358-
local nothrow::Bool = true
1357+
local fully_covered::Bool = true
13591358
for i = 1:nunion
13601359
meth = getsplit(info, i)
13611360
if meth.ambig
@@ -1377,12 +1376,12 @@ function compute_inlining_cases(@nospecialize(info::CallInfo), flag::UInt8, sig:
13771376
only_method = false
13781377
end
13791378
end
1379+
local split_fully_covered::Bool = false
13801380
for (j, match) in enumerate(meth)
13811381
all_result_count += 1
13821382
result = getresult(info, all_result_count)
13831383
joint_effects = merge_effects(joint_effects, info_effects(result, match, state))
1384-
nothrow &= match.fully_covers
1385-
any_fully_covered |= match.fully_covers
1384+
split_fully_covered |= match.fully_covers
13861385
if !validate_sparams(match.sparams)
13871386
if !match.fully_covers
13881387
handled_all_cases = false
@@ -1399,9 +1398,10 @@ function compute_inlining_cases(@nospecialize(info::CallInfo), flag::UInt8, sig:
13991398
result, match, argtypes, info, flag, state; allow_abstract=true, allow_typevars=false)
14001399
end
14011400
end
1401+
fully_covered &= split_fully_covered
14021402
end
14031403

1404-
joint_effects = Effects(joint_effects; nothrow)
1404+
joint_effects = Effects(joint_effects; nothrow=fully_covered)
14051405

14061406
if handled_all_cases && revisit_idx !== nothing
14071407
# we handled everything except one match with unmatched sparams,
@@ -1428,13 +1428,13 @@ function compute_inlining_cases(@nospecialize(info::CallInfo), flag::UInt8, sig:
14281428
end
14291429
handle_any_const_result!(cases,
14301430
result, match, argtypes, info, flag, state; allow_abstract=true, allow_typevars=true)
1431-
any_fully_covered = handled_all_cases = match.fully_covers
1431+
fully_covered = handled_all_cases = match.fully_covers
14321432
elseif !handled_all_cases
14331433
# if we've not seen all candidates, union split is valid only for dispatch tuples
14341434
filter!(case::InliningCase->isdispatchtuple(case.sig), cases)
14351435
end
14361436

1437-
return cases, (handled_all_cases & any_fully_covered), joint_effects
1437+
return cases, (handled_all_cases & fully_covered), joint_effects
14381438
end
14391439

14401440
function handle_call!(todo::Vector{Pair{Int,Any}},

test/compiler/inline.jl

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1801,3 +1801,22 @@ let src = code_typed1(make_issue47349(Val{4}()), (Any,))
18011801
make_issue47349(Val(4))((x,nothing,Int))
18021802
end |> only === Type{Int}
18031803
end
1804+
1805+
# union splitting should account for uncovered call signature
1806+
# https://github.com/JuliaLang/julia/issues/48397
1807+
f48397(::Bool) = :ok
1808+
f48397(::Tuple{String,String}) = :ok
1809+
let src = code_typed1((Union{Bool,Tuple{String,Any}},)) do x
1810+
f48397(x)
1811+
end
1812+
@test any(iscall((src, f48397)), src.code)
1813+
end
1814+
g48397::Union{Bool,Tuple{String,Any}} = ("48397", 48397)
1815+
@test_throws MethodError let
1816+
Base.Experimental.@force_compile
1817+
f48397(g48397)
1818+
end
1819+
@test_throws MethodError let
1820+
Base.Experimental.@force_compile
1821+
convert(Union{Bool,Tuple{String,String}}, g48397)
1822+
end

0 commit comments

Comments
 (0)