Skip to content

Commit 89faa35

Browse files
mbaumanKristofferC
authored andcommitted
mapreduce: don't inbounds unknown functions (#55329)
More finely scope the `@inbounds` annotations to ensure neither `f` nor `op` are erroneously `@inbounds`ed. (cherry picked from commit 1dffd77)
1 parent 16a7ad9 commit 89faa35

File tree

3 files changed

+48
-23
lines changed

3 files changed

+48
-23
lines changed

base/reduce.jl

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -646,11 +646,11 @@ function mapreduce_impl(f, op::Union{typeof(max), typeof(min)},
646646
start = first + 1
647647
simdstop = start + chunk_len - 4
648648
while simdstop <= last - 3
649-
@inbounds for i in start:4:simdstop
650-
v1 = _fast(op, v1, f(A[i+0]))
651-
v2 = _fast(op, v2, f(A[i+1]))
652-
v3 = _fast(op, v3, f(A[i+2]))
653-
v4 = _fast(op, v4, f(A[i+3]))
649+
for i in start:4:simdstop
650+
v1 = _fast(op, v1, f(@inbounds(A[i+0])))
651+
v2 = _fast(op, v2, f(@inbounds(A[i+1])))
652+
v3 = _fast(op, v3, f(@inbounds(A[i+2])))
653+
v4 = _fast(op, v4, f(@inbounds(A[i+3])))
654654
end
655655
checkbounds(A, simdstop+3)
656656
start += chunk_len

base/reducedim.jl

Lines changed: 19 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -272,19 +272,20 @@ function _mapreducedim!(f, op, R::AbstractArray, A::AbstractArrayOrBroadcasted)
272272
if reducedim1(R, A)
273273
# keep the accumulator as a local variable when reducing along the first dimension
274274
i1 = first(axes1(R))
275-
@inbounds for IA in CartesianIndices(indsAt)
275+
for IA in CartesianIndices(indsAt)
276276
IR = Broadcast.newindex(IA, keep, Idefault)
277-
r = R[i1,IR]
277+
@inbounds r = R[i1,IR]
278278
@simd for i in axes(A, 1)
279-
r = op(r, f(A[i, IA]))
279+
r = op(r, f(@inbounds(A[i, IA])))
280280
end
281-
R[i1,IR] = r
281+
@inbounds R[i1,IR] = r
282282
end
283283
else
284-
@inbounds for IA in CartesianIndices(indsAt)
284+
for IA in CartesianIndices(indsAt)
285285
IR = Broadcast.newindex(IA, keep, Idefault)
286286
@simd for i in axes(A, 1)
287-
R[i,IR] = op(R[i,IR], f(A[i,IA]))
287+
v = op(@inbounds(R[i,IR]), f(@inbounds(A[i,IA])))
288+
@inbounds R[i,IR] = v
288289
end
289290
end
290291
end
@@ -1028,33 +1029,33 @@ function findminmax!(f, op, Rval, Rind, A::AbstractArray{T,N}) where {T,N}
10281029
zi = zero(eltype(ks))
10291030
if reducedim1(Rval, A)
10301031
i1 = first(axes1(Rval))
1031-
@inbounds for IA in CartesianIndices(indsAt)
1032+
for IA in CartesianIndices(indsAt)
10321033
IR = Broadcast.newindex(IA, keep, Idefault)
1033-
tmpRv = Rval[i1,IR]
1034-
tmpRi = Rind[i1,IR]
1034+
@inbounds tmpRv = Rval[i1,IR]
1035+
@inbounds tmpRi = Rind[i1,IR]
10351036
for i in axes(A,1)
10361037
k, kss = y::Tuple
1037-
tmpAv = f(A[i,IA])
1038+
tmpAv = f(@inbounds(A[i,IA]))
10381039
if tmpRi == zi || op(tmpRv, tmpAv)
10391040
tmpRv = tmpAv
10401041
tmpRi = k
10411042
end
10421043
y = iterate(ks, kss)
10431044
end
1044-
Rval[i1,IR] = tmpRv
1045-
Rind[i1,IR] = tmpRi
1045+
@inbounds Rval[i1,IR] = tmpRv
1046+
@inbounds Rind[i1,IR] = tmpRi
10461047
end
10471048
else
1048-
@inbounds for IA in CartesianIndices(indsAt)
1049+
for IA in CartesianIndices(indsAt)
10491050
IR = Broadcast.newindex(IA, keep, Idefault)
10501051
for i in axes(A, 1)
10511052
k, kss = y::Tuple
1052-
tmpAv = f(A[i,IA])
1053-
tmpRv = Rval[i,IR]
1054-
tmpRi = Rind[i,IR]
1053+
tmpAv = f(@inbounds(A[i,IA]))
1054+
@inbounds tmpRv = Rval[i,IR]
1055+
@inbounds tmpRi = Rind[i,IR]
10551056
if tmpRi == zi || op(tmpRv, tmpAv)
1056-
Rval[i,IR] = tmpAv
1057-
Rind[i,IR] = k
1057+
@inbounds Rval[i,IR] = tmpAv
1058+
@inbounds Rind[i,IR] = k
10581059
end
10591060
y = iterate(ks, kss)
10601061
end

test/reducedim.jl

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -587,6 +587,30 @@ end
587587
@test B[argmin(B, dims=[2, 3])] == @inferred(minimum(B, dims=[2, 3]))
588588
end
589589

590+
@testset "careful with @inbounds" begin
591+
Base.@propagate_inbounds f(x) = x == 2 ? x[-10000] : x
592+
Base.@propagate_inbounds op(x,y) = x[-10000] + y[-10000]
593+
for (arr, dims) in (([1,1,2], 1), ([1 1 2], 2), ([ones(Int,256);2], 1))
594+
@test_throws BoundsError mapreduce(f, +, arr)
595+
@test_throws BoundsError mapreduce(f, +, arr; dims)
596+
@test_throws BoundsError mapreduce(f, +, arr; dims, init=0)
597+
@test_throws BoundsError mapreduce(identity, op, arr)
598+
try
599+
#=@test_throws BoundsError=# mapreduce(identity, op, arr; dims)
600+
catch ex
601+
@test_broken ex isa BoundsError
602+
end
603+
@test_throws BoundsError mapreduce(identity, op, arr; dims, init=0)
604+
605+
@test_throws BoundsError findmin(f, arr)
606+
@test_throws BoundsError findmin(f, arr; dims)
607+
608+
@test_throws BoundsError mapreduce(f, max, arr)
609+
@test_throws BoundsError mapreduce(f, max, arr; dims)
610+
@test_throws BoundsError mapreduce(f, max, arr; dims, init=0)
611+
end
612+
end
613+
590614
@testset "in-place reductions with mismatched dimensionalities" begin
591615
B = reshape(1:24, 4, 3, 2)
592616
for R in (fill(0, 4), fill(0, 4, 1), fill(0, 4, 1, 1))

0 commit comments

Comments
 (0)