Better document for @inbounds

[lopezm94]

Recently I opened pull request #17700 due to a confusing statement about @inbounds and @fastmath in the documentation. However even if the documentation is correct, these macros might be better throwing an error when trying to optimize an unsupported expression. If not an error then it could have some kind of warn keyword.

@inbounds warn=true function f()
    ...
end

Edit (yyc): See here for a list of things to improve.

[yuyichao]

What counts as "unsupported"?

so @inbounds f() = 1 should throw an error? What about @inbounds begin f() = 1 end?

[eschnett]

@inbounds is supposed to disable bounds checking everywhere in its argument. If it doesn't do that, it should raise an error instead.

In other words, the code

@inbounds f(a,i) = a[i]

should either disable bounds checking in the array access in the defined function f, or should report an error.

[yuyichao]

I don't see how the error can possibly be raised reliably.

Examples [a[i] for i in 1:10], @inline f(a) = a[1], @macro_that_define_a_function.

[yuyichao]

Also @inbounds is supposed to disable bounds check when evaluating it's argument, not everywhere in its argument, the "everywhere in its argument" is also kind of ill-defined especially for metaprogramming.

[lopezm94]

With unsupported I mean not operating @inbounds recursively, for example:

julia> @inbounds function g(x)
           return x[2]
       end

julia> g([1])
ERROR: BoundsError: attempt to access 1-element Array{Int64,1}:
 1
  at index [2]
 in g at none:2

I'm just grasping now how much I didn't understand about @inbounds, I thought it could be applied anywhere but it can only be used mostly inside function definitions

julia> @inbounds if true
           [1][2]
       end
ERROR: BoundsError: attempt to access 1-element Array{Int64,1} at index [2]
 in getindex(::Array{Int64,1}, ::Int64) at ./array.jl:309
 in eval(::Module, ::Any) at ./boot.jl:225
 in macro expansion at ./REPL.jl:92 [inlined]
 in (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:46

julia> function f()
           @inbounds if true
               [1][2]
           end
       end

julia> f()

Replace if for begin and the same thing will happen, it will work inside the function definition but not outside

The problem is that the explanation in performance tips is not very clear about the usage. And not having errors for cases like the one in the description, makes it even more confusing.

both @inbounds and @fastmath can be applied to several statements at once, e.g. using begin ... end, or even to a whole function

The only exception is the for loop, it can be done anywhere

@inbounds for i in 1:10
    [1][2]
end

It is unclear what works and what doesn't. So errors or something more in the docs would make it less mysterious.

[lopezm94]

If there are some expressions it can't possibly remove bounds check from, then it must say something
somewhere, whether the docs or a warning message.

[yuyichao]

@inbounds does not guarantee what the behavior is if you do have oob access, i.e. it doesn't guarentee bounds check is always disabled. This is the difference you see when operating in global scope. FWIW, [1][2] in a @inbounds can do anything the compiler feel like including segfault and throwing a bounds error.

[yuyichao]

I don't think we should make @inbounds operate recursively, since it's almost impossible to do consistently (it has to do macro expansion and understand everything the lowering creates automatically). For the same reason it can't reliably detect such cases either (if it can detect them it can operate recursively).

If it is not in the doc already, it should be mentioned that @inbounds works when "evaluating" it's argument, and doesn't do anything for the stuff (functions, for example) you define in its argument. Together with the confusion at #17643, it should also be clearly documented that any OOB access in @inbounds (defined as if an error would have been thrown without @inbounds) is undefined bahavior and the compiler can do anything it want, including not removing the bounds check.

[yuyichao]

I've added the doc label ~10s before your comment ... ;-p

[lopezm94]

I guess #17700 should be closed too, I can try to explain it better but I don't particularly understand fully yet

[yuyichao]

I believe you can improve on #17700 by giving an example of operating on a whole function:

f() = @inbounds begin
...
end

And mentions that the bounds check is only/might only be disabled during the evaluation of it's arguments and therefore

@inbounds f() = begin
....
end

only disables boundscheck (might not even since it's most likely interpreted rather than compiled) when defining the method for f and not in the body of f.

[yuyichao]

Do note that f() = begin end is not a recommended style of creating multiline functions.... it's usually more clear to use a function.

[lopezm94]

I would be grateful if you follow that PR in case I say something out of place. I'll definitely add some "silent error" examples and how to avoid them.

It is still kind of weird why the for loopworks and begin end doesn't.

[yuyichao]

A better way to think of @inbounds is that it's a guarantee from the user that there isn't any oob access (defined above) and a hint to the compiler that it can remove bounds check if it feels like it. It is not guaranteed applying it on any particular expressions will disable any bounds check.
In practice the removal depends on inlining so it should work for type stable code in functions or any compiled code.

[lopezm94]

All of this applies applies to @fastmath as well I believe (worth to put in title?), it is explained in the same way as @inbounds.

[yuyichao]

The recursive behavior should be the same between the two. The implementation detail is quite different.

[eschnett]

@fastmath takes the parse tree as input and modifies it. (It's a proper macro.) That is, putting @fastmath in front of a function definition will work fine. @fastmath is lexically scoped.

@inbounds plays with the internal state of the compiler by adding meta tags to the parse tree that are then evaluated later. Apparently this means that

@inbounds f(x) = ...

won't work, and neither will

function f(x)
    @inbounds
    ... more code here
end

But this will work:

f(x) = @inbounds begin
    ... all the code here ...
end

and so will this:

function f(x)
    @inbounds begin
        ... all the code here
    end
end

[eschnett]

@lopezm94 Just to be clear: No, @fastmath does not have these issues.

[lopezm94]

Thank you, I think is now much clearer for me to explain

julia> macroexpand(:(@fastmath function f() 1+1 end))
:(function f() # none, line 1:
        Base.FastMath.add_fast(1,1)
    end)

julia> macroexpand(:(@inbounds function f() 1+1 end))
quote 
    $(Expr(:boundscheck, false))
    begin 
        function f() # none, line 1:
            1 + 1
        end
        $(Expr(:boundscheck, :(Base.pop)))
    end
end

[yuyichao]

No, @fastmath does not have these issues.

Except it does.

julia> macro m(f)
           :($(esc(f))() = sin(1))
       end
@m (macro with 1 method)

julia> macroexpand(:(@fastmath @m(f)))
:(f() = begin  # REPL[1], line 2:
            sin(1)
        end)

And it probably shouldn't really operate recursively for consistency.

[lopezm94]

I finished the docs, if you guys wanna see

[dlfivefifty]

I ran into an issue with stacking functions, each with @inline, where bounds checking was still happening:

@inline unsafe_getindex(data::AbstractMatrix, u::Integer, k::Integer, j::Integer) =
    Base.unsafe_getindex(data,u + k - j + 1, j)

# banded get index, used for banded matrices with other data types
@inline function banded_getindex(data::AbstractMatrix, l::Integer, u::Integer, k::Integer, j::Integer)
    @boundscheck !(-l ≤ j-k ≤ u) && return zero(eltype(data))
    unsafe_getindex(data, u, k, j)
end


# scalar - integer - integer
@inline function getindex(A::AbstractBandedMatrix, k::Integer, j::Integer)
    @boundscheck  checkbounds(A, k, j)
    banded_getindex(A,A.l,A.u,k,j)
end

Copy-and-pasting the definition of banded_getindex into getindex fixes the issue, but I'm confused at why @inline wasn't equivalent to copy-and-paste.

[dlfivefifty]

Just realized this code is a bad idea as Base assumes @inbounds works for all indices:

function copy!{T,N}(dest::AbstractArray{T,N}, src::AbstractArray{T,N})
    @boundscheck checkbounds(dest, indices(src)...)
    for I in eachindex(linearindexing(src,dest), src)
        @inbounds dest[I] = src[I]
    end
    dest
end

It looks like it's not possible to add my own @inbands.... I guess best to use unsafe_getindex directly.

[blakejohnson]

@dlfivefifty inlining can also happen automatically, so we needed to ensure that our bounds check removal strategy would not have surprising effects on code that the user did not write. Therefore, we treat function boundaries as special, even if they are inlined.

What you need to get your example to work is @Base.propagate_inbounds.

[dlfivefifty]

Thanks! so I don't need to use @inline if I use @propagate_inbounds? Or is it

@propogate_inbounds @inline function ...
end

Unfortunately, it doesn't really help for this code as it looks like one shouldn't use @inbounds to mean "within sparsity pattern", which is fair enough.

Hopefully eventually it will be possible to add an @inbands or @insparse macro. Until then, I added functions inbands_getindex and inbands_setindex!

[blakejohnson]

@propagate_inbounds implies @inline, so you don't need both.

[fredrikekre]

#19726 enough to close this?

[KristofferC]

I think so.