Skip to content

Commit

Permalink
optimizer: fix #42754, inline union-split const-prop'ed sources
Browse files Browse the repository at this point in the history
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, 🅰️: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, 🅰️: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
)
```
  • Loading branch information
aviatesk committed Oct 24, 2021
1 parent 774ca28 commit 13d9503
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 17 deletions.
45 changes: 28 additions & 17 deletions base/compiler/ssair/inlining.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1233,29 +1233,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)
Expand Down
27 changes: 27 additions & 0 deletions test/compiler/inline.jl
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 13d9503

Please sign in to comment.