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

improve docs for @inbounds and Base.@propagate_inbounds #50157

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
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
46 changes: 38 additions & 8 deletions base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -682,13 +682,14 @@ macro boundscheck(blk)
end

"""
@inbounds(blk)
@inbounds block

Eliminates array bounds checking within expressions.

In the example below the in-range check for referencing
element `i` of array `A` is skipped to improve performance.
Eliminates bounds checking within the block.
This macro can be used to improve performance by informing the compiler that accesses to
array elemements or object fields are assuredly within bounds.
ViralBShah marked this conversation as resolved.
Show resolved Hide resolved

In the example below the in-range check for referencing element `i` of array `A` is skipped
to improve performance.
```julia
function sum(A::AbstractArray)
r = zero(eltype(A))
Expand All @@ -700,14 +701,43 @@ end
```

!!! warning

Using `@inbounds` may return incorrect results/crashes/corruption
for out-of-bounds indices. The user is responsible for checking it manually.
Only use `@inbounds` when it is certain from the information locally available
that all accesses are in bounds. In particular, using `1:length(A)` instead of
`eachindex(A)` in a function like the one above is _not_ safely inbounds because
that all accesses are in-bounds. In particular, using `1:length(A)` instead of
`eachindex(A)` in a function like the one above is _not_ safely in-bounds because
the first index of `A` may not be `1` for all user defined types that subtype
`AbstractArray`.

!!! note
`@inbounds` eleminates bounds checks that are syntactically within the given block,
as well as ones in methods that are called within the block.
Copy link
Member

Choose a reason for hiding this comment

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

This is tied to inlining, no?

julia> g() = @boundscheck error("check from g")
g (generic function with 1 method)

julia> f() = @inbounds g()
f (generic function with 1 method)

julia> f() # no error
julia> @noinline g() = @boundscheck error("check from g")
g (generic function with 1 method)

julia> f() = @inbounds g()
f (generic function with 1 method)

julia> f()
ERROR: check from g
Stacktrace:

ViralBShah marked this conversation as resolved.
Show resolved Hide resolved
However, keep in mind that the `@inbounds` context propagates only one function call
layer deep. For example, if an `@inbounds` block includes a call to `f()`, which in turn
calls `g()`, bounds checks that are syntactically within `f()` will be eliminated,
while ones within `g()` will not.
If you want to eliminates bounds checks within `g()` also,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
If you want to eliminates bounds checks within `g()` also,
If you want to eliminate bounds checks within `g()` also,

you need to annotate [`Base.@propagate_inbounds`](@ref) on `f()`.

# Example

```julia-repl
julia> code_typed((Vector{Any},Int)) do a, i
Copy link
Member

Choose a reason for hiding this comment

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

The do form of code_typed is hard to parse for me, I think it is better to extract that part into it's own function. Also, I feel that arrayref has to be explicitly commented on and explained that the first argument is if it should be boundschecked.

Copy link
Member

Choose a reason for hiding this comment

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

It is also not accurate anymore, since arrayref is changed to memoryrefget and Expr(:boundscheck) is no longer removed, though I am fine with the do-block form. Perhaps this should show the IR for @boundscheck return true; return false

a[i]
end |> only
CodeInfo(
1 ─ %1 = Base.arrayref(true, a, i)::Any
└── return %1
) => Any

julia> code_typed((Vector{Any},Int)) do a, i
@inbounds a[i] # The bounds check for `arrayref` is turned off.
end |> only
CodeInfo(
1 ─ %1 = Base.arrayref(false, a, i)::Any
└── return %1
) => Any
```
"""
macro inbounds(blk)
return Expr(:block,
Expand Down
38 changes: 36 additions & 2 deletions base/expr.jl
Original file line number Diff line number Diff line change
Expand Up @@ -801,9 +801,43 @@ macro nospecializeinfer(ex)
end

"""
@propagate_inbounds
Base.@propagate_inbounds function f(args...)
...
end
Base.@propagate_inbounds f(args...) = ...

Tells the compiler to inline `f` while retaining the caller's `@inbounds` context.
This macro can be used to pass along the `@inbounds` context within the caller of `f` to
callee methods that are invoked within `f`. Without the `@propagate_inbounds` annotation,
the caller's `@inbounds` context propagates only one function call layer deep.

# Example

```julia-repl
julia> call_func(func, args...) = func(args...);

julia> code_typed((Vector{Any},Int)) do a, i
# This `@inbounds` context does not propagate to `getindex`,
# and thus the bounds check for `arrayref` is not turned off.
@inbounds call_func(getindex, a, i)
end |> only
CodeInfo(
1 ─ %1 = Base.arrayref(true, a, i)::Any
└── return %1
) => Any

julia> Base.@propagate_inbounds call_func(func, args...) = func(args...);

Tells the compiler to inline a function while retaining the caller's inbounds context.
julia> code_typed((Vector{Any},Int)) do a, i
# Now this `@inbounds` context propagates to `getindex`,
# and the bounds check for `arrayref` is turned off.
@inbounds call_func(getindex, a, i)
end |> only
CodeInfo(
1 ─ %1 = Base.arrayref(false, a, i)::Any
└── return %1
) => Any
```
"""
macro propagate_inbounds(ex)
if isa(ex, Expr)
Expand Down