Skip to content

Commit 29681b6

Browse files
Ian AtolIan Atol
authored andcommitted
Implement maybecopy in BoundsError, start optimization, refactor memory_opt!
1 parent fcbafaa commit 29681b6

File tree

12 files changed

+685
-42
lines changed

12 files changed

+685
-42
lines changed

base/abstractarray.jl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,6 +1073,10 @@ function copy(a::AbstractArray)
10731073
copymutable(a)
10741074
end
10751075

1076+
function copy(a::Core.ImmutableArray)
1077+
a
1078+
end
1079+
10761080
function copyto!(B::AbstractVecOrMat{R}, ir_dest::AbstractRange{Int}, jr_dest::AbstractRange{Int},
10771081
A::AbstractVecOrMat{S}, ir_src::AbstractRange{Int}, jr_src::AbstractRange{Int}) where {R,S}
10781082
if length(ir_dest) != length(ir_src)

base/array.jl

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,35 @@ Union type of [`DenseVector{T}`](@ref) and [`DenseMatrix{T}`](@ref).
118118
"""
119119
const DenseVecOrMat{T} = Union{DenseVector{T}, DenseMatrix{T}}
120120

121+
"""
122+
ImmutableArray
123+
124+
Dynamically allocated, immutable array.
125+
126+
"""
127+
const ImmutableArray = Core.ImmutableArray
128+
129+
"""
130+
IMArray{T,N}
131+
132+
Union type of [`Array{T,N}`](@ref) and [`ImmutableArray{T,N}`](@ref)
133+
"""
134+
const IMArray{T,N} = Union{Array{T, N}, ImmutableArray{T,N}}
135+
136+
"""
137+
IMVector{T}
138+
139+
One-dimensional [`ImmutableArray`](@ref) or [`Array`](@ref) with elements of type `T`. Alias for `IMArray{T, 1}`.
140+
"""
141+
const IMVector{T} = IMArray{T, 1}
142+
143+
"""
144+
IMMatrix{T}
145+
146+
Two-dimensional [`ImmutableArray`](@ref) or [`Array`](@ref) with elements of type `T`. Alias for `IMArray{T,2}`.
147+
"""
148+
const IMMatrix{T} = IMArray{T, 2}
149+
121150
## Basic functions ##
122151

123152
import Core: arraysize, arrayset, arrayref, const_arrayref
@@ -147,14 +176,13 @@ function vect(X...)
147176
return copyto!(Vector{T}(undef, length(X)), X)
148177
end
149178

150-
const ImmutableArray = Core.ImmutableArray
151-
const IMArray{T,N} = Union{Array{T, N}, ImmutableArray{T,N}}
152-
const IMVector{T} = IMArray{T, 1}
153-
const IMMatrix{T} = IMArray{T, 2}
154-
179+
# Freeze and thaw constructors
155180
ImmutableArray(a::Array) = Core.arrayfreeze(a)
156181
Array(a::ImmutableArray) = Core.arraythaw(a)
157182

183+
ImmutableArray(a::AbstractArray{T,N}) where {T,N} = ImmutableArray{T,N}(a)
184+
185+
# Size functions for arrays, both mutable and immutable
158186
size(a::IMArray, d::Integer) = arraysize(a, convert(Int, d))
159187
size(a::IMVector) = (arraysize(a,1),)
160188
size(a::IMMatrix) = (arraysize(a,1), arraysize(a,2))
@@ -393,6 +421,18 @@ similar(a::Array{T}, m::Int) where {T} = Vector{T}(undef, m)
393421
similar(a::Array, T::Type, dims::Dims{N}) where {N} = Array{T,N}(undef, dims)
394422
similar(a::Array{T}, dims::Dims{N}) where {T,N} = Array{T,N}(undef, dims)
395423

424+
ImmutableArray{T}(undef::UndefInitializer, m::Int) where T = ImmutableArray(Array{T}(undef, m))
425+
ImmutableArray{T}(undef::UndefInitializer, dims::Dims) where T = ImmutableArray(Array{T}(undef, dims))
426+
427+
"""
428+
maybecopy(x)
429+
430+
`maybecopy` provides access to `x` while ensuring it does not escape.
431+
To do so, the optimizer decides whether to create a copy of `x` or not based on the implementation
432+
That is, `maybecopy` will either be a call to [`copy`](@ref) or just a reference to x.
433+
"""
434+
maybecopy = Core.maybecopy
435+
396436
# T[x...] constructs Array{T,1}
397437
"""
398438
getindex(type[, elements...])
@@ -626,8 +666,8 @@ oneunit(x::AbstractMatrix{T}) where {T} = _one(oneunit(T), x)
626666

627667
## Conversions ##
628668

629-
convert(::Type{T}, a::AbstractArray) where {T<:Array} = a isa T ? a : T(a)
630669
convert(::Type{Union{}}, a::AbstractArray) = throw(MethodError(convert, (Union{}, a)))
670+
convert(T::Union{Type{<:Array},Type{<:Core.ImmutableArray}}, a::AbstractArray) = a isa T ? a : T(a)
631671

632672
promote_rule(a::Type{Array{T,n}}, b::Type{Array{S,n}}) where {T,n,S} = el_same(promote_type(T,S), a, b)
633673

@@ -637,6 +677,7 @@ if nameof(@__MODULE__) === :Base # avoid method overwrite
637677
# constructors should make copies
638678
Array{T,N}(x::AbstractArray{S,N}) where {T,N,S} = copyto_axcheck!(Array{T,N}(undef, size(x)), x)
639679
AbstractArray{T,N}(A::AbstractArray{S,N}) where {T,N,S} = copyto_axcheck!(similar(A,T), A)
680+
ImmutableArray{T,N}(Ar::AbstractArray{S,N}) where {T,N,S} = Core.arrayfreeze(copyto_axcheck!(Array{T,N}(undef, size(Ar)), Ar))
640681
end
641682

642683
## copying iterators to containers

base/boot.jl

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -279,8 +279,17 @@ struct BoundsError <: Exception
279279
a::Any
280280
i::Any
281281
BoundsError() = new()
282-
BoundsError(@nospecialize(a)) = (@_noinline_meta; new(a))
283-
BoundsError(@nospecialize(a), i) = (@_noinline_meta; new(a,i))
282+
# For now, always copy arrays to avoid escaping them
283+
# Eventually, we want to figure out if the copy is needed to save the performance of copying
284+
# (i.e., if a escapes elsewhere, don't bother to make a copy)
285+
286+
BoundsError(@nospecialize(a)) = a isa Array ?
287+
(@_noinline_meta; new(Core.maybecopy(a))) :
288+
(@_noinline_meta; new(a))
289+
290+
BoundsError(@nospecialize(a), i) = a isa Array ?
291+
(@_noinline_meta; new(Core.maybecopy(a), i)) :
292+
(@_noinline_meta; new(a, i))
284293
end
285294
struct DivideError <: Exception end
286295
struct OutOfMemoryError <: Exception end

base/broadcast.jl

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1385,4 +1385,17 @@ function Base.show(io::IO, op::BroadcastFunction)
13851385
end
13861386
Base.show(io::IO, ::MIME"text/plain", op::BroadcastFunction) = show(io, op)
13871387

1388+
struct IMArrayStyle <: Broadcast.AbstractArrayStyle{Any} end
1389+
BroadcastStyle(::Type{<:Core.ImmutableArray}) = IMArrayStyle()
1390+
1391+
#similar has to return mutable array
1392+
function Base.similar(bc::Broadcasted{IMArrayStyle}, ::Type{ElType}) where ElType
1393+
similar(Array{ElType}, axes(bc))
1394+
end
1395+
1396+
@inline function copy(bc::Broadcasted{IMArrayStyle})
1397+
ElType = combine_eltypes(bc.f, bc.args)
1398+
return Core.ImmutableArray(copyto!(similar(bc, ElType), bc))
1399+
end
1400+
13881401
end # module

base/compiler/optimize.jl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ function run_passes(ci::CodeInfo, sv::OptimizationState)
307307
ir = adce_pass!(ir)
308308
#@Base.show ("after_adce", ir)
309309
@timeit "type lift" ir = type_lift_pass!(ir)
310+
#@timeit "compact 3" ir = compact!(ir)
310311
ir = memory_opt!(ir)
311312
#@Base.show ir
312313
if JLOptions().debug_level == 2

base/compiler/ssair/passes.jl

Lines changed: 135 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1256,76 +1256,184 @@ function cfg_simplify!(ir::IRCode)
12561256
return finish(compact)
12571257
end
12581258

1259-
function is_allocation(stmt)
1259+
# function is_known_fcall(stmt::Expr, @nospecialize(func))
1260+
# isexpr(stmt, :foreigncall) || return false
1261+
# s = stmt.args[1]
1262+
# isa(s, QuoteNode) && (s = s.value)
1263+
# return s === func
1264+
# end
1265+
1266+
function is_known_fcall(stmt::Expr, funcs::Vector{Symbol})
12601267
isexpr(stmt, :foreigncall) || return false
12611268
s = stmt.args[1]
12621269
isa(s, QuoteNode) && (s = s.value)
1263-
return s === :jl_alloc_array_1d
1270+
# return any(e -> s === e, funcs)
1271+
return true in map(e -> s === e, funcs)
1272+
end
1273+
1274+
function is_allocation(stmt::Expr)
1275+
isexpr(stmt, :foreigncall) || return false
1276+
s = stmt.args[1]
1277+
isa(s, QuoteNode) && (s = s.value)
1278+
return (s === :jl_alloc_array_1d
1279+
|| s === :jl_alloc_array_2d
1280+
|| s === :jl_alloc_array_3d
1281+
|| s === :jl_new_array)
12641282
end
12651283

12661284
function memory_opt!(ir::IRCode)
12671285
compact = IncrementalCompact(ir, false)
12681286
uses = IdDict{Int, Vector{Int}}()
1269-
relevant = IdSet{Int}()
1270-
revisit = Int[]
1271-
function mark_val(val)
1287+
relevant = IdSet{Int}() # allocations
1288+
revisit = Int[] # potential targets for a mutating_arrayfreeze drop-in
1289+
maybecopies = Int[] # calls to maybecopy
1290+
1291+
function mark_escape(@nospecialize val)
12721292
isa(val, SSAValue) || return
1293+
#println(val.id, " escaped.")
12731294
val.id in relevant && pop!(relevant, val.id)
12741295
end
1296+
1297+
function mark_use(val, idx)
1298+
isa(val, SSAValue) || return
1299+
id = val.id
1300+
id in relevant || return
1301+
(haskey(uses, id)) || (uses[id] = Int[])
1302+
push!(uses[id], idx)
1303+
end
1304+
12751305
for ((_, idx), stmt) in compact
1306+
1307+
#println("idx: ", idx, " = ", stmt)
1308+
12761309
if isa(stmt, ReturnNode)
12771310
isdefined(stmt, :val) || continue
12781311
val = stmt.val
1279-
if isa(val, SSAValue) && val.id in relevant
1280-
(haskey(uses, val.id)) || (uses[val.id] = Int[])
1281-
push!(uses[val.id], idx)
1282-
end
1312+
mark_use(val, idx)
12831313
continue
1314+
1315+
# check for phinodes that are possibly allocations
1316+
elseif isa(stmt, PhiNode)
1317+
1318+
# ensure all of the phinode values are defined
1319+
defined = true
1320+
for i = 1:length(stmt.values)
1321+
if !isassigned(stmt.values, i)
1322+
defined = false
1323+
end
1324+
end
1325+
1326+
defined || continue
1327+
1328+
for val in stmt.values
1329+
if isa(val, SSAValue) && val.id in relevant
1330+
push!(relevant, idx)
1331+
end
1332+
end
12841333
end
1334+
12851335
(isexpr(stmt, :call) || isexpr(stmt, :foreigncall)) || continue
1336+
1337+
if is_known_call(stmt, Core.maybecopy, compact)
1338+
push!(maybecopies, idx)
1339+
continue
1340+
end
1341+
12861342
if is_allocation(stmt)
12871343
push!(relevant, idx)
12881344
# TODO: Mark everything else here
12891345
continue
12901346
end
1291-
# TODO: Replace this by interprocedural escape analysis
1292-
if is_known_call(stmt, arrayset, compact)
1347+
1348+
if is_known_call(stmt, arrayset, compact) && length(stmt.args) >= 5
12931349
# The value being set escapes, everything else doesn't
1294-
mark_val(stmt.args[4])
1350+
mark_escape(stmt.args[4])
12951351
arr = stmt.args[3]
1296-
if isa(arr, SSAValue) && arr.id in relevant
1297-
(haskey(uses, arr.id)) || (uses[arr.id] = Int[])
1298-
push!(uses[arr.id], idx)
1299-
end
1352+
mark_use(arr, idx)
1353+
1354+
elseif is_known_call(stmt, arrayref, compact) && length(stmt.args) == 4
1355+
arr = stmt.args[3]
1356+
mark_use(arr, idx)
1357+
1358+
elseif is_known_call(stmt, setindex!, compact) && length(stmt.args) == 4
1359+
# handle similarly to arrayset
1360+
val = stmt.args[3]
1361+
mark_escape(val)
1362+
1363+
arr = stmt.args[2]
1364+
mark_use(arr, idx)
1365+
1366+
elseif is_known_call(stmt, (===), compact) && length(stmt.args) == 3
1367+
arr1 = stmt.args[2]
1368+
arr2 = stmt.args[3]
1369+
1370+
mark_use(arr1, idx)
1371+
mark_use(arr2, idx)
1372+
1373+
# these foreigncalls have similar structure and don't escape our array, so handle them all at once
1374+
elseif is_known_fcall(stmt, [:jl_array_ptr, :jl_array_copy]) && length(stmt.args) == 6
1375+
arr = stmt.args[6]
1376+
mark_use(arr, idx)
1377+
1378+
elseif is_known_call(stmt, arraysize, compact) && isa(stmt.args[2], SSAValue)
1379+
arr = stmt.args[2]
1380+
mark_use(arr, idx)
1381+
13001382
elseif is_known_call(stmt, Core.arrayfreeze, compact) && isa(stmt.args[2], SSAValue)
1383+
# mark these for potential replacement with mutating_arrayfreeze
13011384
push!(revisit, idx)
1385+
13021386
else
1303-
# For now we assume everything escapes
1304-
# TODO: We could handle PhiNodes specially and improve this
1387+
# Assume everything else escapes
13051388
for ur in userefs(stmt)
1306-
mark_val(ur[])
1389+
mark_escape(ur[])
13071390
end
13081391
end
13091392
end
1393+
13101394
ir = finish(compact)
1311-
isempty(revisit) && return ir
1395+
isempty(revisit) && isempty(maybecopies) && return ir
1396+
13121397
domtree = construct_domtree(ir.cfg.blocks)
1398+
13131399
for idx in revisit
13141400
# Make sure that the value we reference didn't escape
1315-
id = ir.stmts[idx][:inst].args[2].id
1401+
stmt = ir.stmts[idx][:inst]::Expr
1402+
id = (stmt.args[2]::SSAValue).id
13161403
(id in relevant) || continue
13171404

1405+
#println("Revisiting ", stmt)
1406+
13181407
# We're ok to steal the memory if we don't dominate any uses
13191408
ok = true
1320-
for use in uses[id]
1321-
if ssadominates(ir, domtree, idx, use)
1322-
ok = false
1323-
break
1409+
if haskey(uses, id)
1410+
for use in uses[id]
1411+
if ssadominates(ir, domtree, idx, use)
1412+
ok = false
1413+
break
1414+
end
13241415
end
13251416
end
13261417
ok || continue
1327-
1328-
ir.stmts[idx][:inst].args[1] = Core.mutating_arrayfreeze
1418+
stmt.args[1] = Core.mutating_arrayfreeze
13291419
end
1420+
1421+
# TODO: Use escape analysis info to determine if maybecopy should copy
1422+
1423+
# for idx in maybecopies
1424+
# stmt = ir.stmts[idx][:inst]::Expr
1425+
# #println(stmt.args)
1426+
# arr = stmt.args[2]
1427+
# id = isa(arr, SSAValue) ? arr.id : arr.n # SSAValue or Core.Argument
1428+
1429+
# if (id in relevant) # didn't escape elsewhere, so make a copy to keep it un-escaped
1430+
# #println("didn't escape maybecopy")
1431+
# stmt.args[1] = Main.Base.copy
1432+
# else # already escaped, so save the cost of copying and just pass the actual object
1433+
# #println("escaped maybecopy")
1434+
# ir.stmts[idx][:inst] = arr
1435+
# end
1436+
# end
1437+
13301438
return ir
13311439
end

base/pointer.jl

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ cconvert(::Type{Ptr{UInt8}}, s::AbstractString) = String(s)
6363
cconvert(::Type{Ptr{Int8}}, s::AbstractString) = String(s)
6464

6565
unsafe_convert(::Type{Ptr{T}}, a::Array{T}) where {T} = ccall(:jl_array_ptr, Ptr{T}, (Any,), a)
66+
unsafe_convert(::Type{Ptr{T}}, a::Core.ImmutableArray{T}) where {T} = ccall(:jl_array_ptr, Ptr{T}, (Any,), a)
6667
unsafe_convert(::Type{Ptr{S}}, a::AbstractArray{T}) where {S,T} = convert(Ptr{S}, unsafe_convert(Ptr{T}, a))
6768
unsafe_convert(::Type{Ptr{T}}, a::AbstractArray{T}) where {T} = error("conversion to pointer not defined for $(typeof(a))")
6869

src/builtin_proto.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ DECLARE_BUILTIN(_typevar);
5454
DECLARE_BUILTIN(arrayfreeze);
5555
DECLARE_BUILTIN(arraythaw);
5656
DECLARE_BUILTIN(mutating_arrayfreeze);
57+
DECLARE_BUILTIN(maybecopy);
5758

5859
JL_CALLABLE(jl_f_invoke_kwsorter);
5960
JL_CALLABLE(jl_f__structtype);

0 commit comments

Comments
 (0)