Skip to content

Commit 98f4747

Browse files
authored
optimizer: do not delete statements that may not :terminate (#52999)
Fixes #52991. Currently this commit marks the test case added in #52954 as `broken` since it has relied on the behavior of #52991. I'm planning to add followup changes in a separate commit.
1 parent 7e7e280 commit 98f4747

File tree

3 files changed

+66
-25
lines changed

3 files changed

+66
-25
lines changed

base/compiler/optimize.jl

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ const IR_FLAG_INACCESSIBLEMEM_OR_ARGMEM = one(UInt32) << 11
4848
const NUM_IR_FLAGS = 12 # sync with julia.h
4949

5050
const IR_FLAGS_EFFECTS =
51-
IR_FLAG_CONSISTENT | IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW | IR_FLAG_NOUB
51+
IR_FLAG_CONSISTENT | IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW | IR_FLAG_TERMINATES | IR_FLAG_NOUB
5252

53-
const IR_FLAGS_REMOVABLE = IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW
53+
const IR_FLAGS_REMOVABLE = IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW | IR_FLAG_TERMINATES
5454

5555
const IR_FLAGS_NEEDS_EA = IR_FLAG_EFIIMO | IR_FLAG_INACCESSIBLEMEM_OR_ARGMEM
5656

@@ -69,6 +69,9 @@ function flags_for_effects(effects::Effects)
6969
if is_nothrow(effects)
7070
flags |= IR_FLAG_NOTHROW
7171
end
72+
if is_terminates(effects)
73+
flags |= IR_FLAG_TERMINATES
74+
end
7275
if is_inaccessiblemem_or_argmemonly(effects)
7376
flags |= IR_FLAG_INACCESSIBLEMEM_OR_ARGMEM
7477
end
@@ -331,7 +334,8 @@ function stmt_effect_flags(𝕃ₒ::AbstractLattice, @nospecialize(stmt), @nospe
331334
consistent = is_consistent(effects)
332335
effect_free = is_effect_free(effects)
333336
nothrow = is_nothrow(effects)
334-
removable = effect_free & nothrow
337+
terminates = is_terminates(effects)
338+
removable = effect_free & nothrow & terminates
335339
return (consistent, removable, nothrow)
336340
elseif head === :new
337341
return new_expr_effect_flags(𝕃ₒ, args, src)
@@ -342,7 +346,8 @@ function stmt_effect_flags(𝕃ₒ::AbstractLattice, @nospecialize(stmt), @nospe
342346
consistent = is_consistent(effects)
343347
effect_free = is_effect_free(effects)
344348
nothrow = is_nothrow(effects)
345-
removable = effect_free & nothrow
349+
terminates = is_terminates(effects)
350+
removable = effect_free & nothrow & terminates
346351
return (consistent, removable, nothrow)
347352
elseif head === :new_opaque_closure
348353
length(args) < 4 && return (false, false, false)

base/dict.jl

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -868,34 +868,25 @@ struct PersistentDict{K,V} <: AbstractDict{K,V}
868868
@noinline function KeyValue.set(::Type{PersistentDict{K, V}}, ::Nothing, key, val) where {K, V}
869869
new{K, V}(HAMT.HAMT{K, V}(key => val))
870870
end
871-
@noinline @Base.assume_effects :effect_free function KeyValue.set(dict::PersistentDict{K, V}, key, val) where {K, V}
871+
@noinline Base.@assume_effects :effect_free :terminates_globally KeyValue.set(
872+
dict::PersistentDict{K, V}, key, val) where {K, V} = @inline _keyvalueset(dict, key, val)
873+
@noinline Base.@assume_effects :nothrow :effect_free :terminates_globally KeyValue.set(
874+
dict::PersistentDict{K, V}, key::K, val::V) where {K, V} = @inline _keyvalueset(dict, key, val)
875+
global function _keyvalueset(dict::PersistentDict{K, V}, key, val) where {K, V}
872876
trie = dict.trie
873877
h = HAMT.HashState(key)
874-
found, present, trie, i, bi, top, hs = HAMT.path(trie, key, h, #=persistent=# true)
878+
found, present, trie, i, bi, top, hs = HAMT.path(trie, key, h, #=persistent=#true)
875879
HAMT.insert!(found, present, trie, i, bi, hs, val)
876880
return new{K, V}(top)
877881
end
878-
@noinline @Base.assume_effects :nothrow :effect_free function KeyValue.set(dict::PersistentDict{K, V}, key::K, val::V) where {K, V}
882+
@noinline Base.@assume_effects :effect_free :terminates_globally KeyValue.set(
883+
dict::PersistentDict{K, V}, key) where {K, V} = @inline _keyvalueset(dict, key)
884+
@noinline Base.@assume_effects :nothrow :effect_free :terminates_globally KeyValue.set(
885+
dict::PersistentDict{K, V}, key::K) where {K, V} = @inline _keyvalueset(dict, key)
886+
global function _keyvalueset(dict::PersistentDict{K, V}, key) where {K, V}
879887
trie = dict.trie
880888
h = HAMT.HashState(key)
881-
found, present, trie, i, bi, top, hs = HAMT.path(trie, key, h, #=persistent=# true)
882-
HAMT.insert!(found, present, trie, i, bi, hs, val)
883-
return new{K, V}(top)
884-
end
885-
@noinline @Base.assume_effects :effect_free function KeyValue.set(dict::PersistentDict{K, V}, key) where {K, V}
886-
trie = dict.trie
887-
h = HAMT.HashState(key)
888-
found, present, trie, i, bi, top, _ = HAMT.path(trie, key, h, #=persistent=# true)
889-
if found && present
890-
deleteat!(trie.data, i)
891-
HAMT.unset!(trie, bi)
892-
end
893-
return new{K, V}(top)
894-
end
895-
@noinline @Base.assume_effects :nothrow :effect_free function KeyValue.set(dict::PersistentDict{K, V}, key::K) where {K, V}
896-
trie = dict.trie
897-
h = HAMT.HashState(key)
898-
found, present, trie, i, bi, top, _ = HAMT.path(trie, key, h, #=persistent=# true)
889+
found, present, trie, i, bi, top, _ = HAMT.path(trie, key, h, #=persistent=#true)
899890
if found && present
900891
deleteat!(trie.data, i)
901892
HAMT.unset!(trie, bi)

test/compiler/irpasses.jl

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1836,3 +1836,48 @@ let code = Any[
18361836
@test made_changes
18371837
@test (ir[Core.SSAValue(length(ir.stmts))][:flag] & Core.Compiler.IR_FLAG_REFINED) != 0
18381838
end
1839+
1840+
# JuliaLang/julia#52991: statements that may not :terminate should not be deleted
1841+
@noinline Base.@assume_effects :effect_free :nothrow function issue52991(n)
1842+
local s = 0
1843+
try
1844+
while true
1845+
yield()
1846+
if n - rand(1:10) > 0
1847+
s += 1
1848+
else
1849+
break
1850+
end
1851+
end
1852+
catch
1853+
end
1854+
return s
1855+
end
1856+
@test !Core.Compiler.is_removable_if_unused(Base.infer_effects(issue52991, (Int,)))
1857+
let src = code_typed1((Int,)) do x
1858+
issue52991(x)
1859+
nothing
1860+
end
1861+
@test count(isinvoke(:issue52991), src.code) == 1
1862+
end
1863+
let t = @async begin
1864+
issue52991(11) # this call never terminates
1865+
nothing
1866+
end
1867+
sleep(1)
1868+
if istaskdone(t)
1869+
ok = false
1870+
else
1871+
ok = true
1872+
schedule(t, InterruptException(); error=true)
1873+
end
1874+
@test ok
1875+
end
1876+
1877+
# JuliaLang/julia47664
1878+
@test !fully_eliminated() do
1879+
any(isone, Iterators.repeated(0))
1880+
end
1881+
@test !fully_eliminated() do
1882+
all(iszero, Iterators.repeated(0))
1883+
end

0 commit comments

Comments
 (0)