Skip to content

Commit 07f16d5

Browse files
committed
inlining: make union splitting account for uncovered call
#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 6ab660d commit 07f16d5

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
@@ -1335,14 +1335,13 @@ function compute_inlining_cases(@nospecialize(info::CallInfo), flag::UInt8, sig:
13351335
nunion === nothing && return nothing
13361336
cases = InliningCase[]
13371337
argtypes = sig.argtypes
1338-
local any_fully_covered = false
13391338
local handled_all_cases::Bool = true
13401339
local revisit_idx = nothing
13411340
local only_method = nothing
13421341
local meth::MethodLookupResult
13431342
local all_result_count = 0
13441343
local joint_effects::Effects = EFFECTS_TOTAL
1345-
local nothrow::Bool = true
1344+
local fully_covered::Bool = true
13461345
for i = 1:nunion
13471346
meth = getsplit(info, i)
13481347
if meth.ambig
@@ -1364,12 +1363,12 @@ function compute_inlining_cases(@nospecialize(info::CallInfo), flag::UInt8, sig:
13641363
only_method = false
13651364
end
13661365
end
1366+
local split_fully_covered::Bool = false
13671367
for (j, match) in enumerate(meth)
13681368
all_result_count += 1
13691369
result = getresult(info, all_result_count)
13701370
joint_effects = merge_effects(joint_effects, info_effects(result, match, state))
1371-
nothrow &= match.fully_covers
1372-
any_fully_covered |= match.fully_covers
1371+
split_fully_covered |= match.fully_covers
13731372
if !validate_sparams(match.sparams)
13741373
if !match.fully_covers
13751374
handled_all_cases = false
@@ -1386,9 +1385,10 @@ function compute_inlining_cases(@nospecialize(info::CallInfo), flag::UInt8, sig:
13861385
result, match, argtypes, info, flag, state; allow_abstract=true, allow_typevars=false)
13871386
end
13881387
end
1388+
fully_covered &= split_fully_covered
13891389
end
13901390

1391-
joint_effects = Effects(joint_effects; nothrow)
1391+
joint_effects = Effects(joint_effects; nothrow=fully_covered)
13921392

13931393
if handled_all_cases && revisit_idx !== nothing
13941394
# we handled everything except one match with unmatched sparams,
@@ -1415,13 +1415,13 @@ function compute_inlining_cases(@nospecialize(info::CallInfo), flag::UInt8, sig:
14151415
end
14161416
handle_any_const_result!(cases,
14171417
result, match, argtypes, info, flag, state; allow_abstract=true, allow_typevars=true)
1418-
any_fully_covered = handled_all_cases = match.fully_covers
1418+
fully_covered = handled_all_cases = match.fully_covers
14191419
elseif !handled_all_cases
14201420
# if we've not seen all candidates, union split is valid only for dispatch tuples
14211421
filter!(case::InliningCase->isdispatchtuple(case.sig), cases)
14221422
end
14231423

1424-
return cases, (handled_all_cases & any_fully_covered), joint_effects
1424+
return cases, (handled_all_cases & fully_covered), joint_effects
14251425
end
14261426

14271427
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
@@ -1910,3 +1910,22 @@ function elim_full_ir(y)
19101910
end
19111911

19121912
@test fully_eliminated(elim_full_ir, Tuple{Int})
1913+
1914+
# union splitting should account for uncovered call signature
1915+
# https://github.com/JuliaLang/julia/issues/48397
1916+
f48397(::Bool) = :ok
1917+
f48397(::Tuple{String,String}) = :ok
1918+
let src = code_typed1((Union{Bool,Tuple{String,Any}},)) do x
1919+
f48397(x)
1920+
end
1921+
@test any(iscall((src, f48397)), src.code)
1922+
end
1923+
g48397::Union{Bool,Tuple{String,Any}} = ("48397", 48397)
1924+
@test_throws MethodError let
1925+
Base.Experimental.@force_compile
1926+
f48397(g48397)
1927+
end
1928+
@test_throws MethodError let
1929+
Base.Experimental.@force_compile
1930+
convert(Union{Bool,Tuple{String,String}}, g48397)
1931+
end

0 commit comments

Comments
 (0)