Skip to content

Commit da71d29

Browse files
committed
manually-cherry-picked: optimizer: fix #42754, inline union-split const-prop'ed sources
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 ) ```
1 parent e0f980e commit da71d29

File tree

2 files changed

+184
-47
lines changed

2 files changed

+184
-47
lines changed

base/compiler/ssair/inlining.jl

Lines changed: 97 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,9 +1202,10 @@ function process_simple!(ir::IRCode, todo::Vector{Pair{Int, Any}}, idx::Int, sta
12021202
return sig
12031203
end
12041204

1205+
# TODO inline non-`isdispatchtuple`, union-split callsites
12051206
function analyze_single_call!(
12061207
ir::IRCode, todo::Vector{Pair{Int, Any}}, idx::Int, @nospecialize(stmt),
1207-
sig::Signature, infos::Vector{MethodMatchInfo}, state::InliningState)
1208+
(; atypes, atype)::Signature, infos::Vector{MethodMatchInfo}, state::InliningState)
12081209
cases = InliningCase[]
12091210
local signature_union = Bottom
12101211
local only_method = nothing # keep track of whether there is one matching method
@@ -1236,7 +1237,7 @@ function analyze_single_call!(
12361237
fully_covered = false
12371238
continue
12381239
end
1239-
item = analyze_method!(match, sig.atypes, state)
1240+
item = analyze_method!(match, atypes, state)
12401241
if item === nothing
12411242
fully_covered = false
12421243
continue
@@ -1247,25 +1248,25 @@ function analyze_single_call!(
12471248
end
12481249
end
12491250

1250-
signature_fully_covered = sig.atype <: signature_union
1251-
# If we're fully covered and there's only one applicable method,
1252-
# we inline, even if the signature is not a dispatch tuple
1253-
if signature_fully_covered && length(cases) == 0 && only_method isa Method
1254-
if length(infos) > 1
1255-
(metharg, methsp) = ccall(:jl_type_intersection_with_env, Any, (Any, Any),
1256-
sig.atype, only_method.sig)::SimpleVector
1257-
match = MethodMatch(metharg, methsp, only_method, true)
1258-
else
1259-
meth = meth::MethodLookupResult
1260-
@assert length(meth) == 1
1261-
match = meth[1]
1251+
# if the signature is fully covered and there is only one applicable method,
1252+
# we can try to inline it even if the signature is not a dispatch tuple
1253+
if atype <: signature_union
1254+
if length(cases) == 0 && only_method isa Method
1255+
if length(infos) > 1
1256+
(metharg, methsp) = ccall(:jl_type_intersection_with_env, Any, (Any, Any),
1257+
atype, only_method.sig)::SimpleVector
1258+
match = MethodMatch(metharg, methsp, only_method, true)
1259+
else
1260+
meth = meth::MethodLookupResult
1261+
@assert length(meth) == 1
1262+
match = meth[1]
1263+
end
1264+
item = analyze_method!(match, atypes, state)
1265+
item === nothing && return
1266+
push!(cases, InliningCase(match.spec_types, item))
1267+
fully_covered = true
12621268
end
1263-
fully_covered = true
1264-
item = analyze_method!(match, sig.atypes, state)
1265-
item === nothing && return
1266-
push!(cases, InliningCase(match.spec_types, item))
1267-
end
1268-
if !signature_fully_covered
1269+
else
12691270
fully_covered = false
12701271
end
12711272

@@ -1274,36 +1275,81 @@ function analyze_single_call!(
12741275
# onto the todo list
12751276
if fully_covered && length(cases) == 1
12761277
handle_single_case!(ir, stmt, idx, cases[1].item, false, todo)
1277-
return
1278+
elseif length(cases) > 0
1279+
push!(todo, idx=>UnionSplit(fully_covered, atype, cases))
12781280
end
1279-
length(cases) == 0 && return
1280-
push!(todo, idx=>UnionSplit(fully_covered, sig.atype, cases))
12811281
return nothing
12821282
end
12831283

1284+
# try to create `InliningCase`s using constant-prop'ed results
1285+
# currently it works only when constant-prop' succeeded for all (union-split) signatures
1286+
# TODO use any of constant-prop'ed results, and leave the other unhandled cases to later
1287+
# TODO this function contains a lot of duplications with `analyze_single_call!`, factor them out
12841288
function maybe_handle_const_call!(
1285-
ir::IRCode, idx::Int, stmt::Expr, info::ConstCallInfo, sig::Signature,
1289+
ir::IRCode, idx::Int, stmt::Expr, (; results)::ConstCallInfo, (; atypes, atype)::Signature,
12861290
state::InliningState, isinvoke::Bool, todo::Vector{Pair{Int, Any}})
1287-
# when multiple matches are found, bail out and later inliner will union-split this signature
1288-
# TODO effectively use multiple constant analysis results here
1289-
length(info.results) == 1 || return false
1290-
result = info.results[1]
1291-
isa(result, InferenceResult) || return false
1292-
1293-
(; mi) = item = InliningTodo(result, sig.atypes)
1294-
validate_sparams(mi.sparam_vals) || return true
1295-
state.mi_cache !== nothing && (item = resolve_todo(item, state))
1296-
if sig.atype <: mi.def.sig
1297-
handle_single_case!(ir, stmt, idx, item, isinvoke, todo)
1298-
return true
1291+
cases = InliningCase[] # TODO avoid this allocation for single cases ?
1292+
local fully_covered = true
1293+
local signature_union = Bottom
1294+
for result in results
1295+
isa(result, InferenceResult) || return false
1296+
(; mi) = item = InliningTodo(result, atypes)
1297+
spec_types = mi.specTypes
1298+
signature_union = Union{signature_union, spec_types}
1299+
if !isdispatchtuple(spec_types)
1300+
fully_covered = false
1301+
continue
1302+
end
1303+
if !validate_sparams(mi.sparam_vals)
1304+
fully_covered = false
1305+
continue
1306+
end
1307+
state.mi_cache !== nothing && (item = resolve_todo(item, state))
1308+
if item === nothing
1309+
fully_covered = false
1310+
continue
1311+
end
1312+
push!(cases, InliningCase(spec_types, item))
1313+
end
1314+
1315+
# if the signature is fully covered and there is only one applicable method,
1316+
# we can try to inline it even if the signature is not a dispatch tuple
1317+
if atype <: signature_union
1318+
if length(cases) == 0 && length(results) == 1
1319+
(; mi) = item = InliningTodo(results[1]::InferenceResult, atypes)
1320+
state.mi_cache !== nothing && (item = resolve_todo(item, state))
1321+
validate_sparams(mi.sparam_vals) || return true
1322+
item === nothing && return true
1323+
push!(cases, InliningCase(mi.specTypes, item))
1324+
fully_covered = true
1325+
end
12991326
else
1300-
item === nothing && return true
1301-
# Union split out the error case
1302-
item = UnionSplit(false, sig.atype, InliningCase[InliningCase(mi.specTypes, item)])
1327+
fully_covered = false
1328+
end
1329+
1330+
# If we only have one case and that case is fully covered, we may either
1331+
# be able to do the inlining now (for constant cases), or push it directly
1332+
# onto the todo list
1333+
if fully_covered && length(cases) == 1
1334+
handle_single_case!(ir, stmt, idx, cases[1].item, isinvoke, todo)
1335+
elseif length(cases) > 0
13031336
isinvoke && rewrite_invoke_exprargs!(stmt)
1304-
push!(todo, idx=>item)
1305-
return true
1337+
push!(todo, idx=>UnionSplit(fully_covered, atype, cases))
13061338
end
1339+
return true
1340+
end
1341+
1342+
function handle_const_opaque_closure_call!(
1343+
ir::IRCode, idx::Int, stmt::Expr, (; results)::ConstCallInfo,
1344+
(; atypes)::Signature, state::InliningState, todo::Vector{Pair{Int, Any}})
1345+
@assert length(results) == 1
1346+
result = results[1]::InferenceResult
1347+
item = InliningTodo(result, atypes)
1348+
isdispatchtuple(item.mi.specTypes) || return
1349+
validate_sparams(item.mi.sparam_vals) || return
1350+
state.mi_cache !== nothing && (item = resolve_todo(item, state))
1351+
handle_single_case!(ir, stmt, idx, item, false, todo)
1352+
return nothing
13071353
end
13081354

13091355
function assemble_inline_todo!(ir::IRCode, state::InliningState)
@@ -1336,18 +1382,22 @@ function assemble_inline_todo!(ir::IRCode, state::InliningState)
13361382
# if inference arrived here with constant-prop'ed result(s),
13371383
# we can perform a specialized analysis for just this case
13381384
if isa(info, ConstCallInfo)
1339-
if maybe_handle_const_call!(
1340-
ir, idx, stmt, info, sig,
1341-
state, sig.f === Core.invoke, todo)
1385+
if isa(info.call, OpaqueClosureCallInfo)
1386+
handle_const_opaque_closure_call!(
1387+
ir, idx, stmt, info,
1388+
sig, state, todo)
13421389
continue
13431390
else
1344-
info = info.call
1391+
maybe_handle_const_call!(
1392+
ir, idx, stmt, info, sig,
1393+
state, sig.f === Core.invoke, todo) && continue
13451394
end
1395+
info = info.call
13461396
end
13471397

13481398
if isa(info, OpaqueClosureCallInfo)
1349-
result = analyze_method!(info.match, sig.atypes, state)
1350-
handle_single_case!(ir, stmt, idx, result, false, todo)
1399+
item = analyze_method!(info.match, sig.atypes, state)
1400+
handle_single_case!(ir, stmt, idx, item, false, todo)
13511401
continue
13521402
end
13531403

test/compiler/inline.jl

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,3 +392,90 @@ let f(x) = (x...,)
392392
# the the original apply call is not union-split, but the inserted `iterate` call is.
393393
@test code_typed(f, Tuple{Union{Int64, CartesianIndex{1}, CartesianIndex{3}}})[1][2] == Tuple{Int64}
394394
end
395+
396+
# check if `x` is a statically-resolved call of a function whose name is `sym`
397+
isinvoke(@nospecialize(x), sym::Symbol) = isinvoke(x, mi->mi.def.name===sym)
398+
function isinvoke(@nospecialize(x), pred)
399+
if Meta.isexpr(x, :invoke)
400+
return pred(x.args[1]::Core.MethodInstance)
401+
end
402+
return false
403+
end
404+
code_typed1(args...; kwargs...) = (first(only(code_typed(args...; kwargs...)))::Core.CodeInfo).code
405+
406+
# https://github.com/JuliaLang/julia/issues/42754
407+
# inline union-split constant-prop'ed sources
408+
mutable struct X42754
409+
# NOTE in order to confuse `fieldtype_tfunc`, we need to have at least two fields with different types
410+
a::Union{Nothing, Int}
411+
b::Symbol
412+
end
413+
let code = code_typed1((X42754, Union{Nothing,Int})) do x, a
414+
# this `setproperty` call would be union-split and constant-prop will happen for
415+
# each signature: inlining would fail if we don't use constant-prop'ed source
416+
# since the approximate inlining cost of `convert(fieldtype(X, sym), a)` would
417+
# end up very high if we don't propagate `sym::Const(:a)`
418+
x.a = a
419+
x
420+
end
421+
@test all(code) do @nospecialize(x)
422+
isinvoke(x, :setproperty!) && return false
423+
if Meta.isexpr(x, :call)
424+
f = x.args[1]
425+
isa(f, GlobalRef) && f.name === :setproperty! && return false
426+
end
427+
return true
428+
end
429+
end
430+
431+
import Base: @constprop
432+
433+
# test single, non-dispatchtuple callsite inlining
434+
435+
@constprop :none @inline test_single_nondispatchtuple(@nospecialize(t)) =
436+
isa(t, DataType) && t.name === Type.body.name
437+
let
438+
code = code_typed1((Any,)) do x
439+
test_single_nondispatchtuple(x)
440+
end
441+
@test all(code) do @nospecialize(x)
442+
isinvoke(x, :test_single_nondispatchtuple) && return false
443+
if Meta.isexpr(x, :call)
444+
f = x.args[1]
445+
isa(f, GlobalRef) && f.name === :test_single_nondispatchtuple && return false
446+
end
447+
return true
448+
end
449+
end
450+
451+
@constprop :aggressive @inline test_single_nondispatchtuple(c, @nospecialize(t)) =
452+
c && isa(t, DataType) && t.name === Type.body.name
453+
let
454+
code = code_typed1((Any,)) do x
455+
test_single_nondispatchtuple(true, x)
456+
end
457+
@test all(code) do @nospecialize(x)
458+
isinvoke(x, :test_single_nondispatchtuple) && return false
459+
if Meta.isexpr(x, :call)
460+
f = x.args[1]
461+
isa(f, GlobalRef) && f.name === :test_single_nondispatchtuple && return false
462+
end
463+
return true
464+
end
465+
end
466+
467+
# validate inlining processing
468+
469+
@constprop :none @inline validate_unionsplit_inlining(@nospecialize(t)) = throw("invalid inlining processing detected")
470+
@constprop :none @noinline validate_unionsplit_inlining(i::Integer) = (println(IOBuffer(), "prevent inlining"); false)
471+
let
472+
invoke(xs) = validate_unionsplit_inlining(xs[1])
473+
@test invoke(Any[10]) === false
474+
end
475+
476+
@constprop :aggressive @inline validate_unionsplit_inlining(c, @nospecialize(t)) = c && throw("invalid inlining processing detected")
477+
@constprop :aggressive @noinline validate_unionsplit_inlining(c, i::Integer) = c && (println(IOBuffer(), "prevent inlining"); false)
478+
let
479+
invoke(xs) = validate_unionsplit_inlining(true, xs[1])
480+
@test invoke(Any[10]) === false
481+
end

0 commit comments

Comments
 (0)