Skip to content

Commit

Permalink
respect the current semantics of BoundsError (#77)
Browse files Browse the repository at this point in the history
  • Loading branch information
aviatesk authored Jan 13, 2022
1 parent 184afaa commit 592f39c
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 66 deletions.
54 changes: 11 additions & 43 deletions src/EscapeAnalysis.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1221,22 +1221,22 @@ end

function escape_builtin!(::typeof(arrayref), astate::AnalysisState, pc::Int, args::Vector{Any})
length(args) 4 || return false
# check potential escapes from this arrayref call
# NOTE here we essentially only need to account for TypeError, assuming that
# UndefRefError or BoundsError don't capture any of the arguments here
# check potential thrown escapes from this arrayref call
argtypes = Any[argextype(args[i], astate.ir) for i in 2:length(args)]
boundcheckt = argtypes[1]
aryt = argtypes[2]
if !array_builtin_common_typecheck(boundcheckt, aryt, argtypes, 3)
add_thrown_escapes!(astate, pc, args, 2)
end
ary = args[3]
inbounds = isa(boundcheckt, Const) && !boundcheckt.val::Bool
inbounds || add_escape_change!(astate, ary, ThrownEscape(pc))
# we don't track precise index information about this array and thus don't know what values
# can be referenced here: directly propagate the escape information imposed on the return
# value of this `arrayref` call to the array itself as the most conservative propagation
# but also with updated index information
# TODO enable index analysis when constant values are available?
estate = astate.estate
ary = args[3]
if isa(ary, SSAValue) || isa(ary, Argument)
aryinfo = estate[ary]
else
Expand All @@ -1253,21 +1253,12 @@ function escape_builtin!(::typeof(arrayref), astate::AnalysisState, pc::Int, arg
@label conservative_propagation
ssainfo = estate[ret]
add_escape_change!(astate, ary, ssainfo)
if isa(boundcheckt, Const)
if boundcheckt.val::Bool
add_escape_change!(astate, ary, ThrownEscape(pc))
end
end
elseif isa(AliasEscapes, ArrayEscapes)
# record the return value of this `arrayref` call as a possibility that imposes escape
AliasEscapes = copy(AliasEscapes)
@label record_element_escape
push!(AliasEscapes, iridx(ret, estate))
if isa(boundcheckt, Const) # record possible BoundsError at this arrayref
if boundcheckt.val::Bool
push!(AliasEscapes, SSAValue(pc))
end
end
inbounds || push!(AliasEscapes, SSAValue(pc)) # record possible BoundsError at this arrayref
add_escape_change!(astate, ary, EscapeLattice(aryinfo, AliasEscapes))
else
# this object has been used as struct, but it is used as array here (thus should throw)
Expand All @@ -1292,13 +1283,15 @@ function escape_builtin!(::typeof(arrayset), astate::AnalysisState, pc::Int, arg
arrayset_typecheck(aryt, valt))
add_thrown_escapes!(astate, pc, args, 2)
end
ary = args[3]
val = args[4]
inbounds = isa(boundcheckt, Const) && !boundcheckt.val::Bool
inbounds || add_escape_change!(astate, ary, ThrownEscape(pc))
# we don't track precise index information about this array and won't record what value
# is being assigned here: directly propagate the escape information of this array to
# the value being assigned as the most conservative propagation
# TODO enable index analysis when constant values are available?
estate = astate.estate
ary = args[3]
val = args[4]
if isa(ary, SSAValue) || isa(ary, Argument)
aryinfo = estate[ary]
else
Expand Down Expand Up @@ -1387,33 +1380,8 @@ function escape_array_resize!(boundserror::Bool, ninds::Int,
indt ₜ Integer || return add_thrown_escapes!(astate, pc, args)
end
if boundserror
if isa(ary, SSAValue) || isa(ary, Argument)
estate = astate.estate
aryinfo = estate[ary]
AliasEscapes = aryinfo.AliasEscapes
if isa(AliasEscapes, Bool)
if !AliasEscapes
# the elements of this array haven't been analyzed yet: set ArrayEscapes now
AliasEscapes = ArrayEscapes()
@goto record_element_escape
end
@label conservative_propagation
# array resizing can potentially throw `BoundsError`, impose it now
add_escape_change!(astate, ary, ThrownEscape(pc))
elseif isa(AliasEscapes, ArrayEscapes)
AliasEscapes = copy(AliasEscapes)
@label record_element_escape
# array resizing can potentially throw `BoundsError`, record it now
push!(AliasEscapes, SSAValue(pc))
add_escape_change!(astate, ary, EscapeLattice(aryinfo, AliasEscapes))
else
# this object has been used as struct, but it is used as array here (thus should throw)
# update ary's element information and just handle this case conservatively
@assert isa(AliasEscapes, FieldEscapes)
aryinfo = escape_unanalyzable_obj!(astate, ary, aryinfo)
@goto conservative_propagation
end
end
# this array resizing can potentially throw `BoundsError`, impose it now
add_escape_change!(astate, ary, ThrownEscape(pc))
end
end

Expand Down
86 changes: 63 additions & 23 deletions test/EscapeAnalysis.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1304,14 +1304,34 @@ let result = @code_escapes compute!(MPoint(1+.5im, 2+.5im), MPoint(2+.25im, 4+.7
end

@testset "array primitives" begin
inbounds = Base.JLOptions().check_bounds == 0

# arrayref
let result = code_escapes((Vector{String},Int)) do xs, i
s = Base.arrayref(true, xs, i)
return s
end
r = only(findall(isreturn, result.ir.stmts.inst))
@test has_return_escape(result.state[Argument(2)], r) # xs
@test_broken !has_thrown_escape(result.state[Argument(2)]) # xs
@test has_thrown_escape(result.state[Argument(2)]) # xs
@test !has_return_escape(result.state[Argument(3)], r) # i
end
let result = code_escapes((Vector{String},Int)) do xs, i
s = Base.arrayref(false, xs, i)
return s
end
r = only(findall(isreturn, result.ir.stmts.inst))
@test has_return_escape(result.state[Argument(2)], r) # xs
@test !has_thrown_escape(result.state[Argument(2)]) # xs
@test !has_return_escape(result.state[Argument(3)], r) # i
end
inbounds && let result = code_escapes((Vector{String},Int)) do xs, i
s = @inbounds xs[i]
return s
end
r = only(findall(isreturn, result.ir.stmts.inst))
@test has_return_escape(result.state[Argument(2)], r) # xs
@test !has_thrown_escape(result.state[Argument(2)]) # xs
@test !has_return_escape(result.state[Argument(3)], r) # i
end
let result = code_escapes((Vector{String},Bool)) do xs, i
Expand Down Expand Up @@ -1350,6 +1370,25 @@ end
end
r = only(findall(isreturn, result.ir.stmts.inst))
@test has_return_escape(result.state[Argument(2)], r) # xs
@test has_thrown_escape(result.state[Argument(2)]) # xs
@test has_return_escape(result.state[Argument(3)], r) # x
end
let result = code_escapes((Vector{String},String,Int,)) do xs, x, i
Base.arrayset(false, xs, x, i)
return xs
end
r = only(findall(isreturn, result.ir.stmts.inst))
@test has_return_escape(result.state[Argument(2)], r) # xs
@test !has_thrown_escape(result.state[Argument(2)]) # xs
@test has_return_escape(result.state[Argument(3)], r) # x
end
inbounds && let result = code_escapes((Vector{String},String,Int,)) do xs, x, i
@inbounds xs[i] = x
return xs
end
r = only(findall(isreturn, result.ir.stmts.inst))
@test has_return_escape(result.state[Argument(2)], r) # xs
@test !has_thrown_escape(result.state[Argument(2)]) # xs
@test has_return_escape(result.state[Argument(3)], r) # x
end
let result = code_escapes((String,String,String,)) do s, t, u
Expand Down Expand Up @@ -1398,14 +1437,6 @@ end
@test has_thrown_escape(result.state[Argument(2)], t) # xs
@test has_thrown_escape(result.state[Argument(3)], t) # x
end
let result = code_escapes((Vector{Any},AbstractString,Int,)) do xs, x, i
Base.arrayset(true, xs, x, i) # TypeError may happen here
return xs
end
t = only(findall(iscall((result.ir, Base.arrayset)), result.ir.stmts.inst))
@test !has_thrown_escape(result.state[Argument(2)], t) # xs
@test !has_thrown_escape(result.state[Argument(3)], t) # x
end

# arrayref and arrayset
let result = code_escapes() do
Expand Down Expand Up @@ -1520,7 +1551,7 @@ end
end
i = only(findall(isarrayalloc, result.ir.stmts.inst))
t = only(findall(isarrayresize, result.ir.stmts.inst))
@test !has_thrown_escape(result.state[SSAValue(i)], t) # xs
@test has_thrown_escape(result.state[SSAValue(i)], t) # xs
@test has_thrown_escape(result.state[Argument(2)], t) # x
end
let result = code_escapes((String,)) do x
Expand All @@ -1531,7 +1562,7 @@ end
end
i = only(findall(isarrayalloc, result.ir.stmts.inst))
t = only(findall(isarrayresize, result.ir.stmts.inst))
@test !has_thrown_escape(result.state[SSAValue(i)], t) # xs
@test has_thrown_escape(result.state[SSAValue(i)], t) # xs
@test has_thrown_escape(result.state[Argument(2)], t) # x
end
let result = code_escapes((String,)) do x
Expand All @@ -1540,7 +1571,7 @@ end
end
i = only(findall(isarrayalloc, result.ir.stmts.inst))
t = only(findall(isarrayresize, result.ir.stmts.inst))
@test !has_thrown_escape(result.state[SSAValue(i)], t) # xs
@test has_thrown_escape(result.state[SSAValue(i)], t) # xs
@test has_thrown_escape(result.state[Argument(2)], t) # x
end
let result = code_escapes((String,)) do x
Expand All @@ -1549,7 +1580,16 @@ end
end
i = only(findall(isarrayalloc, result.ir.stmts.inst))
t = only(findall(isarrayresize, result.ir.stmts.inst))
@test !has_thrown_escape(result.state[SSAValue(i)], t) # xs
@test has_thrown_escape(result.state[SSAValue(i)], t) # xs
@test has_thrown_escape(result.state[Argument(2)], t) # x
end
inbounds && let result = code_escapes((String,)) do x
xs = @inbounds Any[x]
@ccall jl_array_del_at(xs::Any, 1::UInt, 2::UInt)::Cvoid # can potentially throw
end
i = only(findall(isarrayalloc, result.ir.stmts.inst))
t = only(findall(isarrayresize, result.ir.stmts.inst))
@test has_thrown_escape(result.state[SSAValue(i)], t) # xs
@test has_thrown_escape(result.state[Argument(2)], t) # x
end

Expand Down Expand Up @@ -1626,9 +1666,9 @@ let result = code_escapes((Int,String,)) do n,s
i = only(findall(isarrayalloc, result.ir.stmts.inst))
r = only(findall(isreturn, result.ir.stmts.inst))
@test has_return_escape(result.state[SSAValue(i)], r)
@test !has_thrown_escape(result.state[SSAValue(i)])
@test has_thrown_escape(result.state[SSAValue(i)])
@test has_return_escape(result.state[Argument(3)], r) # s
@test !has_thrown_escape(result.state[Argument(3)]) # s
@test has_thrown_escape(result.state[Argument(3)]) # s
end
let result = code_escapes((Int,String,)) do n,s
xs = String[]
Expand All @@ -1639,10 +1679,10 @@ let result = code_escapes((Int,String,)) do n,s
end
i = only(findall(isarrayalloc, result.ir.stmts.inst))
r = only(findall(isreturn, result.ir.stmts.inst))
@test has_return_escape(result.state[SSAValue(i)], r)
@test !has_thrown_escape(result.state[SSAValue(i)])
@test has_return_escape(result.state[SSAValue(i)], r) # xs
@test has_thrown_escape(result.state[SSAValue(i)]) # xs
@test has_return_escape(result.state[Argument(3)], r) # s
@test !has_thrown_escape(result.state[Argument(3)]) # s
@test has_thrown_escape(result.state[Argument(3)]) # s
end
let result = code_escapes((String,String,String)) do s, t, u
xs = String[]
Expand All @@ -1655,7 +1695,7 @@ let result = code_escapes((String,String,String)) do s, t, u
i = only(findall(isarrayalloc, result.ir.stmts.inst))
r = only(findall(isreturn, result.ir.stmts.inst))
@test has_return_escape(result.state[SSAValue(i)], r)
@test !has_thrown_escape(result.state[SSAValue(i)])
@test has_thrown_escape(result.state[SSAValue(i)]) # xs
@test has_return_escape(result.state[Argument(2)], r) # s
@test has_return_escape(result.state[Argument(3)], r) # t
@test has_return_escape(result.state[Argument(4)], r) # u
Expand Down Expand Up @@ -1755,20 +1795,20 @@ import Core: ImmutableArray, arrayfreeze, mutating_arrayfreeze, arraythaw
end

# demonstrate some arrayfreeze optimizations
# has_no_escape(ary) means ary is eligible for arrayfreeze to mutating_arrayfreeze optimization
# !has_return_escape(ary) means ary is eligible for arrayfreeze to mutating_arrayfreeze optimization
let result = code_escapes((Int,)) do n
xs = collect(1:n)
ImmutableArray(xs)
end
i = only(findall(isarrayalloc, result.ir.stmts.inst))
@test has_no_escape(result.state[SSAValue(i)])
@test !has_return_escape(result.state[SSAValue(i)])
end
let result = code_escapes((Vector{Float64},)) do xs
ys = sin.(xs)
ImmutableArray(ys)
end
i = only(findall(isarrayalloc, result.ir.stmts.inst))
@test_broken has_no_escape(result.state[SSAValue(i)])
@test !has_return_escape(result.state[SSAValue(i)])
end
let result = code_escapes((Vector{Pair{Int,String}},)) do xs
n = maximum(first, xs)
Expand All @@ -1779,7 +1819,7 @@ let result = code_escapes((Vector{Pair{Int,String}},)) do xs
ImmutableArray(xs)
end
i = only(findall(isarrayalloc, result.ir.stmts.inst))
@test has_no_escape(result.state[SSAValue(i)])
@test !has_return_escape(result.state[SSAValue(i)])
end

end # @static if isdefined(Core, :ImmutableArray)
Expand Down

0 comments on commit 592f39c

Please sign in to comment.