-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Compiler/ssair/passes: _lift_svec_ref
: improve type stability
#57633
Compiler/ssair/passes: _lift_svec_ref
: improve type stability
#57633
Conversation
Making the closure capture a `SimpleVector` instead of a type should improve type stability. Also deduplicated some common subexpressions while at it.
Decreases the invalidation count when running the following code from 512 to 490: struct I <: Integer end
Base.Int(::I) = 7 Reproduce with: ./julia -E 'using SnoopCompileCore; i=(@snoop_invalidations (struct I <: Integer end; Base.Int(::I) = 7)); using SnoopCompile; length(uinvalidated(i))' |
bump |
In case it's useful, here are some (maybe all, didn't check the diff comprehensively) of the fixed invalidations: {
"method_instance": {
"method": "_mapreducedim!(f, op, R::AbstractArray, A::Union{Base.AbstractBroadcasted, AbstractArray}) @ Base reducedim.jl:253",
"method_instance": "MethodInstance for Base._mapreducedim!(::Function, ::Function, ::AbstractArray, ::UnitRange{Int64})"
},
"children": [
{
"method_instance": {
"method": "mapreducedim!(f, op, R::AbstractArray, A::Union{Base.AbstractBroadcasted, AbstractArray}) @ Base reducedim.jl:293",
"method_instance": "MethodInstance for Base.mapreducedim!(::Function, ::Function, ::AbstractArray, ::UnitRange{Int64})"
},
"children": [
{
"method_instance": {
"method": "_mapreduce_dim(f, op, ::Base._InitialValue, A::Union{Base.AbstractBroadcasted, AbstractArray}, dims) @ Base reducedim.jl:340",
"method_instance": "MethodInstance for Base._mapreduce_dim(::Function, ::Function, ::Base._InitialValue, ::UnitRange{Int64}, ::Function)"
},
"children": [
{
"method_instance": {
"method": "var\"#mapreduce#719\"(dims, init, ::typeof(mapreduce), f, op, A::Union{Base.AbstractBroadcasted, AbstractArray}) @ Base reducedim.jl:326",
"method_instance": "MethodInstance for Base.var\"#mapreduce#719\"(::Function, ::Base._InitialValue, ::typeof(mapreduce), ::Function, ::typeof(|), ::UnitRange{Int64})"
},
"children": [
{
"method_instance": {
"method": "kwcall(::NamedTuple, ::typeof(mapreduce), f, op, A::Union{Base.AbstractBroadcasted, AbstractArray}) @ Base reducedim.jl:326",
"method_instance": "MethodInstance for Core.kwcall(::NamedTuple{(:dims,), <:Tuple{Function}}, ::typeof(mapreduce), ::Function, ::typeof(|), ::UnitRange{Int64})"
},
"children": [
{
"method_instance": {
"method": "var\"#_any#773\"(kw::Base.Pairs{Symbol, V, NTuple{N, Symbol}, NamedTuple{names, T}} where {V, N, names, T<:NTuple{N, Any}}, ::typeof(Base._any), f, A, dims) @ Base reducedim.jl:1008",
"method_instance": "MethodInstance for Base.var\"#_any#773\"(::Base.Pairs{Symbol, Union{}, Tuple{}, @NamedTuple{}}, ::typeof(Base._any), ::Function, ::UnitRange{Int64}, ::Function)"
},
"children": [
{
"method_instance": {
"method": "_any(f, A, dims; kw...) @ Base reducedim.jl:1008",
"method_instance": "MethodInstance for Base._any(::Function, ::UnitRange{Int64}, ::Function)"
},
"children": [
{
"method_instance": {
"method": "var\"#any#747\"(dims, ::typeof(any), f::Function, a::AbstractArray) @ Base reducedim.jl:989",
"method_instance": "MethodInstance for Base.var\"#any#747\"(::Function, ::typeof(any), ::Function, ::UnitRange{Int64})"
},
"children": [
{
"method_instance": {
"method": "any(f::Function, a::AbstractArray; dims) @ Base reducedim.jl:989",
"method_instance": "MethodInstance for any(::Compiler.var\"#_lift_svec_ref##2#_lift_svec_ref##3\"{TypeVar, Type{_A}} where _A, ::UnitRange{Int64})"
},
"children": [
{
"method_instance": {
"method": "_lift_svec_ref(def::Expr, compact::Compiler.IncrementalCompact) @ Compiler ../usr/share/julia/Compiler/src/ssair/passes.jl:1016",
"method_instance": "MethodInstance for Compiler._lift_svec_ref(::Expr, ::Compiler.IncrementalCompact)"
},
"children": [
{
"method_instance": {
"method": "lift_svec_ref!(compact::Compiler.IncrementalCompact, idx::Int64, stmt::Expr) @ Compiler ../usr/share/julia/Compiler/src/ssair/passes.jl:875",
"method_instance": "MethodInstance for Compiler.lift_svec_ref!(::Compiler.IncrementalCompact, ::Int64, ::Expr)"
},
"children": [
{
"method_instance": {
"method": "sroa_pass!(ir::Compiler.IRCode, inlining::Union{Nothing, Compiler.InliningState}) @ Compiler ../usr/share/julia/Compiler/src/ssair/passes.jl:1212",
"method_instance": "MethodInstance for Compiler.sroa_pass!(::Compiler.IRCode, ::Compiler.InliningState)"
},
"children": [
{
"method_instance": {
"method": "run_passes_ipo_safe(ci::Core.CodeInfo, sv::Compiler.OptimizationState, optimize_until) @ Compiler ~/tmp/jl/julia-git/Compiler_ssair_lift_svec_ref_closure_type_stability/usr/share/julia/Compiler/src/optimize.jl:1036",
"method_instance": "MethodInstance for Compiler.run_passes_ipo_safe(::Core.CodeInfo, ::Compiler.OptimizationState, ::Nothing)"
},
"children": [
{
"method_instance": {
"method": "run_passes_ipo_safe(ci::Core.CodeInfo, sv::Compiler.OptimizationState) @ Compiler ~/tmp/jl/julia-git/Compiler_ssair_lift_svec_ref_closure_type_stability/usr/share/julia/Compiler/src/optimize.jl:1036",
"method_instance": "MethodInstance for Compiler.run_passes_ipo_safe(::Core.CodeInfo, ::Compiler.OptimizationState)"
},
"children": [
{
"method_instance": {
"method": "optimize(interp::Compiler.AbstractInterpreter, opt::Compiler.OptimizationState, caller::Compiler.InferenceResult) @ Compiler ~/tmp/jl/julia-git/Compiler_ssair_lift_svec_ref_closure_type_stability/usr/share/julia/Compiler/src/optimize.jl:1015",
"method_instance": "MethodInstance for Compiler.optimize(::Compiler.AbstractInterpreter, ::Compiler.OptimizationState, ::Compiler.InferenceResult)"
},
"children": [
{
"method_instance": {
"method": "finish_cycle(::Compiler.AbstractInterpreter, frames::Vector{Union{Compiler.IRInterpretationState, Compiler.InferenceState}}, cycleid::Int64) @ Compiler ~/tmp/jl/julia-git/Compiler_ssair_lift_svec_ref_closure_type_stability/usr/share/julia/Compiler/src/typeinfer.jl:222",
"method_instance": "MethodInstance for Compiler.finish_cycle(::Compiler.AbstractInterpreter, ::Vector{Union{Compiler.IRInterpretationState, Compiler.InferenceState}}, ::Int64)"
},
"children": [
]
},
{
"method_instance": {
"method": "finish_cycle(::Compiler.AbstractInterpreter, frames::Vector{Union{Compiler.IRInterpretationState, Compiler.InferenceState}}, cycleid::Int64) @ Compiler ~/tmp/jl/julia-git/Compiler_ssair_lift_svec_ref_closure_type_stability/usr/share/julia/Compiler/src/typeinfer.jl:222",
"method_instance": "MethodInstance for Compiler.finish_cycle(::Compiler.NativeInterpreter, ::Vector{Union{Compiler.IRInterpretationState, Compiler.InferenceState}}, ::Int64)"
},
"children": [
]
},
{
"method_instance": {
"method": "finish_nocycle(::Compiler.AbstractInterpreter, frame::Compiler.InferenceState) @ Compiler ~/tmp/jl/julia-git/Compiler_ssair_lift_svec_ref_closure_type_stability/usr/share/julia/Compiler/src/typeinfer.jl:200",
"method_instance": "MethodInstance for Compiler.finish_nocycle(::Compiler.AbstractInterpreter, ::Compiler.InferenceState)"
},
"children": [
]
},
{
"method_instance": {
"method": "finish_nocycle(::Compiler.AbstractInterpreter, frame::Compiler.InferenceState) @ Compiler ~/tmp/jl/julia-git/Compiler_ssair_lift_svec_ref_closure_type_stability/usr/share/julia/Compiler/src/typeinfer.jl:200",
"method_instance": "MethodInstance for Compiler.finish_nocycle(::Compiler.NativeInterpreter, ::Compiler.InferenceState)"
},
"children": [
]
}
]
}
]
}
]
}
]
},
{
"method_instance": {
"method": "sroa_pass!(ir::Compiler.IRCode, inlining::Union{Nothing, Compiler.InliningState}) @ Compiler ../usr/share/julia/Compiler/src/ssair/passes.jl:1212",
"method_instance": "MethodInstance for Compiler.sroa_pass!(::Compiler.IRCode, ::Compiler.InliningState{Compiler.NativeInterpreter})"
},
"children": [
{
"method_instance": {
"method": "run_passes_ipo_safe(ci::Core.CodeInfo, sv::Compiler.OptimizationState, optimize_until) @ Compiler ~/tmp/jl/julia-git/Compiler_ssair_lift_svec_ref_closure_type_stability/usr/share/julia/Compiler/src/optimize.jl:1036",
"method_instance": "MethodInstance for Compiler.run_passes_ipo_safe(::Core.CodeInfo, ::Compiler.OptimizationState{Compiler.NativeInterpreter}, ::Nothing)"
},
"children": [
{
"method_instance": {
"method": "run_passes_ipo_safe(ci::Core.CodeInfo, sv::Compiler.OptimizationState) @ Compiler ~/tmp/jl/julia-git/Compiler_ssair_lift_svec_ref_closure_type_stability/usr/share/julia/Compiler/src/optimize.jl:1036",
"method_instance": "MethodInstance for Compiler.run_passes_ipo_safe(::Core.CodeInfo, ::Compiler.OptimizationState{Compiler.NativeInterpreter})"
},
"children": [
{
"method_instance": {
"method": "optimize(interp::Compiler.AbstractInterpreter, opt::Compiler.OptimizationState, caller::Compiler.InferenceResult) @ Compiler ~/tmp/jl/julia-git/Compiler_ssair_lift_svec_ref_closure_type_stability/usr/share/julia/Compiler/src/optimize.jl:1015",
"method_instance": "MethodInstance for Compiler.optimize(::Compiler.NativeInterpreter, ::Compiler.OptimizationState{Compiler.NativeInterpreter}, ::Compiler.InferenceResult)"
},
"children": [
{
"method_instance": {
"method": "typeinf_frame(interp::Compiler.AbstractInterpreter, mi::Core.MethodInstance, run_optimizer::Bool) @ Compiler ~/tmp/jl/julia-git/Compiler_ssair_lift_svec_ref_closure_type_stability/usr/share/julia/Compiler/src/typeinfer.jl:1065",
"method_instance": "MethodInstance for Compiler.typeinf_frame(::Compiler.NativeInterpreter, ::Core.MethodInstance, ::Bool)"
},
"children": [
]
}
]
}
]
}
]
}
]
},
{
"method_instance": {
"method": "sroa_pass!(ir::Compiler.IRCode, inlining::Union{Nothing, Compiler.InliningState}) @ Compiler ../usr/share/julia/Compiler/src/ssair/passes.jl:1212",
"method_instance": "MethodInstance for Compiler.sroa_pass!(::Compiler.IRCode, ::Nothing)"
},
"children": [
{
"method_instance": {
"method": "sroa_pass!(ir::Compiler.IRCode) @ Compiler ../usr/share/julia/Compiler/src/ssair/passes.jl:1212",
"method_instance": "MethodInstance for Compiler.sroa_pass!(::Compiler.IRCode)"
},
"children": [
]
}
]
}
]
}
]
}
]
},
{
"method_instance": {
"method": "any(f::Function, a::AbstractArray; dims) @ Base reducedim.jl:989",
"method_instance": "MethodInstance for any(::Function, ::UnitRange{Int64})"
},
"children": [
]
}
]
}
]
}
]
}
]
}
]
}
]
}
]
}
]
} |
bump |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sig
should be inferred to be DataType
so there shouldn't be no need to add annotations like this.
As explained in discussion on sibling PR #57631, the issue is that it's known to be something equal to a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this was correct until #40985 broke it
Making the closure capture a
SimpleVector
instead of a type should improve type stability. Also deduplicated some common subexpressions while at it.