Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inlining: Don't inline concrete-eval'ed calls whose result was too large #47371

Merged
merged 1 commit into from
Oct 28, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 25 additions & 74 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -370,8 +370,7 @@ end

function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector{Any},
linetable::Vector{LineInfoNode}, item::InliningTodo,
boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}},
extra_flags::UInt8 = inlined_flags_for_effects(item.effects))
boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}})
# Ok, do the inlining here
sparam_vals = item.mi.sparam_vals
def = item.mi.def::Method
Expand Down Expand Up @@ -412,7 +411,6 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
break
end
inline_compact[idx′] = stmt′
inline_compact[SSAValue(idx′)][:flag] |= extra_flags
end
just_fixup!(inline_compact, new_new_offset, late_fixup_offset)
compact.result_idx = inline_compact.result_idx
Expand Down Expand Up @@ -447,14 +445,6 @@ function ir_inline_item!(compact::IncrementalCompact, idx::Int, argexprs::Vector
stmt′ = PhiNode(Int32[edge+bb_offset for edge in stmt′.edges], stmt′.values)
end
inline_compact[idx′] = stmt′
if extra_flags != 0 && !isa(stmt′, Union{GotoNode, GotoIfNot})
if (extra_flags & IR_FLAG_NOTHROW) != 0 && inline_compact[SSAValue(idx′)][:type] === Union{}
# Shown nothrow, but also guaranteed to throw => unreachable
inline_compact[idx′] = ReturnNode()
else
inline_compact[SSAValue(idx′)][:flag] |= extra_flags
end
end
end
just_fixup!(inline_compact, new_new_offset, late_fixup_offset)
compact.result_idx = inline_compact.result_idx
Expand Down Expand Up @@ -1012,37 +1002,6 @@ function flags_for_effects(effects::Effects)
return flags
end

"""
inlined_flags_for_effects(effects::Effects)

This function answers the query:

Given a call site annotated as `effects`, what can we say about each inlined
statement after the inlining?

Note that this is different from `flags_for_effects`, which just talks about
the call site itself. Consider for example:

````
function foo()
V = Any[]
push!(V, 1)
tuple(V...)
end
```

This function is properly inferred effect_free, because it has no global effects.
However, we may not inline each statement with an :effect_free flag, because
that would incorrectly lose the `push!`.
"""
function inlined_flags_for_effects(effects::Effects)
flags::UInt8 = 0
if is_nothrow(effects)
flags |= IR_FLAG_NOTHROW
end
return flags
end

function handle_single_case!(todo::Vector{Pair{Int,Any}},
ir::IRCode, idx::Int, stmt::Expr, @nospecialize(case), params::OptimizationParams,
isinvoke::Bool = false)
Expand Down Expand Up @@ -1221,24 +1180,20 @@ function handle_invoke_call!(todo::Vector{Pair{Int,Any}},
invokesig = sig.argtypes
override_effects = EFFECTS_UNKNOWN′
if isa(result, ConcreteResult)
if may_inline_concrete_result(result)
item = concrete_result_item(result, state; invokesig)
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
return nothing
end
override_effects = result.effects
end
argtypes = invoke_rewrite(sig.argtypes)
if isa(result, ConstPropResult)
mi = result.result.linfo
validate_sparams(mi.sparam_vals) || return nothing
if argtypes_to_type(argtypes) <: mi.def.sig
item = resolve_todo(mi, result.result, argtypes, info, flag, state; invokesig, override_effects)
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
return nothing
item = concrete_result_item(result, state, info; invokesig)
else
argtypes = invoke_rewrite(sig.argtypes)
if isa(result, ConstPropResult)
mi = result.result.linfo
validate_sparams(mi.sparam_vals) || return nothing
if argtypes_to_type(argtypes) <: mi.def.sig
item = resolve_todo(mi, result.result, argtypes, info, flag, state; invokesig, override_effects)
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
return nothing
end
end
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars=false, invokesig, override_effects)
end
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars=false, invokesig, override_effects)
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
return nothing
end
Expand Down Expand Up @@ -1352,12 +1307,7 @@ function handle_any_const_result!(cases::Vector{InliningCase},
allow_abstract::Bool, allow_typevars::Bool)
override_effects = EFFECTS_UNKNOWN′
if isa(result, ConcreteResult)
if may_inline_concrete_result(result)
return handle_concrete_result!(cases, result, state)
else
override_effects = result.effects
result = nothing
end
return handle_concrete_result!(cases, result, state, info)
end
if isa(result, SemiConcreteResult)
result = inlining_policy(state.interp, result, info, flag, result.mi, argtypes)
Expand Down Expand Up @@ -1538,18 +1488,24 @@ function handle_semi_concrete_result!(cases::Vector{InliningCase}, result::SemiC
return true
end

function handle_concrete_result!(cases::Vector{InliningCase}, result::ConcreteResult, state::InliningState)
case = concrete_result_item(result, state)
function handle_concrete_result!(cases::Vector{InliningCase}, result::ConcreteResult, state::InliningState, @nospecialize(info::CallInfo))
case = concrete_result_item(result, state, info)
push!(cases, InliningCase(result.mi.specTypes, case))
return true
end

may_inline_concrete_result(result::ConcreteResult) =
isdefined(result, :result) && is_inlineable_constant(result.result)

function concrete_result_item(result::ConcreteResult, state::InliningState;
function concrete_result_item(result::ConcreteResult, state::InliningState, @nospecialize(info::CallInfo);
invokesig::Union{Nothing,Vector{Any}}=nothing)
@assert may_inline_concrete_result(result)
if !may_inline_concrete_result(result)
et = InliningEdgeTracker(state.et, invokesig)
case = compileable_specialization(result.mi, result.effects, et, info;
compilesig_invokes=state.params.compilesig_invokes)
@assert case !== nothing "concrete evaluation should never happen for uncompileable callsite"
return case
end
@assert result.effects === EFFECTS_TOTAL
return ConstantCase(quoted(result.result))
end
Expand Down Expand Up @@ -1583,12 +1539,7 @@ function handle_opaque_closure_call!(todo::Vector{Pair{Int,Any}},
validate_sparams(mi.sparam_vals) || return nothing
item = resolve_todo(mi, result.result, sig.argtypes, info, flag, state)
elseif isa(result, ConcreteResult)
if may_inline_concrete_result(result)
item = concrete_result_item(result, state)
else
override_effects = result.effects
item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false, override_effects)
end
item = concrete_result_item(result, state, info)
else
item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false)
end
Expand Down
20 changes: 0 additions & 20 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1039,26 +1039,6 @@ struct FooTheRef
x::Ref
FooTheRef(v) = new(v === nothing ? THE_REF_NULL : THE_REF)
end
let src = code_typed1() do
FooTheRef(nothing)
end
@test count(isnew, src.code) == 1
end
let src = code_typed1() do
FooTheRef(0)
end
@test count(isnew, src.code) == 1
end
let src = code_typed1() do
@invoke FooTheRef(nothing::Any)
end
@test count(isnew, src.code) == 1
end
let src = code_typed1() do
@invoke FooTheRef(0::Any)
end
@test count(isnew, src.code) == 1
end
@test fully_eliminated() do
FooTheRef(nothing)
nothing
Expand Down