Skip to content

Commit

Permalink
macros for turning off bounds checks. fixes #1392
Browse files Browse the repository at this point in the history 
`@inbounds expr` allows bounds checks to be omitted for code syntactically
inside the argument expression.

adds @inbounds to matmul, comprehensions, and vectorized binary operators.
  • Loading branch information
@JeffBezanson
JeffBezanson committed Jun 2, 2013
1 parent 5af0445 commit 66ab577
Show file tree
Hide file tree
Showing 11 changed files with 91 additions and 35 deletions.
16 changes: 8 additions & 8 deletions base/array.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -934,7 +934,7 @@ for f in (:+, :-, :div, :mod, :&, :|, :$)
function ($f){S,T}(A::StridedArray{S}, B::StridedArray{T})
F = Array(promote_type(S,T), promote_shape(size(A),size(B)))
for i=1:length(A)
F[i] = ($f)(A[i], B[i])
@inbounds F[i] = ($f)(A[i], B[i])
end
return F
end
Expand All @@ -945,14 +945,14 @@ for f in (:+, :-, :.*, :./, :div, :mod, :&, :|, :$)
function ($f){T}(A::Number, B::StridedArray{T})
F = similar(B, promote_array_type(typeof(A),T))
for i=1:length(B)
F[i] = ($f)(A, B[i])
@inbounds F[i] = ($f)(A, B[i])
end
return F
end
function ($f){T}(A::StridedArray{T}, B::Number)
F = similar(A, promote_array_type(typeof(B),T))
for i=1:length(A)
F[i] = ($f)(A[i], B)
@inbounds F[i] = ($f)(A[i], B)
end
return F
end
Expand All @@ -961,7 +961,7 @@ for f in (:+, :-, :.*, :./, :div, :mod, :&, :|, :$)
F = Array(promote_type(S,T), promote_shape(size(A),size(B)))
i = 1
for b in B
F[i] = ($f)(A[i], b)
@inbounds F[i] = ($f)(A[i], b)
i += 1
end
return F
Expand All @@ -970,7 +970,7 @@ for f in (:+, :-, :.*, :./, :div, :mod, :&, :|, :$)
F = Array(promote_type(S,T), promote_shape(size(A),size(B)))
i = 1
for a in A
F[i] = ($f)(a, B[i])
@inbounds F[i] = ($f)(a, B[i])
i += 1
end
return F
Expand Down Expand Up @@ -1000,23 +1000,23 @@ function complex{S<:Real,T<:Real}(A::Array{S}, B::Array{T})
if size(A) != size(B); error("argument dimensions must match"); end
F = similar(A, typeof(complex(zero(S),zero(T))))
for i=1:length(A)
F[i] = complex(A[i], B[i])
@inbounds F[i] = complex(A[i], B[i])
end
return F
end

function complex{T<:Real}(A::Real, B::Array{T})
F = similar(B, typeof(complex(A,zero(T))))
for i=1:length(B)
F[i] = complex(A, B[i])
@inbounds F[i] = complex(A, B[i])
end
return F
end

function complex{T<:Real}(A::Array{T}, B::Real)
F = similar(A, typeof(complex(zero(T),B)))
for i=1:length(A)
F[i] = complex(A[i], B)
@inbounds F[i] = complex(A[i], B)
end
return F
end
Expand Down
12 changes: 12 additions & 0 deletions base/base.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,18 @@ function precompile(f, args::Tuple)
end
end

macro boundscheck(yesno,blk)
quote
$(Expr(:boundscheck,yesno))
$(esc(blk))
$(Expr(:boundscheck,0))

This comment has been minimized.

Sign in to view

Copy link
@timholy

timholy Jun 2, 2013

Member

I'm almost certainly misunderstanding, but doesn't this turn off bounds checking after running the user's block? Shouldn't it turn it on?

This comment has been minimized.

Sign in to view

Copy link
@Keno

Keno Jun 2, 2013

Member

reading the code it seems like only true/false (i.e. of type Bool) enable/disable bounds check. Everything else (as long as it's nonempty), just resets the previous one.

This comment has been minimized.

Sign in to view

Copy link
@JeffBezanson

JeffBezanson Jun 2, 2013

Author Member

Correct. Not exactly my clearest code ever. I'll use (boundscheck pop) instead.
end
end

macro inbounds(blk)
:(@boundscheck false $(esc(blk)))
end

# NOTE: Base shares Array with Core so we can add definitions to it

Array{T,N}(::Type{T}, d::NTuple{N,Int}) =
Expand Down
4 changes: 3 additions & 1 deletion base/exports.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1215,4 +1215,6 @@ export
@show,
@printf,
@sprintf,
@deprecate
@deprecate,
@boundscheck,
@inbounds
2 changes: 2 additions & 0 deletions base/linalg/matmul.jl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ function generic_matmatmul{T,S,R}(C::StridedVecOrMat{R}, tA, tB, A::StridedVecOr
if mA == nA == nB == 2; return matmul2x2(C, tA, tB, A, B); end
if mA == nA == nB == 3; return matmul3x3(C, tA, tB, A, B); end

@inbounds begin
if isbits(R)
tile_size = int(ifloor(sqrt(tilebufsize/sizeof(R))))
sz = (tile_size, tile_size)
Expand Down Expand Up @@ -559,6 +560,7 @@ function generic_matmatmul{T,S,R}(C::StridedVecOrMat{R}, tA, tB, A::StridedVecOr
end
end
end
end
return C
end

Expand Down
2 changes: 1 addition & 1 deletion src/alloc.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ jl_sym_t *anonymous_sym; jl_sym_t *underscore_sym;
jl_sym_t *abstracttype_sym; jl_sym_t *bitstype_sym;
jl_sym_t *compositetype_sym; jl_sym_t *type_goto_sym;
jl_sym_t *global_sym; jl_sym_t *tuple_sym;
jl_sym_t *dot_sym;
jl_sym_t *dot_sym; jl_sym_t *boundscheck_sym;

typedef struct {
int64_t a;
Expand Down
52 changes: 31 additions & 21 deletions src/cgutils.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,10 @@ static Value *emit_bounds_check(Value *i, Value *len, jl_codectx_t *ctx)
{
Value *im1 = builder.CreateSub(i, ConstantInt::get(T_size, 1));
#if CHECK_BOUNDS==1
Value *ok = builder.CreateICmpULT(im1, len);
raise_exception_unless(ok, jlboundserr_var, ctx);
if (ctx->boundsCheck.empty() || ctx->boundsCheck.back()==true) {
Value *ok = builder.CreateICmpULT(im1, len);
raise_exception_unless(ok, jlboundserr_var, ctx);
}
#endif
return im1;
}
Expand Down Expand Up @@ -681,9 +683,13 @@ static Value *emit_array_nd_index(Value *a, jl_value_t *ex, size_t nd, jl_value_
{
Value *i = ConstantInt::get(T_size, 0);
Value *stride = ConstantInt::get(T_size, 1);
bool bc = ctx->boundsCheck.empty() || ctx->boundsCheck.back()==true;
#if CHECK_BOUNDS==1
BasicBlock *failBB = BasicBlock::Create(getGlobalContext(), "oob");
BasicBlock *endBB = BasicBlock::Create(getGlobalContext(), "idxend");
BasicBlock *failBB=NULL, *endBB=NULL;
if (bc) {
failBB = BasicBlock::Create(getGlobalContext(), "oob");
endBB = BasicBlock::Create(getGlobalContext(), "idxend");
}
#endif
for(size_t k=0; k < nidxs; k++) {
Value *ii = emit_unbox(T_size, T_psize, emit_unboxed(args[k], ctx));
Expand All @@ -693,28 +699,32 @@ static Value *emit_array_nd_index(Value *a, jl_value_t *ex, size_t nd, jl_value_
Value *d =
k >= nd ? ConstantInt::get(T_size, 1) : emit_arraysize(a, ex, k+1, ctx);
#if CHECK_BOUNDS==1
BasicBlock *okBB = BasicBlock::Create(getGlobalContext(), "ib");
// if !(i < d) goto error
builder.CreateCondBr(builder.CreateICmpULT(ii, d), okBB, failBB);
ctx->f->getBasicBlockList().push_back(okBB);
builder.SetInsertPoint(okBB);
if (bc) {
BasicBlock *okBB = BasicBlock::Create(getGlobalContext(), "ib");
// if !(i < d) goto error
builder.CreateCondBr(builder.CreateICmpULT(ii, d), okBB, failBB);
ctx->f->getBasicBlockList().push_back(okBB);
builder.SetInsertPoint(okBB);
}
#endif
stride = builder.CreateMul(stride, d);
}
}
#if CHECK_BOUNDS==1
Value *alen = emit_arraylen(a, ex, ctx);
// if !(i < alen) goto error
builder.CreateCondBr(builder.CreateICmpULT(i, alen), endBB, failBB);

ctx->f->getBasicBlockList().push_back(failBB);
builder.SetInsertPoint(failBB);
builder.CreateCall2(jlthrow_line_func, builder.CreateLoad(jlboundserr_var),
ConstantInt::get(T_int32, ctx->lineno));
builder.CreateUnreachable();

ctx->f->getBasicBlockList().push_back(endBB);
builder.SetInsertPoint(endBB);
if (bc) {
Value *alen = emit_arraylen(a, ex, ctx);
// if !(i < alen) goto error
builder.CreateCondBr(builder.CreateICmpULT(i, alen), endBB, failBB);

ctx->f->getBasicBlockList().push_back(failBB);
builder.SetInsertPoint(failBB);
builder.CreateCall2(jlthrow_line_func, builder.CreateLoad(jlboundserr_var),
ConstantInt::get(T_int32, ctx->lineno));
builder.CreateUnreachable();

ctx->f->getBasicBlockList().push_back(endBB);
builder.SetInsertPoint(endBB);
}
#endif

return i;
Expand Down
19 changes: 19 additions & 0 deletions src/codegen.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,7 @@ typedef struct {
bool vaStack; // varargs stack-allocated
int nReqArgs;
int lineno;
std::vector<bool> boundsCheck;
} jl_codectx_t;

static Value *emit_expr(jl_value_t *expr, jl_codectx_t *ctx, bool boxed=true,
Expand Down Expand Up @@ -2057,6 +2058,23 @@ static Value *emit_expr(jl_value_t *expr, jl_codectx_t *ctx, bool isboxed,
#endif
builder.SetInsertPoint(tryblk);
}
else if (head == boundscheck_sym) {
if (jl_array_len(ex->args) > 0) {
jl_value_t *arg = args[0];
if (arg == jl_true) {
ctx->boundsCheck.push_back(true);
}
else if (arg == jl_false) {
ctx->boundsCheck.push_back(false);
}
else {
if (!ctx->boundsCheck.empty())
ctx->boundsCheck.pop_back();
}
}
if (valuepos)
return literal_pointer_val((jl_value_t*)jl_nothing);
}
else {
if (!strcmp(head->name, "$"))
jl_error("syntax: prefix $ in non-quoted expression");
Expand Down Expand Up @@ -2211,6 +2229,7 @@ static Function *emit_function(jl_lambda_info_t *lam, bool cstyle)
ctx.funcName = lam->name->name;
ctx.vaName = NULL;
ctx.vaStack = false;
ctx.boundsCheck.push_back(true);

// step 2. process var-info lists to see what vars are captured, need boxing
jl_array_t *largs = jl_lam_args(ast);
Expand Down
3 changes: 3 additions & 0 deletions src/interpreter.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,9 @@ static jl_value_t *eval(jl_value_t *e, jl_value_t **locals, size_t nl)
jl_errorf("syntax: %s", jl_string_data(args[0]));
jl_throw(args[0]);
}
else if (ex->head == boundscheck_sym) {
return (jl_value_t*)jl_nothing;
}
jl_errorf("unsupported or misplaced expression %s", ex->head->name);
return (jl_value_t*)jl_nothing;
}
Expand Down
1 change: 1 addition & 0 deletions src/jltypes.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2791,4 +2791,5 @@ void jl_init_types(void)
tuple_sym = jl_symbol("tuple");
kw_sym = jl_symbol("kw");
dot_sym = jl_symbol(".");
boundscheck_sym = jl_symbol("boundscheck");
}
14 changes: 10 additions & 4 deletions src/julia-syntax.scm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1565,7 +1565,9 @@
(if (null? ranges)
`(block (= ,oneresult ,expr)
(type_goto ,initlabl)
(boundscheck false)
(call (top setindex!) ,result ,oneresult ,ri)
(boundscheck 0)
(= ,ri (call (top +) ,ri 1)))
`(for ,(car ranges)
,(construct-loops (cdr ranges)))))
Expand Down Expand Up @@ -1593,9 +1595,10 @@
(typed_comprehension atype expr . ranges)
(if (any (lambda (x) (eq? x ':)) ranges)
(lower-nd-comprehension atype expr ranges)
(let ( (result (gensy))
(ri (gensy))
(rs (map (lambda (x) (gensy)) ranges)) )
(let ((result (gensy))
(oneresult (gensy))
(ri (gensy))
(rs (map (lambda (x) (gensy)) ranges)) )

;; compute the dimensions of the result
(define (compute-dims ranges)
Expand All @@ -1605,7 +1608,10 @@
;; construct loops to cycle over all dimensions of an n-d comprehension
(define (construct-loops ranges rs)
(if (null? ranges)
`(block (call (top setindex!) ,result ,expr ,ri)
`(block (= ,oneresult ,expr)
(boundscheck false)
(call (top setindex!) ,result ,oneresult ,ri)
(boundscheck 0)
(= ,ri (call (top +) ,ri 1)))
`(for (= ,(cadr (car ranges)) ,(car rs))
,(construct-loops (cdr ranges) (cdr rs)))))
Expand Down
1 change: 1 addition & 0 deletions src/julia.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,7 @@ extern jl_sym_t *anonymous_sym; extern jl_sym_t *underscore_sym;
extern jl_sym_t *abstracttype_sym; extern jl_sym_t *bitstype_sym;
extern jl_sym_t *compositetype_sym; extern jl_sym_t *type_goto_sym;
extern jl_sym_t *global_sym; extern jl_sym_t *tuple_sym;
extern jl_sym_t *boundscheck_sym;


#ifdef _P64
Expand Down

4 comments on commit 66ab577

@timholy
Copy link
Member

@timholy timholy commented on 66ab577 Jun 2, 2013

Choose a reason for hiding this comment

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

Hooray!

@timholy
Copy link
Member

@timholy timholy commented on 66ab577 Jun 2, 2013

Choose a reason for hiding this comment

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

I also suspect we should have a special startup mode (via a command-line flag) that defines

macro boundscheck(yesno, blk)
    blk
end

That way we can guard against overly-hasty usages of @inbounds and rapidly find the problem point.

@ViralBShah
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to have the @inbounds apply to a whole module?

Totally agree that a command line flag when starting julia that can force all checks to be carried out is a useful debugging tool.

@staticfloat
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps alternatively/additionally, when Julia is compiled in debug mode, these shouldn't work either.

Please sign in to comment.