Skip to content
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.

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

elide code marked with @boundscheck(...). #14474

Merged
merged 18 commits into from
Jan 14, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
60 changes: 30 additions & 30 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ checkbounds(::Type{Bool}, sz::Integer, i) = throw(ArgumentError("unable to check
checkbounds(::Type{Bool}, sz::Integer, i::Real) = 1 <= i <= sz
checkbounds(::Type{Bool}, sz::Integer, ::Colon) = true
function checkbounds(::Type{Bool}, sz::Integer, r::Range)
@_inline_meta
@_propagate_inbounds_meta
isempty(r) || (checkbounds(Bool, sz, minimum(r)) && checkbounds(Bool, sz, maximum(r)))
end
checkbounds(::Type{Bool}, sz::Integer, I::AbstractArray{Bool}) = length(I) == sz
Expand Down Expand Up @@ -350,8 +350,8 @@ function copy!{R,S}(B::AbstractVecOrMat{R}, ir_dest::Range{Int}, jr_dest::Range{
throw(ArgumentError(string("source and destination must have same size (got ",
length(jr_src)," and ",length(jr_dest),")")))
end
checkbounds(B, ir_dest, jr_dest)
checkbounds(A, ir_src, jr_src)
@boundscheck checkbounds(B, ir_dest, jr_dest)
@boundscheck checkbounds(A, ir_src, jr_src)
jdest = first(jr_dest)
for jsrc in jr_src
idest = first(ir_dest)
Expand All @@ -374,8 +374,8 @@ function copy_transpose!{R,S}(B::AbstractVecOrMat{R}, ir_dest::Range{Int}, jr_de
throw(ArgumentError(string("source and destination must have same size (got ",
length(ir_src)," and ",length(jr_dest),")")))
end
checkbounds(B, ir_dest, jr_dest)
checkbounds(A, ir_src, jr_src)
@boundscheck checkbounds(B, ir_dest, jr_dest)
@boundscheck checkbounds(A, ir_src, jr_src)
idest = first(ir_dest)
for jsrc in jr_src
jdest = first(jr_dest)
Expand Down Expand Up @@ -476,35 +476,35 @@ pointer{T}(x::AbstractArray{T}, i::Integer) = (@_inline_meta; unsafe_convert(Ptr
# unsafe method.

function getindex(A::AbstractArray, I...)
@_inline_meta
@_propagate_inbounds_meta
_getindex(linearindexing(A), A, I...)
end
function unsafe_getindex(A::AbstractArray, I...)
@_inline_meta
@_propagate_inbounds_meta
_unsafe_getindex(linearindexing(A), A, I...)
end
## Internal defitions
# 0-dimensional indexing is defined to prevent ambiguities. LinearFast is easy:
_getindex(::LinearFast, A::AbstractArray) = (@_inline_meta; getindex(A, 1))
_getindex(::LinearFast, A::AbstractArray) = (@_propagate_inbounds_meta; getindex(A, 1))
# But LinearSlow must take into account the dimensionality of the array:
_getindex{T}(::LinearSlow, A::AbstractArray{T,0}) = error("indexing not defined for ", typeof(A))
_getindex(::LinearSlow, A::AbstractVector) = (@_inline_meta; getindex(A, 1))
_getindex(l::LinearSlow, A::AbstractArray) = (@_inline_meta; _getindex(l, A, 1))
_getindex(::LinearSlow, A::AbstractVector) = (@_propagate_inbounds_meta; getindex(A, 1))
_getindex(l::LinearSlow, A::AbstractArray) = (@_propagate_inbounds_meta; _getindex(l, A, 1))
_unsafe_getindex(::LinearFast, A::AbstractArray) = (@_inline_meta; unsafe_getindex(A, 1))
_unsafe_getindex{T}(::LinearSlow, A::AbstractArray{T,0}) = error("indexing not defined for ", typeof(A))
_unsafe_getindex(::LinearSlow, A::AbstractVector) = (@_inline_meta; unsafe_getindex(A, 1))
_unsafe_getindex(l::LinearSlow, A::AbstractArray) = (@_inline_meta; _unsafe_getindex(l, A, 1))

_getindex(::LinearIndexing, A::AbstractArray, I...) = error("indexing $(typeof(A)) with types $(typeof(I)) is not supported")
_unsafe_getindex(::LinearIndexing, A::AbstractArray, I...) = (@_inline_meta; getindex(A, I...))
_unsafe_getindex(::LinearIndexing, A::AbstractArray, I...) = (@_propagate_inbounds_meta; getindex(A, I...))

## LinearFast Scalar indexing
_getindex(::LinearFast, A::AbstractArray, I::Int) = error("indexing not defined for ", typeof(A))
function _getindex(::LinearFast, A::AbstractArray, I::Real...)
@_inline_meta
# We must check bounds for sub2ind; so we can then call unsafe_getindex
J = to_indexes(I...)
checkbounds(A, J...)
@boundscheck checkbounds(A, J...)
unsafe_getindex(A, sub2ind(size(A), J...))
end
_unsafe_getindex(::LinearFast, A::AbstractArray, I::Real) = (@_inline_meta; getindex(A, I))
Expand All @@ -520,14 +520,14 @@ end
if all(x->x===Int, I)
:(error("indexing not defined for ", typeof(A)))
else
:($(Expr(:meta, :inline)); getindex(A, to_indexes(I...)...))
:($(Expr(:meta, :inline, :propagate_inbounds)); getindex(A, to_indexes(I...)...))
end
elseif N > AN
# Drop trailing ones
Isplat = Expr[:(I[$d]) for d = 1:AN]
Osplat = Expr[:(to_index(I[$d]) == 1) for d = AN+1:N]
quote
$(Expr(:meta, :inline))
$(Expr(:meta, :inline, :propagate_inbounds))
(&)($(Osplat...)) || throw_boundserror(A, I)
getindex(A, $(Isplat...))
end
Expand All @@ -542,7 +542,7 @@ end
sz.args = Expr[:(size(A, $d)) for d=N:AN]
szcheck = Expr[:(size(A, $d) > 0) for d=N:AN]
quote
$(Expr(:meta, :inline))
$(Expr(:meta, :inline, :propagate_inbounds))
# ind2sub requires all dimensions to be > 0:
(&)($(szcheck...)) || throw_boundserror(A, I)
s = ind2sub($sz, to_index(I[$N]))
Expand All @@ -553,12 +553,12 @@ end
@generated function _unsafe_getindex{T,AN}(::LinearSlow, A::AbstractArray{T,AN}, I::Real...)
N = length(I)
if N == AN
:($(Expr(:meta, :inline)); getindex(A, I...))
:($(Expr(:meta, :inline, :propagate_inbounds)); getindex(A, I...))
elseif N > AN
# Drop trailing dimensions (unchecked)
Isplat = Expr[:(I[$d]) for d = 1:AN]
quote
$(Expr(:meta, :inline))
$(Expr(:meta, :inline, :propagate_inbounds))
unsafe_getindex(A, $(Isplat...))
end
else
Expand All @@ -570,7 +570,7 @@ end
sz = Expr(:tuple)
sz.args = Expr[:(size(A, $d)) for d=N:AN]
quote
$(Expr(:meta, :inline))
$(Expr(:meta, :inline, :propagate_inbounds))
s = ind2sub($sz, to_index(I[$N]))
unsafe_getindex(A, $(Isplat...))
end
Expand All @@ -580,33 +580,33 @@ end
## Setindex! is defined similarly. We first dispatch to an internal _setindex!
# function that allows dispatch on array storage
function setindex!(A::AbstractArray, v, I...)
@_inline_meta
@_propagate_inbounds_meta
_setindex!(linearindexing(A), A, v, I...)
end
function unsafe_setindex!(A::AbstractArray, v, I...)
@_inline_meta
@_propagate_inbounds_meta
_unsafe_setindex!(linearindexing(A), A, v, I...)
end
## Internal defitions
_setindex!(::LinearFast, A::AbstractArray, v) = (@_inline_meta; setindex!(A, v, 1))
_setindex!(::LinearFast, A::AbstractArray, v) = (@_propagate_inbounds_meta; setindex!(A, v, 1))
_setindex!{T}(::LinearSlow, A::AbstractArray{T,0}, v) = error("indexing not defined for ", typeof(A))
_setindex!(::LinearSlow, A::AbstractVector, v) = (@_inline_meta; setindex!(A, v, 1))
_setindex!(l::LinearSlow, A::AbstractArray, v) = (@_inline_meta; _setindex!(l, A, v, 1))
_setindex!(::LinearSlow, A::AbstractVector, v) = (@_propagate_inbounds_meta; setindex!(A, v, 1))
_setindex!(l::LinearSlow, A::AbstractArray, v) = (@_propagate_inbounds_meta; _setindex!(l, A, v, 1))
_unsafe_setindex!(::LinearFast, A::AbstractArray, v) = (@_inline_meta; unsafe_setindex!(A, v, 1))
_unsafe_setindex!{T}(::LinearSlow, A::AbstractArray{T,0}, v) = error("indexing not defined for ", typeof(A))
_unsafe_setindex!(::LinearSlow, A::AbstractVector, v) = (@_inline_meta; unsafe_setindex!(A, v, 1))
_unsafe_setindex!(l::LinearSlow, A::AbstractArray, v) = (@_inline_meta; _unsafe_setindex!(l, A, v, 1))

_setindex!(::LinearIndexing, A::AbstractArray, v, I...) = error("indexing $(typeof(A)) with types $(typeof(I)) is not supported")
_unsafe_setindex!(::LinearIndexing, A::AbstractArray, v, I...) = (@_inline_meta; setindex!(A, v, I...))
_unsafe_setindex!(::LinearIndexing, A::AbstractArray, v, I...) = (@_propagate_inbounds_meta; setindex!(A, v, I...))

## LinearFast Scalar indexing
_setindex!(::LinearFast, A::AbstractArray, v, I::Int) = error("indexed assignment not defined for ", typeof(A))
function _setindex!(::LinearFast, A::AbstractArray, v, I::Real...)
@_inline_meta
# We must check bounds for sub2ind; so we can then call unsafe_setindex!
J = to_indexes(I...)
checkbounds(A, J...)
@boundscheck checkbounds(A, J...)
unsafe_setindex!(A, v, sub2ind(size(A), J...))
end
_unsafe_setindex!(::LinearFast, A::AbstractArray, v, I::Real) = (@_inline_meta; setindex!(A, v, I))
Expand All @@ -622,14 +622,14 @@ end
if all(x->x===Int, I)
:(error("indexing not defined for ", typeof(A)))
else
:($(Expr(:meta, :inline)); setindex!(A, v, to_indexes(I...)...))
:($(Expr(:meta, :inline, :propagate_inbounds)); setindex!(A, v, to_indexes(I...)...))
end
elseif N > AN
# Drop trailing ones
Isplat = Expr[:(I[$d]) for d = 1:AN]
Osplat = Expr[:(to_index(I[$d]) == 1) for d = AN+1:N]
quote
$(Expr(:meta, :inline))
$(Expr(:meta, :inline, :propagate_inbounds))
(&)($(Osplat...)) || throw_boundserror(A, I)
setindex!(A, v, $(Isplat...))
end
Expand All @@ -644,7 +644,7 @@ end
sz.args = Expr[:(size(A, $d)) for d=N:AN]
szcheck = Expr[:(size(A, $d) > 0) for d=N:AN]
quote
$(Expr(:meta, :inline))
$(Expr(:meta, :inline, :propagate_inbounds))
# ind2sub requires all dimensions to be > 0:
(&)($(szcheck...)) || throw_boundserror(A, I)
s = ind2sub($sz, to_index(I[$N]))
Expand All @@ -660,7 +660,7 @@ end
# Drop trailing dimensions (unchecked)
Isplat = Expr[:(I[$d]) for d = 1:AN]
quote
$(Expr(:meta, :inline))
$(Expr(:meta, :inline, :propagate_inbounds))
unsafe_setindex!(A, v, $(Isplat...))
end
else
Expand All @@ -672,7 +672,7 @@ end
sz = Expr(:tuple)
sz.args = Expr[:(size(A, $d)) for d=N:AN]
quote
$(Expr(:meta, :inline))
$(Expr(:meta, :inline, :propagate_inbounds))
s = ind2sub($sz, to_index(I[$N]))
unsafe_setindex!(A, v, $(Isplat...))
end
Expand Down
8 changes: 8 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -952,3 +952,11 @@ end
#14555
@deprecate_binding Coff_t Int64
@deprecate_binding FileOffset Int64

#14474
macro boundscheck(yesno,blk)
depwarn("The meaning of `@boundscheck` has changed. It now indicates that the provided code block performs bounds checking, and may be elided when inbounds.", symbol("@boundscheck"))
if yesno === true
:(@inbounds $(esc(blk)))
end
end
12 changes: 9 additions & 3 deletions base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ end
macro _pure_meta()
Expr(:meta, :pure)
end
# another version of inlining that propagates an inbounds context
macro _propagate_inbounds_meta()
Expr(:meta, :inline, :propagate_inbounds)
end


# constructors for Core types in boot.jl
Expand Down Expand Up @@ -168,15 +172,17 @@ end

esc(e::ANY) = Expr(:escape, e)

macro boundscheck(yesno,blk)
macro boundscheck(blk)
# hack: use this syntax since it avoids introducing line numbers
:($(Expr(:boundscheck,yesno));
:($(Expr(:boundscheck,true));
$(esc(blk));
$(Expr(:boundscheck,:pop)))
end

macro inbounds(blk)
:(@boundscheck false $(esc(blk)))
:($(Expr(:inbounds,true));
$(esc(blk));
$(Expr(:inbounds,:pop)))
end

macro label(name::Symbol)
Expand Down
15 changes: 15 additions & 0 deletions base/expr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,21 @@ macro pure(ex)
esc(isa(ex, Expr) ? pushmeta!(ex, :pure) : ex)
end

"""
@propagate_inbounds(ex)

Tells the compiler to inline a function while retaining the caller's inbounds context.
"""
macro propagate_inbounds(ex)
if isa(ex, Expr)
pushmeta!(ex, :inline)
pushmeta!(ex, :propagate_inbounds)
esc(ex)
else
esc(ex)
end
end

## some macro utilities ##

find_vars(e) = find_vars(e, [])
Expand Down
20 changes: 19 additions & 1 deletion base/inference.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1861,6 +1861,7 @@ function typeinf_uncached(linfo::LambdaStaticData, atypes::ANY, sparams::SimpleV
fulltree.args[3] = inlining_pass(fulltree.args[3], sv, fulltree)[1]
# inlining can add variables
sv.vars = append_any(f_argnames(fulltree), fulltree.args[2][1])
inbounds_meta_elim_pass(fulltree.args[3])
end
tuple_elim_pass(fulltree, sv)
getfield_elim_pass(fulltree.args[3], sv)
Expand Down Expand Up @@ -2414,6 +2415,7 @@ function inlineable(f::ANY, e::Expr, atype::ANY, sv::StaticVarInfo, enclosing_as

body = Expr(:block)
body.args = ast.args[3].args::Array{Any,1}
propagate_inbounds, _ = popmeta!(body, :propagate_inbounds)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JeffBezanson For some reason, unpacking returned Tuples like this doesn't seem to exist elsewhere in inference.jl. Is there a reason it is avoided?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know, but I would guess that most of inference.jl was written at a time when that wasn't available at this point in the bootstrap. If it's working for you, maybe now that functionality is available (perhaps due to @vtjnash's reorganization in #11274)?

This is pure speculation on my part.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think there is any reason to avoid it.

cost::Int = 1000
if incompletematch
cost *= 4
Expand Down Expand Up @@ -2773,6 +2775,12 @@ function inlineable(f::ANY, e::Expr, atype::ANY, sv::StaticVarInfo, enclosing_as
end
end

if !isempty(stmts) && !propagate_inbounds
# inlined statements are out-of-bounds by default
unshift!(stmts, Expr(:inbounds, false))
push!(stmts, Expr(:inbounds, :pop))
end

if isa(expr,Expr)
old_t = e.typ
if old_t <: expr.typ
Expand Down Expand Up @@ -2985,7 +2993,7 @@ function inlining_pass(e::Expr, sv, ast)
end
res = inlineable(f, e, atype, sv, ast)
if isa(res,Tuple)
if isa(res[2],Array)
if isa(res[2],Array) && !isempty(res[2])
append!(stmts,res[2])
end
res = res[1]
Expand Down Expand Up @@ -3265,6 +3273,16 @@ function occurs_outside_getfield(e::ANY, sym::ANY, sv::StaticVarInfo, tuplen::In
return false
end

# removes inbounds metadata if we never encounter an inbounds=true or
# boundscheck context in the method body
function inbounds_meta_elim_pass(e::Expr)
if findfirst(x -> isa(x, Expr) &&
((x.head === :inbounds && x.args[1] == true) || x.head === :boundscheck),
e.args) == 0
filter!(x -> !(isa(x, Expr) && x.head === :inbounds), e.args)
end
end

# replace getfield(tuple(exprs...), i) with exprs[i]
function getfield_elim_pass(e::Expr, sv)
for i = 1:length(e.args)
Expand Down
7 changes: 6 additions & 1 deletion doc/devdocs/ast.rst
Original file line number Diff line number Diff line change
Expand Up @@ -139,12 +139,17 @@ These symbols appear in the ``head`` field of ``Expr``\s in lowered form.
``leave``
pop exception handlers. ``args[1]`` is the number of handlers to pop.

``boundscheck``
``inbounds``
controls turning bounds checks on or off. A stack is maintained; if the
first argument of this expression is true or false (``true`` means bounds
checks are enabled), it is pushed onto the stack. If the first argument is
``:pop``, the stack is popped.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this correct or should it be "bounds checks are disabled"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, true means bounds checks are disabled.

``boundscheck``
indicates the beginning or end of a section of code that performs a bounds
check. Like ``inbounds``, a stack is maintained, and the second argument
can be one of: ``true``, ``false``, or ``:pop``.

``copyast``
part of the implementation of quasi-quote. The argument is a surface syntax
AST that is simply copied recursively and returned at run time.
Expand Down
Loading