Skip to content

Commit

Permalink
inlining: Use concrete-eval effects if available
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@Keno
Keno committed Oct 24, 2022
1 parent 1525754 commit 19567df
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 26 deletions.
79 changes: 53 additions & 26 deletions base/compiler/ssair/inlining.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,8 @@ 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}})
boundscheck::Symbol, todo_bbs::Vector{Tuple{Int, Int}},
extra_flags::UInt8)
# Ok, do the inlining here
sparam_vals = item.mi.sparam_vals
def = item.mi.def::Method
Expand Down Expand Up @@ -411,6 +412,7 @@ 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 @@ -445,6 +447,14 @@ 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 @@ -589,7 +599,7 @@ function ir_inline_unionsplit!(compact::IncrementalCompact, idx::Int,
end
end
if isa(case, InliningTodo)
val = ir_inline_item!(compact, idx, argexprs′, linetable, case, boundscheck, todo_bbs)
val = ir_inline_item!(compact, idx, argexprs′, linetable, case, boundscheck, todo_bbs, flags_for_effects(case.effects))
elseif isa(case, InvokeCase)
inst = Expr(:invoke, case.invoke, argexprs′...)
flag = flags_for_effects(case.effects)
Expand Down Expand Up @@ -689,7 +699,7 @@ function batch_inline!(ir::IRCode, todo::Vector{Pair{Int,Any}}, propagate_inboun
end
end
if isa(item, InliningTodo)
compact.ssa_rename[old_idx] = ir_inline_item!(compact, idx, argexprs, ir.linetable, item, boundscheck, state.todo_bbs)
compact.ssa_rename[old_idx] = ir_inline_item!(compact, idx, argexprs, ir.linetable, item, boundscheck, state.todo_bbs, flags_for_effects(item.effects))
elseif isa(item, UnionSplit)
compact.ssa_rename[old_idx] = ir_inline_unionsplit!(compact, idx, argexprs, ir.linetable, item, boundscheck, state.todo_bbs, params)
end
Expand Down Expand Up @@ -838,8 +848,9 @@ end

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

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

if override_effects !== nothing
effects = override_effects
end

# the duplicated check might have been done already within `analyze_method!`, but still
# we need it here too since we may come here directly using a constant-prop' result
if !state.params.inlining || is_stmt_noinline(flag)
Expand Down Expand Up @@ -937,7 +952,7 @@ can_inline_typevars(m::MethodMatch, argtypes::Vector{Any}) = can_inline_typevars

function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
@nospecialize(info::CallInfo), flag::UInt8, state::InliningState;
allow_typevars::Bool, invokesig::Union{Nothing,Vector{Any}}=nothing)
allow_typevars::Bool, invokesig::Union{Nothing,Vector{Any}}=nothing, override_effects::Union{Nothing,Effects}=nothing)
method = match.method
spec_types = match.spec_types

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

return resolve_todo(mi, match, argtypes, info, flag, state; invokesig)
return resolve_todo(mi, match, argtypes, info, flag, state; invokesig, override_effects)
end

function retrieve_ir_for_inlining(mi::MethodInstance, src::Array{UInt8, 1})
Expand Down Expand Up @@ -1170,21 +1185,26 @@ function handle_invoke_call!(todo::Vector{Pair{Int,Any}},
end
result = info.result
invokesig = sig.argtypes
if isa(result, ConcreteResult) && may_inline_concrete_result(result)
item = concrete_result_item(result, state; 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)
handle_single_case!(todo, ir, idx, stmt, item, state.params, true)
return nothing
end
override_effects = nothing
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
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars=false, invokesig)
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
end
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 @@ -1296,10 +1316,12 @@ function handle_any_const_result!(cases::Vector{InliningCase},
@nospecialize(result), match::MethodMatch, argtypes::Vector{Any},
@nospecialize(info::CallInfo), flag::UInt8, state::InliningState;
allow_abstract::Bool, allow_typevars::Bool)
override_effects = nothing
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
end
Expand All @@ -1313,7 +1335,7 @@ function handle_any_const_result!(cases::Vector{InliningCase},
return handle_const_prop_result!(cases, result, argtypes, info, flag, state; allow_abstract, allow_typevars)
else
@assert result === nothing
return handle_match!(cases, match, argtypes, info, flag, state; allow_abstract, allow_typevars)
return handle_match!(cases, match, argtypes, info, flag, state; allow_abstract, allow_typevars, override_effects)
end
end

Expand Down Expand Up @@ -1444,14 +1466,14 @@ end
function handle_match!(cases::Vector{InliningCase},
match::MethodMatch, argtypes::Vector{Any}, @nospecialize(info::CallInfo), flag::UInt8,
state::InliningState;
allow_abstract::Bool, allow_typevars::Bool)
allow_abstract::Bool, allow_typevars::Bool, override_effects::Union{Effects, Nothing})
spec_types = match.spec_types
allow_abstract || isdispatchtuple(spec_types) || return false
# We may see duplicated dispatch signatures here when a signature gets widened
# during abstract interpretation: for the purpose of inlining, we can just skip
# processing this dispatch candidate (unless unmatched type parameters are present)
!allow_typevars && _any(case->case.sig === spec_types, cases) && return true
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars)
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars, override_effects)
item === nothing && return false
push!(cases, InliningCase(spec_types, item))
return true
Expand Down Expand Up @@ -1526,8 +1548,13 @@ function handle_opaque_closure_call!(todo::Vector{Pair{Int,Any}},
mi = result.result.linfo
validate_sparams(mi.sparam_vals) || return nothing
item = resolve_todo(mi, result.result, sig.argtypes, info, flag, state)
elseif isa(result, ConcreteResult) && may_inline_concrete_result(result)
item = concrete_result_item(result, 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
else
item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false)
end
Expand Down
25 changes: 25 additions & 0 deletions test/compiler/inline.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1811,3 +1811,28 @@ let src = code_typed1((NewInstruction,Any,Any,CallInfo)) do newinst, stmt, type,
@test count(iscall((src,NamedTuple)), src.code) == 0
@test count(isnew, src.code) == 1
end

# Test that inlining can still use nothrow information from concrete-eval
# even if the result itself is too big to be inlined, and nothrow is not
# known without concrete-eval
const THE_BIG_TUPLE = ntuple(identity, 1024)
function return_the_big_tuple(err::Bool)
err && error("BAD")
return THE_BIG_TUPLE
end
@noinline function return_the_big_tuple_noinline(err::Bool)
err && error("BAD")
return THE_BIG_TUPLE
end
big_tuple_test1() = return_the_big_tuple(false)[1]
big_tuple_test2() = return_the_big_tuple_noinline(false)[1]

@test fully_eliminated(big_tuple_test2, Tuple{})
# Currently we don't run these cleanup passes, but let's make sure that
# if we did, inlining would be able to remove this
let ir = Base.code_ircode(big_tuple_test1, Tuple{})[1][1]
ir = Core.Compiler.compact!(ir, true)
ir = Core.Compiler.cfg_simplify!(ir)
ir = Core.Compiler.compact!(ir, true)
@test length(ir.stmts) == 1
end

0 comments on commit 19567df

Please sign in to comment.