Skip to content

optimizer: do not delete statements that may not :terminate #52999

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 15, 2024
Merged

optimizer: do not delete statements that may not :terminate #52999

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
11315ea
optimizer: do not delete statements that may not `:terminate`
aviatesk Jan 22, 2024
61fc9ed
fix up broken test
aviatesk Jan 23, 2024
84786d6
add test cases from #47664
aviatesk Jan 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions base/compiler/optimize.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ const IR_FLAG_INACCESSIBLEMEM_OR_ARGMEM = one(UInt32) << 11
const NUM_IR_FLAGS = 12 # sync with julia.h

const IR_FLAGS_EFFECTS =
IR_FLAG_CONSISTENT | IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW | IR_FLAG_NOUB
IR_FLAG_CONSISTENT | IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW | IR_FLAG_TERMINATES | IR_FLAG_NOUB

const IR_FLAGS_REMOVABLE = IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW
const IR_FLAGS_REMOVABLE = IR_FLAG_EFFECT_FREE | IR_FLAG_NOTHROW | IR_FLAG_TERMINATES

const IR_FLAGS_NEEDS_EA = IR_FLAG_EFIIMO | IR_FLAG_INACCESSIBLEMEM_OR_ARGMEM

Expand All @@ -69,6 +69,9 @@ function flags_for_effects(effects::Effects)
if is_nothrow(effects)
flags |= IR_FLAG_NOTHROW
end
if is_terminates(effects)
flags |= IR_FLAG_TERMINATES
end
if is_inaccessiblemem_or_argmemonly(effects)
flags |= IR_FLAG_INACCESSIBLEMEM_OR_ARGMEM
end
Expand Down Expand Up @@ -331,7 +334,8 @@ function stmt_effect_flags(𝕃ₒ::AbstractLattice, @nospecialize(stmt), @nospe
consistent = is_consistent(effects)
effect_free = is_effect_free(effects)
nothrow = is_nothrow(effects)
removable = effect_free & nothrow
terminates = is_terminates(effects)
removable = effect_free & nothrow & terminates
return (consistent, removable, nothrow)
elseif head === :new
return new_expr_effect_flags(𝕃ₒ, args, src)
Expand All @@ -342,7 +346,8 @@ function stmt_effect_flags(𝕃ₒ::AbstractLattice, @nospecialize(stmt), @nospe
consistent = is_consistent(effects)
effect_free = is_effect_free(effects)
nothrow = is_nothrow(effects)
removable = effect_free & nothrow
terminates = is_terminates(effects)
removable = effect_free & nothrow & terminates
return (consistent, removable, nothrow)
elseif head === :new_opaque_closure
length(args) < 4 && return (false, false, false)
Expand Down
33 changes: 12 additions & 21 deletions base/dict.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -868,34 +868,25 @@ struct PersistentDict{K,V} <: AbstractDict{K,V}
@noinline function KeyValue.set(::Type{PersistentDict{K, V}}, ::Nothing, key, val) where {K, V}
new{K, V}(HAMT.HAMT{K, V}(key => val))
end
@noinline @Base.assume_effects :effect_free function KeyValue.set(dict::PersistentDict{K, V}, key, val) where {K, V}
@noinline Base.@assume_effects :effect_free :terminates_globally KeyValue.set(
dict::PersistentDict{K, V}, key, val) where {K, V} = @inline _keyvalueset(dict, key, val)
@noinline Base.@assume_effects :nothrow :effect_free :terminates_globally KeyValue.set(
dict::PersistentDict{K, V}, key::K, val::V) where {K, V} = @inline _keyvalueset(dict, key, val)
global function _keyvalueset(dict::PersistentDict{K, V}, key, val) where {K, V}
trie = dict.trie
h = HAMT.HashState(key)
found, present, trie, i, bi, top, hs = HAMT.path(trie, key, h, #=persistent=# true)
found, present, trie, i, bi, top, hs = HAMT.path(trie, key, h, #=persistent=#true)
HAMT.insert!(found, present, trie, i, bi, hs, val)
return new{K, V}(top)
end
@noinline @Base.assume_effects :nothrow :effect_free function KeyValue.set(dict::PersistentDict{K, V}, key::K, val::V) where {K, V}
@noinline Base.@assume_effects :effect_free :terminates_globally KeyValue.set(
dict::PersistentDict{K, V}, key) where {K, V} = @inline _keyvalueset(dict, key)
@noinline Base.@assume_effects :nothrow :effect_free :terminates_globally KeyValue.set(
dict::PersistentDict{K, V}, key::K) where {K, V} = @inline _keyvalueset(dict, key)
global function _keyvalueset(dict::PersistentDict{K, V}, key) where {K, V}
trie = dict.trie
h = HAMT.HashState(key)
found, present, trie, i, bi, top, hs = HAMT.path(trie, key, h, #=persistent=# true)
HAMT.insert!(found, present, trie, i, bi, hs, val)
return new{K, V}(top)
end
@noinline @Base.assume_effects :effect_free function KeyValue.set(dict::PersistentDict{K, V}, key) where {K, V}
trie = dict.trie
h = HAMT.HashState(key)
found, present, trie, i, bi, top, _ = HAMT.path(trie, key, h, #=persistent=# true)
if found && present
deleteat!(trie.data, i)
HAMT.unset!(trie, bi)
end
return new{K, V}(top)
end
@noinline @Base.assume_effects :nothrow :effect_free function KeyValue.set(dict::PersistentDict{K, V}, key::K) where {K, V}
trie = dict.trie
h = HAMT.HashState(key)
found, present, trie, i, bi, top, _ = HAMT.path(trie, key, h, #=persistent=# true)
found, present, trie, i, bi, top, _ = HAMT.path(trie, key, h, #=persistent=#true)
if found && present
deleteat!(trie.data, i)
HAMT.unset!(trie, bi)
Expand Down
45 changes: 45 additions & 0 deletions test/compiler/irpasses.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1836,3 +1836,48 @@ let code = Any[
@test made_changes
@test (ir[Core.SSAValue(length(ir.stmts))][:flag] & Core.Compiler.IR_FLAG_REFINED) != 0
end

# JuliaLang/julia#52991: statements that may not :terminate should not be deleted
@noinline Base.@assume_effects :effect_free :nothrow function issue52991(n)
local s = 0
try
while true
yield()
if n - rand(1:10) > 0
s += 1
else
break
end
end
catch
end
return s
end
@test !Core.Compiler.is_removable_if_unused(Base.infer_effects(issue52991, (Int,)))
let src = code_typed1((Int,)) do x
issue52991(x)
nothing
end
@test count(isinvoke(:issue52991), src.code) == 1
end
let t = @async begin
issue52991(11) # this call never terminates
nothing
end
sleep(1)
if istaskdone(t)
ok = false
else
ok = true
schedule(t, InterruptException(); error=true)
end
@test ok
end

# JuliaLang/julia47664
@test !fully_eliminated() do
any(isone, Iterators.repeated(0))
end
@test !fully_eliminated() do
all(iszero, Iterators.repeated(0))
end