Skip to content

Commit f6d1f0b

Browse files
committed
inlining: relax finalizer inlining control-flow restriction
Eager `finalizer` inlining (#45272) currently has a restriction that requires all the def/uses are in a same basic block. This commit relaxes that restriction a bit by allowing def/uses to involve control flow when all of them are dominated by a `finalizer` call to be inlined, since in that case it is safe to insert the body of `finalizer` at the end of all the def/uses, e.g. ```julia Base.@assume_effects :nothrow safeprint(@nospecialize x...) = print(x...) @noinline nothrow_side_effect(x) = Base.@assume_effects :total !:effect_free @CCall jl_(x::Any)::Cvoid mutable struct DoAllocWithFieldInter x end function register_finalizer!(obj::DoAllocWithFieldInter) finalizer(obj) do this nothrow_side_effect(nothing) end end let src = code_typed1() do for i = -1000:1000 o = DoAllocWithFieldInter(i) register_finalizer!(o) if i == 1000 safeprint(o.x, '\n') elseif i > 0 safeprint(o.x) end end end # the finalzier call gets inlined @test count(isinvoke(:nothrow_side_effect), src.code) == 1 end ``` There is a room for further improvement though -- if we have a way to compute the post-domination, we can also inline a `finalizer` call in a case when a `finalizer` block is post-dominated by all the def/uses e.g. ``` let src = code_typed1() do for i = -1000:1000 o = DoAllocWithFieldInter(i) if i == 1000 safeprint(o.x, '\n') elseif i > 0 safeprint(o.x) end register_finalizer!(o) end end # currently this is broken @test_broken count(isinvoke(:nothrow_side_effect), src.code) == 1 end ```
1 parent 1670e30 commit f6d1f0b

File tree

3 files changed

+123
-23
lines changed

3 files changed

+123
-23
lines changed

base/compiler/ssair/passes.jl

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,32 +1100,42 @@ end
11001100

11011101
is_nothrow(ir::IRCode, pc::Int) = ir.stmts[pc][:flag] & (IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW) 0
11021102

1103-
function try_resolve_finalizer!(ir::IRCode, idx::Int, finalizer_idx::Int, defuse::SSADefUse, inlining::InliningState)
1104-
# For now: Require that all uses and defs are in the same basic block,
1105-
# so that live range calculations are easy.
1106-
bb = ir.cfg.blocks[block_for_inst(ir.cfg, first(defuse.uses).idx)]
1103+
function try_resolve_finalizer!(ir::IRCode, idx::Int, finalizer_idx::Int, defuse::SSADefUse, inlining::InliningState, lazydomtree::LazyDomtree)
1104+
# For now: Require that all the uses and defs are dominated by `finalizer` block
1105+
# TODO: further relax this restriction by allowing a case when `finalizer` block is post-dominated by them
11071106
minval::Int = typemax(Int)
11081107
maxval::Int = 0
1109-
1110-
function check_in_range(x::Union{Int,SSAUse})
1111-
if isa(x, SSAUse)
1112-
didx = x.idx
1108+
function update_idx!(duidx::Int)
1109+
if duidx < minval
1110+
minval = duidx
1111+
end
1112+
if duidx > maxval
1113+
maxval = duidx
1114+
end
1115+
end
1116+
1117+
domtree = get!(lazydomtree)
1118+
finalizer_bb = block_for_inst(ir, finalizer_idx)
1119+
update_idx!(finalizer_idx)
1120+
alloc_bb = block_for_inst(ir, idx)
1121+
dominates(domtree, alloc_bb, finalizer_bb) || return nothing
1122+
update_idx!(idx)
1123+
function check_defuse(x::Union{Int,SSAUse})
1124+
duidx = x isa SSAUse ? x.idx : x
1125+
duidx == finalizer_idx && return true
1126+
bb = block_for_inst(ir, duidx)
1127+
if dominates(domtree, finalizer_bb, bb)
1128+
update_idx!(duidx)
1129+
return true
1130+
# TODO elseif post_dominates(domtree, bb, finalizer_bb)
1131+
# update_idx!(duidx)
1132+
# return true
11131133
else
1114-
didx = x
1115-
end
1116-
didx in bb.stmts || return false
1117-
if didx < minval
1118-
minval = didx
1134+
return false
11191135
end
1120-
if didx > maxval
1121-
maxval = didx
1122-
end
1123-
return true
11241136
end
1125-
1126-
check_in_range(idx) || return nothing
1127-
all(check_in_range, defuse.uses) || return nothing
1128-
all(check_in_range, defuse.defs) || return nothing
1137+
all(check_defuse, defuse.uses) || return nothing
1138+
all(check_defuse, defuse.defs) || return nothing
11291139

11301140
# For now: Require all statements in the basic block range to be nothrow.
11311141
all(minval:maxval) do idx::Int
@@ -1184,7 +1194,7 @@ function sroa_mutables!(ir::IRCode, defuses::IdDict{Int, Tuple{SPCSet, SSADefUse
11841194
end
11851195
end
11861196
if finalizer_idx !== nothing && inlining !== nothing
1187-
try_resolve_finalizer!(ir, idx, finalizer_idx, defuse, inlining)
1197+
try_resolve_finalizer!(ir, idx, finalizer_idx, defuse, inlining, lazydomtree)
11881198
continue
11891199
end
11901200
# Partition defuses by field

test/compiler/inline.jl

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1377,6 +1377,91 @@ let
13771377
@test get_finalization_count() == 1000
13781378
end
13791379

1380+
# tests finalizer inlining when def/uses involve control flow
1381+
Base.@assume_effects :nothrow safeprint(@nospecialize x...) = print(x...)
1382+
mutable struct DoAllocWithField
1383+
x
1384+
function DoAllocWithField(x)
1385+
finalizer(new(x)) do this
1386+
nothrow_side_effect(nothing)
1387+
end
1388+
end
1389+
end
1390+
mutable struct DoAllocWithFieldInter
1391+
x
1392+
end
1393+
function register_finalizer!(obj::DoAllocWithFieldInter)
1394+
finalizer(obj) do this
1395+
nothrow_side_effect(nothing)
1396+
end
1397+
end
1398+
let src = code_typed1() do
1399+
for i = -1000:1000
1400+
o = DoAllocWithField(i)
1401+
if i == 1000
1402+
safeprint(o.x, '\n')
1403+
elseif i > 0
1404+
safeprint(o.x)
1405+
end
1406+
end
1407+
end
1408+
@test count(isinvoke(:nothrow_side_effect), src.code) == 1
1409+
end
1410+
let src = code_typed1() do
1411+
for i = -1000:1000
1412+
o = DoAllocWithField(0)
1413+
o.x = i # with `setfield!`
1414+
if i == 1000
1415+
safeprint(o.x, '\n')
1416+
elseif i > 0
1417+
safeprint(o.x)
1418+
end
1419+
end
1420+
end
1421+
@test count(isinvoke(:nothrow_side_effect), src.code) == 1
1422+
end
1423+
let src = code_typed1() do
1424+
for i = -1000:1000
1425+
o = DoAllocWithFieldInter(i)
1426+
register_finalizer!(o)
1427+
if i == 1000
1428+
safeprint(o.x, '\n')
1429+
elseif i > 0
1430+
safeprint(o.x)
1431+
end
1432+
end
1433+
end
1434+
@test count(isinvoke(:nothrow_side_effect), src.code) == 1
1435+
end
1436+
let src = code_typed1() do
1437+
for i = -1000:1000
1438+
o = DoAllocWithFieldInter(0)
1439+
o.x = i # with `setfield!``
1440+
register_finalizer!(o)
1441+
if i == 1000
1442+
safeprint(o.x, '\n')
1443+
elseif i > 0
1444+
safeprint(o.x)
1445+
end
1446+
end
1447+
end
1448+
@test count(isinvoke(:nothrow_side_effect), src.code) == 1
1449+
end
1450+
let src = code_typed1() do
1451+
for i = -1000:1000
1452+
o = DoAllocWithFieldInter(i)
1453+
if i == 1000
1454+
safeprint(o.x, '\n')
1455+
elseif i > 0
1456+
safeprint(o.x)
1457+
end
1458+
register_finalizer!(o)
1459+
end
1460+
end
1461+
# TODO we can fix this case by checking a case when a finalizer block is post-dominated by all the def/uses
1462+
@test_broken count(isinvoke(:nothrow_side_effect), src.code) == 1
1463+
end
1464+
13801465
# optimize `[push!|pushfirst!](::Vector{Any}, x...)`
13811466
@testset "optimize `$f(::Vector{Any}, x...)`" for f = Any[push!, pushfirst!]
13821467
@eval begin

test/compiler/irutils.jl

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,12 @@ function iscall((src, f)::Tuple{IR,Base.Callable}, @nospecialize(x)) where IR<:U
1717
singleton_type(argextype(x, src)) === f
1818
end
1919
end
20-
iscall(pred::Base.Callable, @nospecialize(x)) = isexpr(x, :call) && pred(x.args[1])
20+
function iscall(pred::Base.Callable, @nospecialize(x))
21+
if isexpr(x, :(=))
22+
x = x.args[2]
23+
end
24+
return isexpr(x, :call) && pred(x.args[1])
25+
end
2126

2227
# check if `x` is a statically-resolved call of a function whose name is `sym`
2328
isinvoke(y) = @nospecialize(x) -> isinvoke(y, x)

0 commit comments

Comments
 (0)