Skip to content

Commit 4ffedb3

Browse files
author
Ian Atol
committed
Partially revert 44512 to handle duplicated spec_types inlining bug
1 parent 72622d5 commit 4ffedb3

File tree

3 files changed

+52
-32
lines changed

3 files changed

+52
-32
lines changed

base/compiler/ssair/inlining.jl

Lines changed: 50 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -892,12 +892,7 @@ function validate_sparams(sparams::SimpleVector)
892892
end
893893

894894
function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
895-
<<<<<<< HEAD
896-
flag::UInt8, state::InliningState,
897-
do_resolve::Bool = true)
898-
=======
899-
flag::UInt8, state::InliningState, check_sparams::Bool = false)
900-
>>>>>>> 0d6db0ef08 (Fixups and use concrete spvals in ssa_substitute)
895+
flag::UInt8, state::InliningState, do_resolve::Bool = true, check_sparams::Bool = false)
901896
method = match.method
902897
spec_types = match.spec_types
903898

@@ -1254,7 +1249,8 @@ function compute_inlining_cases(
12541249
cases = InliningCase[]
12551250
local any_fully_covered = false
12561251
local handled_all_cases = true
1257-
local only_method = nothing # tri-valued: nothing if unknown, false if proven untrue, otherwise the method itself
1252+
local revisit_idx = nothing
1253+
local only_method = nothing
12581254
local meth::MethodLookupResult
12591255
for i in 1:length(infos)
12601256
meth = infos[i].results
@@ -1277,19 +1273,53 @@ function compute_inlining_cases(
12771273
only_method = false
12781274
end
12791275
end
1280-
1281-
check_sparams = isa(only_method, Bool)
1282-
for match in meth
1283-
<<<<<<< HEAD
1284-
handled_all_cases &= handle_match!(match, argtypes, flag, state, cases, true, do_resolve)
1285-
=======
1286-
handled_all_cases &= handle_match!(match, argtypes, flag, state, cases, true, check_sparams)
1287-
>>>>>>> 0d6db0ef08 (Fixups and use concrete spvals in ssa_substitute)
1276+
for (j, match) in enumerate(meth)
1277+
if !isdispatchtuple(match.spec_types)
1278+
if !match.fully_covers
1279+
handled_all_cases = false
1280+
continue
1281+
end
1282+
if revisit_idx === nothing
1283+
revisit_idx = (i, j)
1284+
else
1285+
handled_all_cases = false
1286+
revisit_idx = nothing
1287+
end
1288+
else
1289+
handled_all_cases &= handle_match!(match, argtypes, flag, state, cases, true, do_resolve, check_sparams)
1290+
end
12881291
any_fully_covered |= match.fully_covers
12891292
end
12901293
end
12911294

1292-
if !handled_all_cases
1295+
atype = argtypes_to_type(argtypes)
1296+
if handled_all_cases && revisit_idx !== nothing
1297+
# If there's only one case that's not a dispatchtuple, we can
1298+
# still unionsplit by visiting all the other cases first.
1299+
# This is useful for code like:
1300+
# foo(x::Int) = 1
1301+
# foo(@nospecialize(x::Any)) = 2
1302+
# where we where only a small number of specific dispatchable
1303+
# cases are split off from an ::Any typed fallback.
1304+
(i, j) = revisit_idx
1305+
match = infos[i].results[j]
1306+
handled_all_cases &= handle_match!(match, argtypes, flag, state, cases, true, true)
1307+
elseif length(cases) == 0 && only_method isa Method
1308+
# if the signature is fully covered and there is only one applicable method,
1309+
# we can try to inline it even if the signature is not a dispatch tuple.
1310+
# -- But don't try it if we already tried to handle the match in the revisit_idx
1311+
# case, because that'll (necessarily) be the same method.
1312+
if length(infos) > 1
1313+
(metharg, methsp) = ccall(:jl_type_intersection_with_env, Any, (Any, Any),
1314+
atype, only_method.sig)::SimpleVector
1315+
match = MethodMatch(metharg, methsp::SimpleVector, only_method, true)
1316+
else
1317+
@assert length(meth) == 1
1318+
match = meth[1]
1319+
end
1320+
handle_match!(match, argtypes, flag, state, cases, true, false) || return nothing
1321+
any_fully_covered = handled_all_cases = match.fully_covers
1322+
elseif !handled_all_cases
12931323
# if we've not seen all candidates, union split is valid only for dispatch tuples
12941324
filter!(case::InliningCase->isdispatchtuple(case.sig), cases)
12951325
end
@@ -1369,30 +1399,19 @@ end
13691399

13701400
function handle_match!(
13711401
match::MethodMatch, argtypes::Vector{Any}, flag::UInt8, state::InliningState,
1372-
<<<<<<< HEAD
1373-
cases::Vector{InliningCase}, allow_abstract::Bool = false,
1374-
do_resolve::Bool = true)
1375-
spec_types = match.spec_types
1376-
allow_abstract || isdispatchtuple(spec_types) || return false
1377-
# we may see duplicated dispatch signatures here when a signature gets widened
1378-
# during abstract interpretation: for the purpose of inlining, we can just skip
1379-
# processing this dispatch candidate
1380-
_any(case->case.sig === spec_types, cases) && return true
1381-
item = analyze_method!(match, argtypes, flag, state, do_resolve)
1382-
=======
1383-
cases::Vector{InliningCase}, allow_abstract::Bool = false, check_sparams::Bool = false)
1402+
cases::Vector{InliningCase}, allow_abstract::Bool = false,
1403+
do_resolve::Bool = true, check_sparams::Bool = false)
13841404
spec_types = match.spec_types
13851405
allow_abstract || isdispatchtuple(spec_types) || return false
13861406
if check_sparams
13871407
# we may see duplicated dispatch signatures here when a signature gets widened
13881408
# during abstract interpretation: for the purpose of inlining, we can just skip
13891409
# processing this dispatch candidate
13901410
_any(case->case.sig === spec_types, cases) && return true
1391-
item = analyze_method!(match, argtypes, flag, state, true)
1411+
item = analyze_method!(match, argtypes, flag, state, do_resolve, true)
13921412
else
1393-
item = analyze_method!(match, argtypes, flag, state)
1413+
item = analyze_method!(match, argtypes, flag, state, do_resolve)
13941414
end
1395-
>>>>>>> 0d6db0ef08 (Fixups and use concrete spvals in ssa_substitute)
13961415
item === nothing && return false
13971416
push!(cases, InliningCase(spec_types, item))
13981417
return true

base/compiler/ssair/ir.jl

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1505,6 +1505,8 @@ function fixup_phinode_values!(compact::IncrementalCompact, old_values::Vector{A
15051505
return (values, fixup)
15061506
end
15071507

1508+
56
1509+
15081510
function fixup_node(compact::IncrementalCompact, @nospecialize(stmt), reify_new_nodes::Bool)
15091511
if isa(stmt, PhiNode)
15101512
(node, needs_fixup) = fixup_phinode_values!(compact, stmt.values, reify_new_nodes)

src/base

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)