Skip to content

Commit d696360

Browse files
committed
inlining: Use concrete-eval effects if available
It is possible for concrete-eval to refine effects, but be unable to inline (because the constant is too large, c.f. #47283). However, in that case, we would still like to use the extra effect information to make sure that the optimizer can delete the statement if it turns out to be unused. There are two cases: the first is where the call is not inlineable at all. This one is simple, because we just apply the effects on the :invoke as we usually would. The second is trickier: If we do end up inlining the call, we need to apply the overriden effects to every inlined statement, because we lose the identity of the function as a whole. This is a bit nasty and I don't really like it, but I'm not sure what a better alternative would be. We could always refuse to inline calls with large-constant results (since we currently pessimize what is being inlined anyway), but I'm not sure that would be better. This is a simple solution and works for the case I have in practice, but we may want to revisit it in the future.
1 parent 0c382c2 commit d696360

File tree

2 files changed

+78
-26
lines changed

2 files changed

+78
-26
lines changed

base/compiler/ssair/inlining.jl

Lines changed: 53 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,8 @@ end
370370

371371
function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector{Any},
372372
linetable::Vector{LineInfoNode}, item::InliningTodo,
373-
boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}})
373+
boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}},
374+
extra_flags::UInt8)
374375
# Ok, do the inlining here
375376
sparam_vals = item.mi.sparam_vals
376377
def = item.mi.def::Method
@@ -411,6 +412,7 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
411412
break
412413
end
413414
inline_compact[idx′] = stmt′
415+
inline_compact[SSAValue(idx′)][:flag] |= extra_flags
414416
end
415417
just_fixup!(inline_compact, new_new_offset, late_fixup_offset)
416418
compact.result_idx = inline_compact.result_idx
@@ -445,6 +447,14 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
445447
stmt′ = PhiNode(Int32[edge+bb_offset for edge in stmt′.edges], stmt′.values)
446448
end
447449
inline_compact[idx′] = stmt′
450+
if extra_flags != 0 && !isa(stmt′, Union{GotoNode, GotoIfNot})
451+
if (extra_flags & IR_FLAG_NOTHROW) != 0 && inline_compact[SSAValue(idx′)][:type] === Union{}
452+
# Shown nothrow, but also guaranteed to throw => unreachable
453+
inline_compact[idx′] = ReturnNode()
454+
else
455+
inline_compact[SSAValue(idx′)][:flag] |= extra_flags
456+
end
457+
end
448458
end
449459
just_fixup!(inline_compact, new_new_offset, late_fixup_offset)
450460
compact.result_idx = inline_compact.result_idx
@@ -589,7 +599,7 @@ function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
589599
end
590600
end
591601
if isa(case, InliningTodo)
592-
val = ir_inline_item!(compact, idx, argexprs′, linetable, case, boundscheck, todo_bbs)
602+
val = ir_inline_item!(compact, idx, argexprs′, linetable, case, boundscheck, todo_bbs, flags_for_effects(case.effects))
593603
elseif isa(case, InvokeCase)
594604
inst = Expr(:invoke, case.invoke, argexprs′...)
595605
flag = flags_for_effects(case.effects)
@@ -689,7 +699,7 @@ function batch_inline!(ir::IRCode, todo::Vector{Pair{Int,Any}}, propagate_inboun
689699
end
690700
end
691701
if isa(item, InliningTodo)
692-
compact.ssa_rename[old_idx] = ir_inline_item!(compact, idx, argexprs, ir.linetable, item, boundscheck, state.todo_bbs)
702+
compact.ssa_rename[old_idx] = ir_inline_item!(compact, idx, argexprs, ir.linetable, item, boundscheck, state.todo_bbs, flags_for_effects(item.effects))
693703
elseif isa(item, UnionSplit)
694704
compact.ssa_rename[old_idx] = ir_inline_unionsplit!(compact, idx, argexprs, ir.linetable, item, boundscheck, state.todo_bbs, params)
695705
end
@@ -838,8 +848,9 @@ end
838848

839849
# the general resolver for usual and const-prop'ed calls
840850
function resolve_todo(mi::MethodInstance, result::Union{MethodMatch,InferenceResult},
841-
argtypes::Vector{Any}, @nospecialize(info::CallInfo), flag::UInt8,
842-
state::InliningState; invokesig::Union{Nothing,Vector{Any}}=nothing)
851+
argtypes::Vector{Any}, @nospecialize(info::CallInfo), flag::UInt8,
852+
state::InliningState; invokesig::Union{Nothing,Vector{Any}}=nothing,
853+
override_effects::Union{Nothing,Effects}=nothing)
843854
et = InliningEdgeTracker(state.et, invokesig)
844855

845856
#XXX: update_valid_age!(min_valid[1], max_valid[1], sv)
@@ -860,6 +871,10 @@ function resolve_todo(mi::MethodInstance, result::Union{MethodMatch,InferenceRes
860871
(; src, effects) = cached_result
861872
end
862873

874+
if override_effects !== nothing
875+
effects = override_effects
876+
end
877+
863878
# the duplicated check might have been done already within `analyze_method!`, but still
864879
# we need it here too since we may come here directly using a constant-prop' result
865880
if !state.params.inlining || is_stmt_noinline(flag)
@@ -937,7 +952,7 @@ can_inline_typevars(m::MethodMatch, argtypes::Vector{Any}) = can_inline_typevars
937952

938953
function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
939954
@nospecialize(info::CallInfo), flag::UInt8, state::InliningState;
940-
allow_typevars::Bool, invokesig::Union{Nothing,Vector{Any}}=nothing)
955+
allow_typevars::Bool, invokesig::Union{Nothing,Vector{Any}}=nothing, override_effects::Union{Nothing,Effects}=nothing)
941956
method = match.method
942957
spec_types = match.spec_types
943958

@@ -967,11 +982,11 @@ function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
967982
mi = specialize_method(match; preexisting=true) # Union{Nothing, MethodInstance}
968983
if mi === nothing
969984
et = InliningEdgeTracker(state.et, invokesig)
970-
return compileable_specialization(match, Effects(), et, info;
985+
return compileable_specialization(match, override_effects === nothing ? Effects() : override_effects, et, info;
971986
compilesig_invokes=state.params.compilesig_invokes)
972987
end
973988

974-
return resolve_todo(mi, match, argtypes, info, flag, state; invokesig)
989+
return resolve_todo(mi, match, argtypes, info, flag, state; invokesig, override_effects)
975990
end
976991

977992
function retrieve_ir_for_inlining(mi::MethodInstance, src::Array{UInt8, 1})
@@ -1170,21 +1185,26 @@ function handle_invoke_call!(todo::Vector{Pair{Int,Any}},
11701185
end
11711186
result = info.result
11721187
invokesig = sig.argtypes
1173-
if isa(result, ConcreteResult) && may_inline_concrete_result(result)
1174-
item = concrete_result_item(result, state; invokesig)
1175-
else
1176-
argtypes = invoke_rewrite(sig.argtypes)
1177-
if isa(result, ConstPropResult)
1178-
mi = result.result.linfo
1179-
validate_sparams(mi.sparam_vals) || return nothing
1180-
if argtypes_to_type(argtypes) <: mi.def.sig
1181-
item = resolve_todo(mi, result.result, argtypes, info, flag, state; invokesig)
1182-
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
1183-
return nothing
1184-
end
1188+
override_effects = nothing
1189+
if isa(result, ConcreteResult)
1190+
if may_inline_concrete_result(result)
1191+
item = concrete_result_item(result, state; invokesig)
1192+
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
1193+
return nothing
11851194
end
1186-
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars=false, invokesig)
1195+
override_effects = result.effects
11871196
end
1197+
argtypes = invoke_rewrite(sig.argtypes)
1198+
if isa(result, ConstPropResult)
1199+
mi = result.result.linfo
1200+
validate_sparams(mi.sparam_vals) || return nothing
1201+
if argtypes_to_type(argtypes) <: mi.def.sig
1202+
item = resolve_todo(mi, result.result, argtypes, info, flag, state; invokesig, override_effects)
1203+
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
1204+
return nothing
1205+
end
1206+
end
1207+
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars=false, invokesig, override_effects)
11881208
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
11891209
return nothing
11901210
end
@@ -1296,10 +1316,12 @@ function handle_any_const_result!(cases::Vector{InliningCase},
12961316
@nospecialize(result), match::MethodMatch, argtypes::Vector{Any},
12971317
@nospecialize(info::CallInfo), flag::UInt8, state::InliningState;
12981318
allow_abstract::Bool, allow_typevars::Bool)
1319+
override_effects = nothing
12991320
if isa(result, ConcreteResult)
13001321
if may_inline_concrete_result(result)
13011322
return handle_concrete_result!(cases, result, state)
13021323
else
1324+
override_effects = result.effects
13031325
result = nothing
13041326
end
13051327
end
@@ -1313,7 +1335,7 @@ function handle_any_const_result!(cases::Vector{InliningCase},
13131335
return handle_const_prop_result!(cases, result, argtypes, info, flag, state; allow_abstract, allow_typevars)
13141336
else
13151337
@assert result === nothing
1316-
return handle_match!(cases, match, argtypes, info, flag, state; allow_abstract, allow_typevars)
1338+
return handle_match!(cases, match, argtypes, info, flag, state; allow_abstract, allow_typevars, override_effects)
13171339
end
13181340
end
13191341

@@ -1444,14 +1466,14 @@ end
14441466
function handle_match!(cases::Vector{InliningCase},
14451467
match::MethodMatch, argtypes::Vector{Any}, @nospecialize(info::CallInfo), flag::UInt8,
14461468
state::InliningState;
1447-
allow_abstract::Bool, allow_typevars::Bool)
1469+
allow_abstract::Bool, allow_typevars::Bool, override_effects::Union{Effects, Nothing})
14481470
spec_types = match.spec_types
14491471
allow_abstract || isdispatchtuple(spec_types) || return false
14501472
# We may see duplicated dispatch signatures here when a signature gets widened
14511473
# during abstract interpretation: for the purpose of inlining, we can just skip
14521474
# processing this dispatch candidate (unless unmatched type parameters are present)
14531475
!allow_typevars && _any(case->case.sig === spec_types, cases) && return true
1454-
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars)
1476+
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars, override_effects)
14551477
item === nothing && return false
14561478
push!(cases, InliningCase(spec_types, item))
14571479
return true
@@ -1526,8 +1548,13 @@ function handle_opaque_closure_call!(todo::Vector{Pair{Int,Any}},
15261548
mi = result.result.linfo
15271549
validate_sparams(mi.sparam_vals) || return nothing
15281550
item = resolve_todo(mi, result.result, sig.argtypes, info, flag, state)
1529-
elseif isa(result, ConcreteResult) && may_inline_concrete_result(result)
1530-
item = concrete_result_item(result, state)
1551+
elseif isa(result, ConcreteResult)
1552+
if may_inline_concrete_result(result)
1553+
item = concrete_result_item(result, state)
1554+
else
1555+
override_effects = result.effects
1556+
item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false, override_effects)
1557+
end
15311558
else
15321559
item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false)
15331560
end

test/compiler/inline.jl

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1811,3 +1811,28 @@ let src = code_typed1((NewInstruction,Any,Any,CallInfo)) do newinst, stmt, type,
18111811
@test count(iscall((src,NamedTuple)), src.code) == 0
18121812
@test count(isnew, src.code) == 1
18131813
end
1814+
1815+
# Test that inlining can still use nothrow information from concrete-eval
1816+
# even if the result itself is too big to be inlined, and nothrow is not
1817+
# known without concrete-eval
1818+
const THE_BIG_TUPLE = ntuple(identity, 1024)
1819+
function return_the_big_tuple(err::Bool)
1820+
err && error("BAD")
1821+
return THE_BIG_TUPLE
1822+
end
1823+
@noinline function return_the_big_tuple_noinline(err::Bool)
1824+
err && error("BAD")
1825+
return THE_BIG_TUPLE
1826+
end
1827+
big_tuple_test1() = return_the_big_tuple(false)[1]
1828+
big_tuple_test2() = return_the_big_tuple_noinline(false)[1]
1829+
1830+
@test fully_eliminated(big_tuple_test2, Tuple{})
1831+
# Currently we don't run these cleanup passes, but let's make sure that
1832+
# if we did, inlining would be able to remove this
1833+
let ir = Base.code_ircode(big_tuple_test1, Tuple{})[1][1]
1834+
ir = Core.Compiler.compact!(ir, true)
1835+
ir = Core.Compiler.cfg_simplify!(ir)
1836+
ir = Core.Compiler.compact!(ir, true)
1837+
@test length(ir.stmts) == 1
1838+
end

0 commit comments

Comments
 (0)