Skip to content

Commit

Permalink
inlining: stop passing SemiConcreteResult to inlining_policy (#52064
Browse files Browse the repository at this point in the history 
)

It feels a bit inconsistent that the `src` argument of `inlining_policy`
needs to handle `SemiConcreteResult` while it doesn't need to handle the
other container objects that propagates sources like `CodeInstance`
`InferenceResult`, or `VolatileInferenceResult`.

This commit makes `inlining_policy` take `result.ir::IRCode` instead
when dealing with `result::SemiConcreteResult` for more consistency and
clarity.
  • Loading branch information
@aviatesk
aviatesk authored Nov 9, 2023
1 parent 529e4e7 commit 29d78fa
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 66 deletions.
2 changes: 1 addition & 1 deletion base/compiler/abstractinterpretation.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1137,7 +1137,7 @@ function const_prop_methodinstance_heuristic(interp::AbstractInterpreter,
if isa(code, CodeInstance)
inferred = @atomic :monotonic code.inferred
# TODO propagate a specific `CallInfo` that conveys information about this call
if inlining_policy(interp, inferred, NoCallInfo(), IR_FLAG_NULL, mi, arginfo.argtypes) !== nothing
if inlining_policy(interp, inferred, NoCallInfo(), IR_FLAG_NULL) !== nothing
return true
end
end
Expand Down
9 changes: 1 addition & 8 deletions base/compiler/optimize.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,21 +74,14 @@ is_source_inferred(@nospecialize src::MaybeCompressed) =
ccall(:jl_ir_flag_inferred, Bool, (Any,), src)

function inlining_policy(interp::AbstractInterpreter,
@nospecialize(src), @nospecialize(info::CallInfo), stmt_flag::UInt32, mi::MethodInstance,
_::Vector{Any})
@nospecialize(src), @nospecialize(info::CallInfo), stmt_flag::UInt32)
if isa(src, MaybeCompressed)
is_source_inferred(src) || return nothing
src_inlineable = is_stmt_inline(stmt_flag) || is_inlineable(src)
return src_inlineable ? src : nothing
elseif isa(src, IRCode)
return src
elseif isa(src, SemiConcreteResult)
if is_declared_noinline(mi.def::Method)
# For `NativeInterpreter`, `SemiConcreteResult` may be produced for
# a `@noinline`-declared method when it's marked as `@constprop :aggressive`.
# Suppress the inlining here.
return nothing
end
return src
end
return nothing
Expand Down
97 changes: 45 additions & 52 deletions base/compiler/ssair/inlining.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -874,7 +874,7 @@ end

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

Expand All @@ -901,7 +901,7 @@ function resolve_todo(mi::MethodInstance, result::Union{Nothing,InferenceResult,
compilesig_invokes=OptimizationParams(state.interp).compilesig_invokes)
end

src = inlining_policy(state.interp, src, info, flag, mi, argtypes)
src = inlining_policy(state.interp, src, info, flag)
src === nothing && return compileable_specialization(mi, effects, et, info;
compilesig_invokes=OptimizationParams(state.interp).compilesig_invokes)

Expand All @@ -911,8 +911,8 @@ function resolve_todo(mi::MethodInstance, result::Union{Nothing,InferenceResult,
end

# the special resolver for :invoke-d call
function resolve_todo(mi::MethodInstance, argtypes::Vector{Any},
@nospecialize(info::CallInfo), flag::UInt32, state::InliningState)
function resolve_todo(mi::MethodInstance, @nospecialize(info::CallInfo), flag::UInt32,
state::InliningState)
if !OptimizationParams(state.interp).inlining || is_stmt_noinline(flag)
return nothing
end
Expand All @@ -926,7 +926,7 @@ function resolve_todo(mi::MethodInstance, argtypes::Vector{Any},
end
(; src, effects) = cached_result

src = inlining_policy(state.interp, src, info, flag, mi, argtypes)
src = inlining_policy(state.interp, src, info, flag)

src === nothing && return nothing

Expand Down Expand Up @@ -981,7 +981,7 @@ function analyze_method!(match::MethodMatch, argtypes::Vector{Any},
# Get the specialization for this method signature
# (later we will decide what to do with it)
mi = specialize_method(match)
return resolve_todo(mi, volatile_inf_result, argtypes, info, flag, state; invokesig)
return resolve_todo(mi, volatile_inf_result, info, flag, state; invokesig)
end

function retrieve_ir_for_inlining(mi::MethodInstance, src::String, ::Bool=true)
Expand Down Expand Up @@ -1204,26 +1204,21 @@ function handle_invoke_call!(todo::Vector{Pair{Int,Any}},
invokesig = sig.argtypes
if isa(result, ConcreteResult)
item = concrete_result_item(result, info, state; invokesig)
elseif isa(result, SemiConcreteResult)
item = semiconcrete_result_item(result, info, flag, state)
else
argtypes = invoke_rewrite(sig.argtypes)
if isa(result, SemiConcreteResult)
result = inlining_policy(state.interp, result, info, flag, result.mi, argtypes)
end
if isa(result, SemiConcreteResult)
item = semiconcrete_result_item(result, info, flag, state)
else
if isa(result, ConstPropResult)
mi = result.result.linfo
validate_sparams(mi.sparam_vals) || return nothing
if Union{} !== 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, true)
return nothing
end
if isa(result, ConstPropResult)
mi = result.result.linfo
validate_sparams(mi.sparam_vals) || return nothing
if Union{} !== argtypes_to_type(argtypes) <: mi.def.sig
item = resolve_todo(mi, result.result, info, flag, state; invokesig)
handle_single_case!(todo, ir, idx, stmt, item, true)
return nothing
end
volatile_inf_result = result isa VolatileInferenceResult ? result : nothing
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars=false, invokesig, volatile_inf_result)
end
volatile_inf_result = result isa VolatileInferenceResult ? result : nothing
item = analyze_method!(match, argtypes, info, flag, state; allow_typevars=false, invokesig, volatile_inf_result)
end
handle_single_case!(todo, ir, idx, stmt, item, true)
return nothing
Expand Down Expand Up @@ -1352,15 +1347,10 @@ function handle_any_const_result!(cases::Vector{InliningCase},
allow_abstract::Bool, allow_typevars::Bool)
if isa(result, ConcreteResult)
return handle_concrete_result!(cases, result, info, state)
end
if isa(result, SemiConcreteResult)
result = inlining_policy(state.interp, result, info, flag, result.mi, argtypes)
if isa(result, SemiConcreteResult)
return handle_semi_concrete_result!(cases, result, info, flag, state; allow_abstract)
end
end
if isa(result, ConstPropResult)
return handle_const_prop_result!(cases, result, argtypes, info, flag, state; allow_abstract, allow_typevars)
elseif isa(result, SemiConcreteResult)
return handle_semi_concrete_result!(cases, result, info, flag, state; allow_abstract)
elseif isa(result, ConstPropResult)
return handle_const_prop_result!(cases, result, info, flag, state; allow_abstract, allow_typevars)
else
@assert result === nothing || result isa VolatileInferenceResult
return handle_match!(cases, match, argtypes, info, flag, state; allow_abstract, allow_typevars, volatile_inf_result = result)
Expand Down Expand Up @@ -1507,16 +1497,15 @@ function handle_match!(cases::Vector{InliningCase},
end

function handle_const_prop_result!(cases::Vector{InliningCase},
result::ConstPropResult, argtypes::Vector{Any}, @nospecialize(info::CallInfo),
flag::UInt32, state::InliningState;
result::ConstPropResult, @nospecialize(info::CallInfo), flag::UInt32, state::InliningState;
allow_abstract::Bool, allow_typevars::Bool)
mi = result.result.linfo
spec_types = mi.specTypes
allow_abstract || isdispatchtuple(spec_types) || return false
if !validate_sparams(mi.sparam_vals)
(allow_typevars && !may_have_fcalls(mi.def::Method)) || return false
end
item = resolve_todo(mi, result.result, argtypes, info, flag, state)
item = resolve_todo(mi, result.result, info, flag, state)
item === nothing && return false
push!(cases, InliningCase(spec_types, item))
return true
Expand All @@ -1525,15 +1514,24 @@ end
function semiconcrete_result_item(result::SemiConcreteResult,
@nospecialize(info::CallInfo), flag::UInt32, state::InliningState)
mi = result.mi
if !OptimizationParams(state.interp).inlining || is_stmt_noinline(flag)
et = InliningEdgeTracker(state)
et = InliningEdgeTracker(state)

if (!OptimizationParams(state.interp).inlining || is_stmt_noinline(flag) ||
# For `NativeInterpreter`, `SemiConcreteResult` may be produced for
# a `@noinline`-declared method when it's marked as `@constprop :aggressive`.
# Suppress the inlining here (unless inlining is requested at the callsite).
(is_declared_noinline(mi.def::Method) && !is_stmt_inline(flag)))
return compileable_specialization(mi, result.effects, et, info;
compilesig_invokes=OptimizationParams(state.interp).compilesig_invokes)
else
preserve_local_sources = OptimizationParams(state.interp).preserve_local_sources
ir = retrieve_ir_for_inlining(mi, result.ir, preserve_local_sources)
return InliningTodo(mi, ir, result.effects)
end
ir = inlining_policy(state.interp, result.ir, info, flag)
ir === nothing && return compileable_specialization(mi, result.effects, et, info;
compilesig_invokes=OptimizationParams(state.interp).compilesig_invokes)

add_inlining_backedge!(et, mi)
preserve_local_sources = OptimizationParams(state.interp).preserve_local_sources
ir = retrieve_ir_for_inlining(mi, ir, preserve_local_sources)
return InliningTodo(mi, ir, result.effects)
end

function handle_semi_concrete_result!(cases::Vector{InliningCase}, result::SemiConcreteResult,
Expand Down Expand Up @@ -1597,20 +1595,15 @@ function handle_opaque_closure_call!(todo::Vector{Pair{Int,Any}},
if isa(result, ConstPropResult)
mi = result.result.linfo
validate_sparams(mi.sparam_vals) || return nothing
item = resolve_todo(mi, result.result, sig.argtypes, info, flag, state)
item = resolve_todo(mi, result.result, info, flag, state)
elseif isa(result, ConcreteResult)
item = concrete_result_item(result, info, state)
elseif isa(result, SemiConcreteResult)
item = item = semiconcrete_result_item(result, info, flag, state)
else
if isa(result, SemiConcreteResult)
result = inlining_policy(state.interp, result, info, flag, result.mi, sig.argtypes)
end
if isa(result, SemiConcreteResult)
item = semiconcrete_result_item(result, info, flag, state)
else
@assert result === nothing || result isa VolatileInferenceResult
volatile_inf_result = result
item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false, volatile_inf_result)
end
@assert result === nothing || result isa VolatileInferenceResult
volatile_inf_result = result
item = analyze_method!(info.match, sig.argtypes, info, flag, state; allow_typevars=false, volatile_inf_result)
end
handle_single_case!(todo, ir, idx, stmt, item)
return nothing
Expand Down Expand Up @@ -1678,7 +1671,7 @@ end
function handle_invoke_expr!(todo::Vector{Pair{Int,Any}}, ir::IRCode,
idx::Int, stmt::Expr, @nospecialize(info::CallInfo), flag::UInt32, sig::Signature, state::InliningState)
mi = stmt.args[1]::MethodInstance
case = resolve_todo(mi, sig.argtypes, info, flag, state)
case = resolve_todo(mi, info, flag, state)
handle_single_case!(todo, ir, idx, stmt, case, false)
return nothing
end
Expand Down
2 changes: 1 addition & 1 deletion base/compiler/ssair/passes.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1236,7 +1236,7 @@ function try_inline_finalizer!(ir::IRCode, argexprs::Vector{Any}, idx::Int,
src = nothing
end

src = inlining_policy(inlining.interp, src, info, IR_FLAG_NULL, mi, Any[])
src = inlining_policy(inlining.interp, src, info, IR_FLAG_NULL)
src === nothing && return false
src = retrieve_ir_for_inlining(mi, src)

Expand Down
6 changes: 2 additions & 4 deletions test/compiler/AbstractInterpreter.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,14 +335,12 @@ function CC.abstract_call(interp::NoinlineInterpreter,
return ret
end
function CC.inlining_policy(interp::NoinlineInterpreter,
@nospecialize(src), @nospecialize(info::CallInfo), stmt_flag::UInt32, mi::MethodInstance,
argtypes::Vector{Any})
@nospecialize(src), @nospecialize(info::CallInfo), stmt_flag::UInt32)
if isa(info, NoinlineCallInfo)
return nothing
end
return @invoke CC.inlining_policy(interp::CC.AbstractInterpreter,
src::Any, info::CallInfo, stmt_flag::UInt32, mi::MethodInstance,
argtypes::Vector{Any})
src::Any, info::CallInfo, stmt_flag::UInt32)
end

@inline function inlined_usually(x, y, z)
Expand Down

5 comments on commit 29d78fa

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The package evaluation job you requested has completed - possible new issues were detected.
The full report is available.

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

@vtjnash
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know what was in #52089, but it was pretty good for SparseArrays allocations

Please sign in to comment.