Skip to content

Commit 1510eaa

Browse files
committed
optimizer: fix JuliaLang#42754, inline union-split const-prop'ed sources
This commit complements JuliaLang#39754 and JuliaLang#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 ) ```
1 parent 2e388e3 commit 1510eaa

File tree

2 files changed

+177
-47
lines changed

2 files changed

+177
-47
lines changed

base/compiler/ssair/inlining.jl

Lines changed: 100 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1147,9 +1147,10 @@ function process_simple!(ir::IRCode, todo::Vector{Pair{Int, Any}}, idx::Int, sta
11471147
return sig
11481148
end
11491149

1150+
# TODO inline non-`isdispatchtuple`, union-split callsites
11501151
function analyze_single_call!(
11511152
ir::IRCode, todo::Vector{Pair{Int, Any}}, idx::Int, @nospecialize(stmt),
1152-
sig::Signature, infos::Vector{MethodMatchInfo}, state::InliningState, flag::UInt8)
1153+
(; atypes, atype)::Signature, infos::Vector{MethodMatchInfo}, state::InliningState, flag::UInt8)
11531154
cases = InliningCase[]
11541155
local signature_union = Bottom
11551156
local only_method = nothing # keep track of whether there is one matching method
@@ -1181,7 +1182,7 @@ function analyze_single_call!(
11811182
fully_covered = false
11821183
continue
11831184
end
1184-
item = analyze_method!(match, sig.atypes, state, flag)
1185+
item = analyze_method!(match, atypes, state, flag)
11851186
if item === nothing
11861187
fully_covered = false
11871188
continue
@@ -1192,25 +1193,25 @@ function analyze_single_call!(
11921193
end
11931194
end
11941195

1195-
signature_fully_covered = sig.atype <: signature_union
1196-
# If we're fully covered and there's only one applicable method,
1197-
# we inline, even if the signature is not a dispatch tuple
1198-
if signature_fully_covered && length(cases) == 0 && only_method isa Method
1199-
if length(infos) > 1
1200-
(metharg, methsp) = ccall(:jl_type_intersection_with_env, Any, (Any, Any),
1201-
sig.atype, only_method.sig)::SimpleVector
1202-
match = MethodMatch(metharg, methsp, only_method, true)
1203-
else
1204-
meth = meth::MethodLookupResult
1205-
@assert length(meth) == 1
1206-
match = meth[1]
1196+
# if the signature is fully covered and there is only one applicable method,
1197+
# we can try to inline it even if the signature is not a dispatch tuple
1198+
if atype <: signature_union
1199+
if length(cases) == 0 && only_method isa Method
1200+
if length(infos) > 1
1201+
(metharg, methsp) = ccall(:jl_type_intersection_with_env, Any, (Any, Any),
1202+
atype, only_method.sig)::SimpleVector
1203+
match = MethodMatch(metharg, methsp, only_method, true)
1204+
else
1205+
meth = meth::MethodLookupResult
1206+
@assert length(meth) == 1
1207+
match = meth[1]
1208+
end
1209+
item = analyze_method!(match, atypes, state, flag)
1210+
item === nothing && return
1211+
push!(cases, InliningCase(match.spec_types, item))
1212+
fully_covered = true
12071213
end
1208-
fully_covered = true
1209-
item = analyze_method!(match, sig.atypes, state, flag)
1210-
item === nothing && return
1211-
push!(cases, InliningCase(match.spec_types, item))
1212-
end
1213-
if !signature_fully_covered
1214+
else
12141215
fully_covered = false
12151216
end
12161217

@@ -1219,36 +1220,81 @@ function analyze_single_call!(
12191220
# onto the todo list
12201221
if fully_covered && length(cases) == 1
12211222
handle_single_case!(ir, stmt, idx, cases[1].item, false, todo)
1222-
return
1223+
elseif length(cases) > 0
1224+
push!(todo, idx=>UnionSplit(fully_covered, atype, cases))
12231225
end
1224-
length(cases) == 0 && return
1225-
push!(todo, idx=>UnionSplit(fully_covered, sig.atype, cases))
12261226
return nothing
12271227
end
12281228

1229+
# try to create `InliningCase`s using constant-prop'ed results
1230+
# currently it works only when constant-prop' succeeded for all (union-split) signatures
1231+
# TODO use any of constant-prop'ed results, and leave the other unhandled cases to later
1232+
# TODO this function contains a lot of duplications with `analyze_single_call!`, factor them out
12291233
function maybe_handle_const_call!(
1230-
ir::IRCode, idx::Int, stmt::Expr, info::ConstCallInfo, sig::Signature,
1234+
ir::IRCode, idx::Int, stmt::Expr, (; results)::ConstCallInfo, (; atypes, atype)::Signature,
12311235
state::InliningState, flag::UInt8, isinvoke::Bool, todo::Vector{Pair{Int, Any}})
1232-
# when multiple matches are found, bail out and later inliner will union-split this signature
1233-
# TODO effectively use multiple constant analysis results here
1234-
length(info.results) == 1 || return false
1235-
result = info.results[1]
1236-
isa(result, InferenceResult) || return false
1237-
1238-
(; mi) = item = InliningTodo(result, sig.atypes)
1239-
validate_sparams(mi.sparam_vals) || return true
1240-
state.mi_cache !== nothing && (item = resolve_todo(item, state, flag))
1241-
if sig.atype <: mi.def.sig
1242-
handle_single_case!(ir, stmt, idx, item, isinvoke, todo)
1243-
return true
1236+
cases = InliningCase[] # TODO avoid this allocation for single cases ?
1237+
local fully_covered = true
1238+
local signature_union = Bottom
1239+
for result in results
1240+
isa(result, InferenceResult) || return false
1241+
(; mi) = item = InliningTodo(result, atypes)
1242+
spec_types = mi.specTypes
1243+
signature_union = Union{signature_union, spec_types}
1244+
if !isdispatchtuple(spec_types)
1245+
fully_covered = false
1246+
continue
1247+
end
1248+
if !validate_sparams(mi.sparam_vals)
1249+
fully_covered = false
1250+
continue
1251+
end
1252+
state.mi_cache !== nothing && (item = resolve_todo(item, state, flag))
1253+
if item === nothing
1254+
fully_covered = false
1255+
continue
1256+
end
1257+
push!(cases, InliningCase(spec_types, item))
1258+
end
1259+
1260+
# if the signature is fully covered and there is only one applicable method,
1261+
# we can try to inline it even if the signature is not a dispatch tuple
1262+
if atype <: signature_union
1263+
if length(cases) == 0 && length(results) == 1
1264+
(; mi) = item = InliningTodo(results[1]::InferenceResult, atypes)
1265+
state.mi_cache !== nothing && (item = resolve_todo(item, state, flag))
1266+
validate_sparams(mi.sparam_vals) || return true
1267+
item === nothing && return true
1268+
push!(cases, InliningCase(mi.specTypes, item))
1269+
fully_covered = true
1270+
end
12441271
else
1245-
item === nothing && return true
1246-
# Union split out the error case
1247-
item = UnionSplit(false, sig.atype, InliningCase[InliningCase(mi.specTypes, item)])
1272+
fully_covered = false
1273+
end
1274+
1275+
# If we only have one case and that case is fully covered, we may either
1276+
# be able to do the inlining now (for constant cases), or push it directly
1277+
# onto the todo list
1278+
if fully_covered && length(cases) == 1
1279+
handle_single_case!(ir, stmt, idx, cases[1].item, isinvoke, todo)
1280+
elseif length(cases) > 0
12481281
isinvoke && rewrite_invoke_exprargs!(stmt)
1249-
push!(todo, idx=>item)
1250-
return true
1282+
push!(todo, idx=>UnionSplit(fully_covered, atype, cases))
12511283
end
1284+
return true
1285+
end
1286+
1287+
function handle_const_opaque_closure_call!(
1288+
ir::IRCode, idx::Int, stmt::Expr, (; results)::ConstCallInfo,
1289+
(; atypes)::Signature, state::InliningState, flag::UInt8, todo::Vector{Pair{Int, Any}})
1290+
@assert length(results) == 1
1291+
result = results[1]::InferenceResult
1292+
item = InliningTodo(result, atypes)
1293+
isdispatchtuple(item.mi.specTypes) || return
1294+
validate_sparams(item.mi.sparam_vals) || return
1295+
state.mi_cache !== nothing && (item = resolve_todo(item, state, flag))
1296+
handle_single_case!(ir, stmt, idx, item, false, todo)
1297+
return nothing
12521298
end
12531299

12541300
function assemble_inline_todo!(ir::IRCode, state::InliningState)
@@ -1283,18 +1329,25 @@ function assemble_inline_todo!(ir::IRCode, state::InliningState)
12831329
# if inference arrived here with constant-prop'ed result(s),
12841330
# we can perform a specialized analysis for just this case
12851331
if isa(info, ConstCallInfo)
1286-
if !is_stmt_noinline(flag) && maybe_handle_const_call!(
1287-
ir, idx, stmt, info, sig,
1288-
state, flag, sig.f === Core.invoke, todo)
1289-
continue
1332+
if !is_stmt_noinline(flag)
1333+
if isa(info.call, OpaqueClosureCallInfo)
1334+
handle_const_opaque_closure_call!(
1335+
ir, idx, stmt, info,
1336+
sig, state, flag, todo)
1337+
continue
1338+
else
1339+
maybe_handle_const_call!(
1340+
ir, idx, stmt, info, sig,
1341+
state, flag, sig.f === Core.invoke, todo) && continue
1342+
end
12901343
else
12911344
info = info.call
12921345
end
12931346
end
12941347

12951348
if isa(info, OpaqueClosureCallInfo)
1296-
result = analyze_method!(info.match, sig.atypes, state, flag)
1297-
handle_single_case!(ir, stmt, idx, result, false, todo)
1349+
item = analyze_method!(info.match, sig.atypes, state, flag)
1350+
handle_single_case!(ir, stmt, idx, item, false, todo)
12981351
continue
12991352
end
13001353

test/compiler/inline.jl

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -680,3 +680,80 @@ let f(x) = (x...,)
680680
# the the original apply call is not union-split, but the inserted `iterate` call is.
681681
@test code_typed(f, Tuple{Union{Int64, CartesianIndex{1}, CartesianIndex{3}}})[1][2] == Tuple{Int64}
682682
end
683+
684+
# https://github.com/JuliaLang/julia/issues/42754
685+
# inline union-split constant-prop'ed sources
686+
mutable struct X42754
687+
# NOTE in order to confuse `fieldtype_tfunc`, we need to have at least two fields with different types
688+
a::Union{Nothing, Int}
689+
b::Symbol
690+
end
691+
let code = code_typed1((X42754, Union{Nothing,Int})) do x, a
692+
# this `setproperty` call would be union-split and constant-prop will happen for
693+
# each signature: inlining would fail if we don't use constant-prop'ed source
694+
# since the approximate inlining cost of `convert(fieldtype(X, sym), a)` would
695+
# end up very high if we don't propagate `sym::Const(:a)`
696+
x.a = a
697+
x
698+
end
699+
@test all(code) do @nospecialize(x)
700+
isinvoke(x, :setproperty!) && return false
701+
if Meta.isexpr(x, :call)
702+
f = x.args[1]
703+
isa(f, GlobalRef) && f.name === :setproperty! && return false
704+
end
705+
return true
706+
end
707+
end
708+
709+
import Base: @constprop
710+
711+
# test single, non-dispatchtuple callsite inlining
712+
713+
@constprop :none @inline test_single_nondispatchtuple(@nospecialize(t)) =
714+
isa(t, DataType) && t.name === Type.body.name
715+
let
716+
code = code_typed1((Any,)) do x
717+
test_single_nondispatchtuple(x)
718+
end
719+
@test all(code) do @nospecialize(x)
720+
isinvoke(x, :test_single_nondispatchtuple) && return false
721+
if Meta.isexpr(x, :call)
722+
f = x.args[1]
723+
isa(f, GlobalRef) && f.name === :test_single_nondispatchtuple && return false
724+
end
725+
return true
726+
end
727+
end
728+
729+
@constprop :aggressive @inline test_single_nondispatchtuple(c, @nospecialize(t)) =
730+
c && isa(t, DataType) && t.name === Type.body.name
731+
let
732+
code = code_typed1((Any,)) do x
733+
test_single_nondispatchtuple(true, x)
734+
end
735+
@test all(code) do @nospecialize(x)
736+
isinvoke(x, :test_single_nondispatchtuple) && return false
737+
if Meta.isexpr(x, :call)
738+
f = x.args[1]
739+
isa(f, GlobalRef) && f.name === :test_single_nondispatchtuple && return false
740+
end
741+
return true
742+
end
743+
end
744+
745+
# validate inlining processing
746+
747+
@constprop :none @inline validate_unionsplit_inlining(@nospecialize(t)) = throw("invalid inlining processing detected")
748+
@constprop :none @noinline validate_unionsplit_inlining(i::Integer) = (println(IOBuffer(), "prevent inlining"); false)
749+
let
750+
invoke(xs) = validate_unionsplit_inlining(xs[1])
751+
@test invoke(Any[10]) === false
752+
end
753+
754+
@constprop :aggressive @inline validate_unionsplit_inlining(c, @nospecialize(t)) = c && throw("invalid inlining processing detected")
755+
@constprop :aggressive @noinline validate_unionsplit_inlining(c, i::Integer) = c && (println(IOBuffer(), "prevent inlining"); false)
756+
let
757+
invoke(xs) = validate_unionsplit_inlining(true, xs[1])
758+
@test invoke(Any[10]) === false
759+
end

0 commit comments

Comments
 (0)