From fe18ecfa1e61fc43ca6d7a4d72397b1ca642977f Mon Sep 17 00:00:00 2001 From: Shuhei Kadowaki Date: Mon, 25 Oct 2021 01:35:12 +0900 Subject: [PATCH] optimizer: fix #42754, inline union-split const-prop'ed sources MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit complements #39754 and #39305: implements a logic to use constant-prop'ed results for inlining at union-split callsite. Currently it works only for cases when constant-prop' succeeded for all (union-split) signatures. > example ```julia julia> mutable struct X # NOTE in order to confuse `fieldtype_tfunc`, we need to have at least two fields with different types a::Union{Nothing, Int} b::Symbol end; julia> code_typed((X, Union{Nothing,Int})) do x, a # this `setproperty` call would be union-split and constant-prop will happen for # each signature: inlining would fail if we don't use constant-prop'ed source # since the approximated inlining cost of `convert(fieldtype(X, sym), a)` would # end up very high if we don't propagate `sym::Const(:a)` x.a = a x end |> only |> first ``` > before this commit ```julia CodeInfo( 1 ─ %1 = Base.setproperty!::typeof(setproperty!) │ %2 = (isa)(a, Nothing)::Bool └── goto #3 if not %2 2 ─ %4 = π (a, Nothing) │ invoke %1(_2::X, :a::Symbol, %4::Nothing)::Any └── goto #6 3 ─ %7 = (isa)(a, Int64)::Bool └── goto #5 if not %7 4 ─ %9 = π (a, Int64) │ invoke %1(_2::X, :a::Symbol, %9::Int64)::Any └── goto #6 5 ─ Core.throw(ErrorException("fatal error in type inference (type bound)"))::Union{} └── unreachable 6 ┄ return x ) ``` > after this commit ```julia CodeInfo( 1 ─ %1 = (isa)(a, Nothing)::Bool └── goto #3 if not %1 2 ─ Base.setfield!(x, :a, nothing)::Nothing └── goto #6 3 ─ %5 = (isa)(a, Int64)::Bool └── goto #5 if not %5 4 ─ %7 = π (a, Int64) │ Base.setfield!(x, :a, %7)::Int64 └── goto #6 5 ─ Core.throw(ErrorException("fatal error in type inference (type bound)"))::Union{} └── unreachable 6 ┄ return x ) ``` --- base/compiler/ssair/inlining.jl | 45 ++++++++++++++++++++------------- test/compiler/inline.jl | 27 ++++++++++++++++++++ 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/base/compiler/ssair/inlining.jl b/base/compiler/ssair/inlining.jl index 5fc96ba626c8a..53f054494d0e1 100644 --- a/base/compiler/ssair/inlining.jl +++ b/base/compiler/ssair/inlining.jl @@ -1226,29 +1226,40 @@ function analyze_single_call!( return nothing end +# try to create `InliningTodo`s using constant-prop'ed results +# currently it works only when constant-prop' succeeded for all (union-split) signatures +# TODO use any of constant-prop'ed results, and leave the other unhandled cases to later function maybe_handle_const_call!( ir::IRCode, idx::Int, stmt::Expr, info::ConstCallInfo, sig::Signature, state::InliningState, flag::UInt8, isinvoke::Bool, todo::Vector{Pair{Int, Any}}) - # when multiple matches are found, bail out and later inliner will union-split this signature - # TODO effectively use multiple constant analysis results here - length(info.results) == 1 || return false - result = info.results[1] - isa(result, InferenceResult) || return false - - (; mi) = item = InliningTodo(result, sig.atypes) - validate_sparams(mi.sparam_vals) || return true - state.mi_cache !== nothing && (item = resolve_todo(item, state, flag)) - if sig.atype <: mi.def.sig - handle_single_case!(ir, stmt, idx, item, isinvoke, todo) - return true - else - item === nothing && return true - # Union split out the error case - item = UnionSplit(false, sig.atype, InliningCase[InliningCase(mi.specTypes, item)]) + cases = InliningCase[] # TODO avoid this allocation for single cases ? + local fully_covered = true + local signature_union = Bottom + for result in info.results + isa(result, InferenceResult) || return false + (; mi) = item = InliningTodo(result, sig.atypes) + if !validate_sparams(mi.sparam_vals) + fully_covered = false + continue + end + state.mi_cache !== nothing && (item = resolve_todo(item, state, flag)) + if item === nothing + fully_covered = false + continue + end + push!(cases, InliningCase(mi.specTypes, item)) + signature_union = Union{signature_union, mi.def.sig} + end + sigtype = sig.atype + fully_covered &= sigtype <: signature_union + if fully_covered && length(cases) == 1 + handle_single_case!(ir, stmt, idx, cases[1].item, isinvoke, todo) + elseif length(cases) > 0 + item = UnionSplit(fully_covered, sigtype, cases) isinvoke && rewrite_invoke_exprargs!(stmt) push!(todo, idx=>item) - return true end + return true end function assemble_inline_todo!(ir::IRCode, state::InliningState) diff --git a/test/compiler/inline.jl b/test/compiler/inline.jl index 6bdb71bf8f292..00b9bebd7ff0f 100644 --- a/test/compiler/inline.jl +++ b/test/compiler/inline.jl @@ -680,3 +680,30 @@ let f(x) = (x...,) # the the original apply call is not union-split, but the inserted `iterate` call is. @test code_typed(f, Tuple{Union{Int64, CartesianIndex{1}, CartesianIndex{3}}})[1][2] == Tuple{Int64} end + +let # https://github.com/JuliaLang/julia/issues/42754 + # inline union-split constant-prop'ed sources + code = @eval Module() begin + mutable struct X + # NOTE in order to confuse `fieldtype_tfunc`, we need to have at least two fields with different types + a::Union{Nothing, Int} + b::Symbol + end + $code_typed1((X, Union{Nothing,Int})) do x, a + # this `setproperty` call would be union-split and constant-prop will happen for + # each signature: inlining would fail if we don't use constant-prop'ed source + # since the approximate inlining cost of `convert(fieldtype(X, sym), a)` would + # end up very high if we don't propagate `sym::Const(:a)` + x.a = a + x + end + end + @test all(code) do @nospecialize(x) + isinvoke(x, :setproperty!) && return false + if Meta.isexpr(x, :call) + f = x.args[1] + isa(f, GlobalRef) && f.name === :setproperty! && return false + end + return true + end +end