Skip to content

Commit

Permalink
fix #6, escape (possibly) thrown arguments
Browse files Browse the repository at this point in the history 
Currently this PR just re-uses ["effect-freeness" of a statement](https://github.com/JuliaLang/julia/blob/94b9d66b10e8e3ebdb268e4be5f7e1f43079ad4e/base/compiler/ssair/queries.jl#L6)
computed by inliner.

This is necessary but we lose much analysis accuracy, e.g. now we can't
recognize the target below:
```julia
julia> @noinline f_noescape(x) = (broadcast(identity, x); nothing);

julia> analyze_escapes() do
           f_noescape(Ref("Hi"))
       end
var"#3#4"()
1 → 1 ─ %1 = %new(Base.RefValue{String}, "Hi")::Base.RefValue{String}
2 ↑ │   %2 = invoke Main.f_noescape(%1::Base.RefValue{String})::Core.Const(nothing)
3 ◌ └──      return %2
```

This case is particularly because Julia compiler doesn't tell
`getfield(::Base.RefValue, :x)` is "effect-free" since it's caught by
[this `field <= datatype_min_ninitialized(s)` check](https://github.com/JuliaLang/julia/blob/94b9d66b10e8e3ebdb268e4be5f7e1f43079ad4e/base/compiler/tfuncs.jl#L745).
We may need to improve inference instead rather than doing something in
this pass.
  • Loading branch information
@aviatesk
aviatesk committed Jun 30, 2021
1 parent dc4e8a5 commit 28c81b0
Show file tree
Hide file tree
Showing 2 changed files with 86 additions and 46 deletions.
65 changes: 38 additions & 27 deletions src/EscapeAnalysis.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ import .CC:
IRCode,
optimize,
widenconst,
argextype
argextype,
IR_FLAG_EFFECT_FREE

import Base.Meta:
isexpr
Expand Down Expand Up @@ -207,39 +208,49 @@ function find_escapes(ir::IRCode, nargs::Int)
for pc in nstmts:-1:1
stmt = stmts.inst[pc]

# inlinear already computed effect-freeness of this statement (whether it may throw or not)
# and if it may throw, we conservatively escape all the arguments
is_effect_free = stmts.flag[pc] & IR_FLAG_EFFECT_FREE ≠ 0

# collect escape information
if isa(stmt, Expr)
head = stmt.head
if head === :call
ft = widenconst(argextype(first(stmt.args), ir, sptypes, argtypes))
# TODO implement more builtins, make them more accurate
if ft === Core.IntrinsicFunction # XXX we may break soundness here, e.g. `pointerref`
continue
elseif ft === typeof(isa) || ft === typeof(typeof) || ft === typeof(Core.sizeof)
continue
elseif ft === typeof(ifelse) && length(stmt.args) === 4
f, cond, th, el = stmt.args
info = state.ssavalues[pc]
condt = argextype(cond, ir, sptypes, argtypes)
if isa(condt, Const) && isa(condt.val, Bool)
if condt.val
push!(changes, (th, info))
if head === :call # TODO implement more builtins, make them more accurate
if !is_effect_free
# TODO we can have a look at builtins.c and limit the escaped arguments if any of them are not thrown
for arg in stmt.args[2:end]
push!(changes, (arg, Escape()))
end
else
ft = widenconst(argextype(first(stmt.args), ir, sptypes, argtypes))
if ft === Core.IntrinsicFunction # XXX we may break soundness here, e.g. `pointerref`
continue
elseif ft === typeof(isa) || ft === typeof(typeof) || ft === typeof(Core.sizeof)
continue
elseif ft === typeof(ifelse) && length(stmt.args) === 4
f, cond, th, el = stmt.args
info = state.ssavalues[pc]
condt = argextype(cond, ir, sptypes, argtypes)
if isa(condt, Const) && isa(condt.val, Bool)
if condt.val
push!(changes, (th, info))
else
push!(changes, (el, info))
end
else
push!(changes, (th, info))
push!(changes, (el, info))
end
elseif ft === typeof(getfield) || ft === typeof(tuple)
info = state.ssavalues[pc]
info === NoInformation() && (info = NoEscape())
for arg in stmt.args[2:end]
push!(changes, (arg, info))
end
else
push!(changes, (th, info))
push!(changes, (el, info))
end
elseif ft === typeof(getfield) || ft === typeof(tuple)
info = state.ssavalues[pc]
info === NoInformation() && (info = NoEscape())
for arg in stmt.args[2:end]
push!(changes, (arg, info))
end
else
for arg in stmt.args[2:end]
push!(changes, (arg, Escape()))
for arg in stmt.args[2:end]
push!(changes, (arg, Escape()))
end
end
end
elseif head === :invoke
Expand Down
67 changes: 48 additions & 19 deletions test/runtests.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import EscapeAnalysis:
for t in subtypes(EscapeInformation)
canonicalname = Symbol(parentmodule(t), '.', nameof(t))
canonicalpath = Symbol.(split(string(canonicalname), '.'))

modpath = Expr(:., canonicalpath[1:end-1]...)
symname = Expr(:., last(canonicalpath))
ex = Expr(:import, Expr(:(:), modpath, symname))
Expand All @@ -13,26 +12,33 @@ end

@testset "EscapeAnalysis" begin

let # simplest
src, escapes = analyze_escapes((Any,)) do a # return to caller
return nothing
mutable struct MutableSome{T}
value::T
end

@testset "basics" begin
let # simplest
src, escapes = analyze_escapes((Any,)) do a # return to caller
return nothing
end
@test escapes.arguments[2] isa ReturnEscape
end
@test escapes.arguments[2] isa ReturnEscape
end

let # global assignement
src, escapes = analyze_escapes((Any,)) do a
global aa = a
return nothing
let # global assignement
src, escapes = analyze_escapes((Any,)) do a
global aa = a
return nothing
end
@test escapes.arguments[2] isa Escape
end
@test escapes.arguments[2] isa Escape
end

let # return
src, escapes = analyze_escapes((Any,)) do a
return a
let # return
src, escapes = analyze_escapes((Any,)) do a
return a
end
@test escapes.arguments[2] isa ReturnEscape
end
@test escapes.arguments[2] isa ReturnEscape
end

@testset "control flows" begin
Expand Down Expand Up @@ -121,14 +127,18 @@ end
@testset "inter-procedural" begin
m = Module()

# FIXME currently we can't prove the effect-freeness of `getfield(RefValue{String}, :x)`
# because of this check https://github.com/JuliaLang/julia/blob/94b9d66b10e8e3ebdb268e4be5f7e1f43079ad4e/base/compiler/tfuncs.jl#L745
# and thus it leads to the following two broken tests

@eval m @noinline f_noescape(x) = (broadcast(identity, x); nothing)
let
src, escapes = @eval m $analyze_escapes() do
f_noescape(Ref("Hi"))
end
i = findfirst(==(Base.RefValue{String}), src.stmts.type) # find allocation statement
@assert !isnothing(i)
@test escapes.ssavalues[i] isa ReturnEscape
@test_broken escapes.ssavalues[i] isa ReturnEscape
end

@eval m @noinline f_returnescape(x) = broadcast(identity, x)
Expand All @@ -138,7 +148,7 @@ end
end
i = findfirst(==(Base.RefValue{String}), src.stmts.type) # find allocation statement
@assert !isnothing(i)
@test escapes.ssavalues[i] isa ReturnEscape
@test_broken escapes.ssavalues[i] isa ReturnEscape
end

@eval m @noinline f_escape(x) = (global xx = x) # obvious escape
Expand All @@ -153,13 +163,13 @@ end

# if we can't determine the matching method statically, we should be conservative
let
src, escapes = @eval m $analyze_escapes((Some{Any},)) do a
src, escapes = @eval m $analyze_escapes(($MutableSome{Any},)) do a
may_exist(a)
end
@test escapes.arguments[2] isa Escape
end
let
src, escapes = @eval m $analyze_escapes((Some{Any},)) do a
src, escapes = @eval m $analyze_escapes(($MutableSome{Any},)) do a
Base.@invokelatest f_noescape(a)
end
@test escapes.arguments[2] isa Escape
Expand All @@ -179,6 +189,25 @@ end
end

@testset "builtins" begin
let # throw
r = analyze_escapes((Any, )) do a
throw(a)
end
@test r.state.arguments[2] === Escape()
end

let # implicit throws -- currently relies on inliner's effect-freeness check
r = analyze_escapes((Any, )) do a
getfield(a, :may_not_field)
end
@test r.state.arguments[2] === Escape()

r = analyze_escapes((Any, )) do a
sizeof(a)
end
@test r.state.arguments[2] === Escape()
end

let # sizeof
src, escapes = analyze_escapes((Vector{Int}, )) do itr
sizeof(itr)
Expand Down

0 comments on commit 28c81b0

Please sign in to comment.