Skip to content

Commit 71e77fd

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 2ebe577 commit 71e77fd

File tree

2 files changed

+110
-24
lines changed

2 files changed

+110
-24
lines changed

base/compiler/ssair/inlining.jl

Lines changed: 85 additions & 24 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 = inlined_flags_for_effects(item.effects))
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
@@ -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::Effects = EFFECTS_UNKNOWN′)
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 !== EFFECTS_UNKNOWN′
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,8 @@ 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,
956+
override_effects::Effects=EFFECTS_UNKNOWN′)
941957
method = match.method
942958
spec_types = match.spec_types
943959

@@ -967,11 +983,13 @@ function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
967983
mi = specialize_method(match; preexisting=true) # Union{Nothing, MethodInstance}
968984
if mi === nothing
969985
et = InliningEdgeTracker(state.et, invokesig)
970-
return compileable_specialization(match, Effects(), et, info;
986+
effects = override_effects
987+
effects === EFFECTS_UNKNOWN′ && (effects = info_effects(nothing, match, state))
988+
return compileable_specialization(match, effects, et, info;
971989
compilesig_invokes=state.params.compilesig_invokes)
972990
end
973991

974-
return resolve_todo(mi, match, argtypes, info, flag, state; invokesig)
992+
return resolve_todo(mi, match, argtypes, info, flag, state; invokesig, override_effects)
975993
end
976994

977995
function retrieve_ir_for_inlining(mi::MethodInstance, src::Array{UInt8, 1})
@@ -994,6 +1012,37 @@ function flags_for_effects(effects::Effects)
9941012
return flags
9951013
end
9961014

1015+
"""
1016+
inlined_flags_for_effects(effects::Effects)
1017+
1018+
This function answers the query:
1019+
1020+
Given a call site annotated as `effects`, what can we say about each inlined
1021+
statement after the inlining?
1022+
1023+
Note that this is different from `flags_for_effects`, which just talks about
1024+
the call site itself. Consider for example:
1025+
1026+
````
1027+
function foo()
1028+
V = Any[]
1029+
push!(V, 1)
1030+
tuple(V...)
1031+
end
1032+
```
1033+
1034+
This function is properly inferred effect_free, because it has no global effects.
1035+
However, we may not inline each statement with an :effect_free flag, because
1036+
that would incorrectly lose the `push!`.
1037+
"""
1038+
function inlined_flags_for_effects(effects::Effects)
1039+
flags::UInt8 = 0
1040+
if is_nothrow(effects)
1041+
flags |= IR_FLAG_NOTHROW
1042+
end
1043+
return flags
1044+
end
1045+
9971046
function handle_single_case!(todo::Vector{Pair{Int,Any}},
9981047
ir::IRCode, idx::Int, stmt::Expr, @nospecialize(case), params::OptimizationParams,
9991048
isinvoke::Bool = false)
@@ -1170,21 +1219,26 @@ function handle_invoke_call!(todo::Vector{Pair{Int,Any}},
11701219
end
11711220
result = info.result
11721221
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
1222+
override_effects = EFFECTS_UNKNOWN′
1223+
if isa(result, ConcreteResult)
1224+
if may_inline_concrete_result(result)
1225+
item = concrete_result_item(result, state; invokesig)
1226+
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
1227+
return nothing
11851228
end
1186-
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars=false, invokesig)
1229+
override_effects = result.effects
11871230
end
1231+
argtypes = invoke_rewrite(sig.argtypes)
1232+
if isa(result, ConstPropResult)
1233+
mi = result.result.linfo
1234+
validate_sparams(mi.sparam_vals) || return nothing
1235+
if argtypes_to_type(argtypes) <: mi.def.sig
1236+
item = resolve_todo(mi, result.result, argtypes, info, flag, state; invokesig, override_effects)
1237+
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
1238+
return nothing
1239+
end
1240+
end
1241+
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars=false, invokesig, override_effects)
11881242
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
11891243
return nothing
11901244
end
@@ -1296,10 +1350,12 @@ function handle_any_const_result!(cases::Vector{InliningCase},
12961350
@nospecialize(result), match::MethodMatch, argtypes::Vector{Any},
12971351
@nospecialize(info::CallInfo), flag::UInt8, state::InliningState;
12981352
allow_abstract::Bool, allow_typevars::Bool)
1353+
override_effects = EFFECTS_UNKNOWN′
12991354
if isa(result, ConcreteResult)
13001355
if may_inline_concrete_result(result)
13011356
return handle_concrete_result!(cases, result, state)
13021357
else
1358+
override_effects = result.effects
13031359
result = nothing
13041360
end
13051361
end
@@ -1313,7 +1369,7 @@ function handle_any_const_result!(cases::Vector{InliningCase},
13131369
return handle_const_prop_result!(cases, result, argtypes, info, flag, state; allow_abstract, allow_typevars)
13141370
else
13151371
@assert result === nothing
1316-
return handle_match!(cases, match, argtypes, info, flag, state; allow_abstract, allow_typevars)
1372+
return handle_match!(cases, match, argtypes, info, flag, state; allow_abstract, allow_typevars, override_effects)
13171373
end
13181374
end
13191375

@@ -1444,14 +1500,14 @@ end
14441500
function handle_match!(cases::Vector{InliningCase},
14451501
match::MethodMatch, argtypes::Vector{Any}, @nospecialize(info::CallInfo), flag::UInt8,
14461502
state::InliningState;
1447-
allow_abstract::Bool, allow_typevars::Bool)
1503+
allow_abstract::Bool, allow_typevars::Bool, override_effects::Effects)
14481504
spec_types = match.spec_types
14491505
allow_abstract || isdispatchtuple(spec_types) || return false
14501506
# We may see duplicated dispatch signatures here when a signature gets widened
14511507
# during abstract interpretation: for the purpose of inlining, we can just skip
14521508
# processing this dispatch candidate (unless unmatched type parameters are present)
14531509
!allow_typevars && _any(case->case.sig === spec_types, cases) && return true
1454-
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars)
1510+
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars, override_effects)
14551511
item === nothing && return false
14561512
push!(cases, InliningCase(spec_types, item))
14571513
return true
@@ -1526,8 +1582,13 @@ function handle_opaque_closure_call!(todo::Vector{Pair{Int,Any}},
15261582
mi = result.result.linfo
15271583
validate_sparams(mi.sparam_vals) || return nothing
15281584
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)
1585+
elseif isa(result, ConcreteResult)
1586+
if may_inline_concrete_result(result)
1587+
item = concrete_result_item(result, state)
1588+
else
1589+
override_effects = result.effects
1590+
item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false, override_effects)
1591+
end
15311592
else
15321593
item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false)
15331594
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)