Skip to content

Don't @inbounds AbstractArray's iterate method; optimize checkbounds instead #58793

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.

Sign up for GitHub

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

Merged
merged 14 commits into from
Jul 2, 2025
Merged

Don't @inbounds AbstractArray's iterate method; optimize checkbounds instead #58793

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f831f6f
no inbounds on AbstractArray iterate and axe Array/Memory methods
giordano Jun 23, 2025
b09514e
avoid branch in OrdinalRange length
mbauman Jun 23, 2025
9082e68
fix SubArray axes1
mbauman Jun 23, 2025
6d727aa
implement optimized checkbounds for FastSubArray
mbauman Jun 23, 2025
aaa8629
Remove inbounds on CodeUnits, too
mbauman Jun 23, 2025
683f23c
add tests
mbauman Jun 23, 2025
f31083e
fixup! fix SubArray axes1
mbauman Jun 24, 2025
74cf796
remove bugged SubArray checkbounds method
mbauman Jun 24, 2025
6b601bf
Merge remote-tracking branch 'origin/master' into mb+mg/array-iteration
mbauman Jun 24, 2025
6e1afb5
CodeUnits: implement checkbounds and fallback for iterate
mbauman Jun 24, 2025
d97e69f
do not test for unreachable with check-bounds=yes
mbauman Jun 24, 2025
dcb3f55
fix ambiguity and more properly constrain checkbounds for CodeUnits
mbauman Jun 24, 2025
8d378df
use flipsign instead of multiplying sign
mbauman Jul 1, 2025
61e26a0
defer length(::OrdinalRange) changes; just inline it for now
mbauman Jul 1, 2025
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
9 changes: 5 additions & 4 deletions base/abstractarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ function eachindex(A::AbstractArray, B::AbstractArray...)
@inline
eachindex(IndexStyle(A,B...), A, B...)
end
eachindex(::IndexLinear, A::Union{Array, Memory}) = unchecked_oneto(length(A))
eachindex(::IndexLinear, A::AbstractArray) = (@inline; oneto(length(A)))
eachindex(::IndexLinear, A::AbstractVector) = (@inline; axes1(A))
function eachindex(::IndexLinear, A::AbstractArray, B::AbstractArray...)
Expand Down Expand Up @@ -1237,15 +1238,15 @@ oneunit(x::AbstractMatrix{T}) where {T} = _one(oneunit(T), x)
iterate_starting_state(A) = iterate_starting_state(A, IndexStyle(A))
iterate_starting_state(A, ::IndexLinear) = firstindex(A)
iterate_starting_state(A, ::IndexStyle) = (eachindex(A),)
iterate(A::AbstractArray, state = iterate_starting_state(A)) = _iterate(A, state)
function _iterate(A::AbstractArray, state::Tuple)
@inline iterate(A::AbstractArray, state = iterate_starting_state(A)) = _iterate(A, state)
@inline function _iterate(A::AbstractArray, state::Tuple)
y = iterate(state...)
y === nothing && return nothing
A[y[1]], (state[1], tail(y)...)
end
function _iterate(A::AbstractArray, state::Integer)
@inline function _iterate(A::AbstractArray, state::Integer)
Comment on lines +1241 to +1247
Copy link
Member

Choose a reason for hiding this comment

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

are these functions really so big that they aren't being inlined automatically?

Copy link
Member Author

@mbauman mbauman Jun 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Before b09514e, yes, they were (well, kinda. length itself was not inlining; had I added @inline to it, though, it'd make this method not inline). That branch was enough to push things over the inlining limit. As I wrote in #58785 (comment),

this calls abstract infrastructure that could be large (and itself @inline'd)... which would then require these generic methods to be similarly @inline.

checkbounds(Bool, A, state) || return nothing
@inbounds(A[state]), state + one(state)
A[state], state + one(state)
end

isempty(a::AbstractArray) = (length(a) == 0)
Expand Down
4 changes: 0 additions & 4 deletions base/array.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,10 +902,6 @@ function grow_to!(dest, itr, st)
return dest
end

## Iteration ##

iterate(A::Array, i=1) = (@inline; _iterate_array(A, i))

## Indexing: getindex ##

"""
Expand Down
9 changes: 0 additions & 9 deletions base/genericmemory.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,15 +223,6 @@ Memory{T}(x::AbstractArray{S,1}) where {T,S} = copyto_axcheck!(Memory{T}(undef,

## copying iterators to containers

## Iteration ##

function _iterate_array(A::Union{Memory, Array}, i::Int)
@inline
checkbounds(Bool, A, i) ? (A[i], i + 1) : nothing
end

iterate(A::Memory, i=1) = (@inline; _iterate_array(A, i))

## Indexing: getindex ##

# Faster contiguous indexing using copyto! for AbstractUnitRange and Colon
Expand Down
3 changes: 2 additions & 1 deletion base/range.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -687,7 +687,7 @@ end
## interface implementations

length(r::AbstractRange) = error("length implementation missing") # catch mistakes
size(r::AbstractRange) = (length(r),)
size(r::AbstractRange) = (@inline; (length(r),))

isempty(r::StepRange) =
# steprange_last(r.start, r.step, r.stop) == r.stop
Expand Down Expand Up @@ -802,6 +802,7 @@ let bigints = Union{Int, UInt, Int64, UInt64, Int128, UInt128},
# slightly more accurate length and checked_length in extreme cases
# (near typemax) for types with known `unsigned` functions
function length(r::OrdinalRange{T}) where T<:bigints
@inline
s = step(r)
diff = last(r) - first(r)
isempty(r) && return zero(diff)
Expand Down
2 changes: 1 addition & 1 deletion base/strings/basic.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ size(s::CodeUnits) = (length(s),)
elsize(s::Type{<:CodeUnits{T}}) where {T} = sizeof(T)
@propagate_inbounds getindex(s::CodeUnits, i::Int) = codeunit(s.s, i)
IndexStyle(::Type{<:CodeUnits}) = IndexLinear()
@inline iterate(s::CodeUnits, i=1) = checkbounds(Bool, s, i) ? (@inbounds s[i], i + 1) : nothing
checkbounds(::Type{Bool}, s::CodeUnits, i::Integer) = checkbounds(Bool, s.s, i)


write(io::IO, s::CodeUnits) = write(io, s.s)
Expand Down
10 changes: 10 additions & 0 deletions base/subarray.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,16 @@ function _indices_sub(i1::AbstractArray, I...)
(axes(i1)..., _indices_sub(I...)...)
end

axes1(::SubArray{<:Any,0}) = OneTo(1)
axes1(S::SubArray) = (@inline; _axes1_sub(S.indices...))
_axes1_sub() = ()
_axes1_sub(::Real, I...) = (@inline; _axes1_sub(I...))
_axes1_sub(::AbstractArray{<:Any,0}, I...) = _axes1_sub(I...)
function _axes1_sub(i1::AbstractArray, I...)
@inline
axes1(i1)
end

has_offset_axes(S::SubArray) = has_offset_axes(S.indices...)

function replace_in_print_matrix(S::SubArray{<:Any,2,<:AbstractMatrix}, i::Integer, j::Integer, s::AbstractString)
Expand Down
16 changes: 16 additions & 0 deletions test/boundscheck_exec.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,4 +353,20 @@ if bc_opt == bc_default
@test (@allocated no_alias_prove(5)) == 0
end

@testset "automatic boundscheck elision for iteration on some important types" begin
if bc_opt != bc_on
@test !contains(sprint(code_llvm, iterate, (Memory{UInt8}, Int)), "unreachable")

@test !contains(sprint(code_llvm, iterate, (Vector{UInt8}, Int)), "unreachable")
@test !contains(sprint(code_llvm, iterate, (Matrix{UInt8}, Int)), "unreachable")
@test !contains(sprint(code_llvm, iterate, (Array{UInt8,3}, Int)), "unreachable")

@test !contains(sprint(code_llvm, iterate, (SubArray{Float64, 1, Vector{Float64}, Tuple{Base.Slice{Base.OneTo{Int64}}}, true}, Int)), "unreachable")
@test !contains(sprint(code_llvm, iterate, (SubArray{Float64, 2, Matrix{Float64}, Tuple{Base.Slice{Base.OneTo{Int64}}, Base.Slice{Base.OneTo{Int64}}}, true}, Int)), "unreachable")
@test !contains(sprint(code_llvm, iterate, (SubArray{Float64, 2, Matrix{Float64}, Tuple{Base.Slice{Base.OneTo{Int64}}, UnitRange{Int64}}, true}, Int)), "unreachable")

@test !contains(sprint(code_llvm, iterate, (Base.CodeUnits{UInt8,String}, Int)), "unreachable")
end
end

end