From 29d78fae2121eda08d03ebf1abeb4626859ef62b Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com> Date: Fri, 10 Nov 2023 00:04:31 +0900 Subject: [PATCH] inlining: stop passing `SemiConcreteResult` to `inlining_policy` (#52064) 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. --- base/compiler/abstractinterpretation.jl | 2 +- base/compiler/optimize.jl | 9 +-- base/compiler/ssair/inlining.jl | 97 ++++++++++++------------- base/compiler/ssair/passes.jl | 2 +- test/compiler/AbstractInterpreter.jl | 6 +- 5 files changed, 50 insertions(+), 66 deletions(-) diff --git a/base/compiler/abstractinterpretation.jl b/base/compiler/abstractinterpretation.jl index 21ff1c951bf85..a092b0312eaf1 100644 --- a/base/compiler/abstractinterpretation.jl +++ b/base/compiler/abstractinterpretation.jl @@ -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 diff --git a/base/compiler/optimize.jl b/base/compiler/optimize.jl index cbeb447e0c9aa..62a20896334b3 100644 --- a/base/compiler/optimize.jl +++ b/base/compiler/optimize.jl @@ -74,8 +74,7 @@ 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) @@ -83,12 +82,6 @@ function inlining_policy(interp::AbstractInterpreter, 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 diff --git a/base/compiler/ssair/inlining.jl b/base/compiler/ssair/inlining.jl index 831133f84ca74..7f6514b16d6e7 100644 --- a/base/compiler/ssair/inlining.jl +++ b/base/compiler/ssair/inlining.jl @@ -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) @@ -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) @@ -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 @@ -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 @@ -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) @@ -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 @@ -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) @@ -1507,8 +1497,7 @@ 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 @@ -1516,7 +1505,7 @@ function handle_const_prop_result!(cases::Vector{InliningCase}, 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 @@ -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, @@ -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 @@ -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 diff --git a/base/compiler/ssair/passes.jl b/base/compiler/ssair/passes.jl index 1bdd1c642a17a..ce3d5f96337a6 100644 --- a/base/compiler/ssair/passes.jl +++ b/base/compiler/ssair/passes.jl @@ -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) diff --git a/test/compiler/AbstractInterpreter.jl b/test/compiler/AbstractInterpreter.jl index 083071b566da6..2980e456e9f9c 100644 --- a/test/compiler/AbstractInterpreter.jl +++ b/test/compiler/AbstractInterpreter.jl @@ -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)