Skip to content

Commit 3830665

Browse files
mbaumangiordano
authored andcommitted
Remove bugged and typically slower minimum/maximum method (#58267)
This method intends to be a SIMD-able optimization for reductions with `min` and `max`, but it fails to achieve those goals on nearly every architecture. For example, on my Mac M1 the generic implementation is more than **6x** faster than this method. Fixes #45932, fixes #36412, fixes #36081. --------- Co-authored-by: Mosè Giordano <[email protected]> (cherry picked from commit 99ba3c7)
1 parent b310ecf commit 3830665

File tree

2 files changed

+19
-58
lines changed

2 files changed

+19
-58
lines changed

base/reduce.jl

Lines changed: 0 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -609,63 +609,6 @@ julia> prod(1:5; init = 1.0)
609609
prod(a; kw...) = mapreduce(identity, mul_prod, a; kw...)
610610

611611
## maximum, minimum, & extrema
612-
_fast(::typeof(min),x,y) = min(x,y)
613-
_fast(::typeof(max),x,y) = max(x,y)
614-
function _fast(::typeof(max), x::AbstractFloat, y::AbstractFloat)
615-
ifelse(isnan(x),
616-
x,
617-
ifelse(x > y, x, y))
618-
end
619-
620-
function _fast(::typeof(min),x::AbstractFloat, y::AbstractFloat)
621-
ifelse(isnan(x),
622-
x,
623-
ifelse(x < y, x, y))
624-
end
625-
626-
isbadzero(::typeof(max), x::AbstractFloat) = (x == zero(x)) & signbit(x)
627-
isbadzero(::typeof(min), x::AbstractFloat) = (x == zero(x)) & !signbit(x)
628-
isbadzero(op, x) = false
629-
isgoodzero(::typeof(max), x) = isbadzero(min, x)
630-
isgoodzero(::typeof(min), x) = isbadzero(max, x)
631-
632-
function mapreduce_impl(f, op::Union{typeof(max), typeof(min)},
633-
A::AbstractArrayOrBroadcasted, first::Int, last::Int)
634-
a1 = @inbounds A[first]
635-
v1 = mapreduce_first(f, op, a1)
636-
v2 = v3 = v4 = v1
637-
chunk_len = 256
638-
start = first + 1
639-
simdstop = start + chunk_len - 4
640-
while simdstop <= last - 3
641-
for i in start:4:simdstop
642-
v1 = _fast(op, v1, f(@inbounds(A[i+0])))
643-
v2 = _fast(op, v2, f(@inbounds(A[i+1])))
644-
v3 = _fast(op, v3, f(@inbounds(A[i+2])))
645-
v4 = _fast(op, v4, f(@inbounds(A[i+3])))
646-
end
647-
checkbounds(A, simdstop+3)
648-
start += chunk_len
649-
simdstop += chunk_len
650-
end
651-
v = op(op(v1,v2),op(v3,v4))
652-
for i in start:last
653-
@inbounds ai = A[i]
654-
v = op(v, f(ai))
655-
end
656-
657-
# enforce correct order of 0.0 and -0.0
658-
# e.g. maximum([0.0, -0.0]) === 0.0
659-
# should hold
660-
if isbadzero(op, v)
661-
for i in first:last
662-
x = @inbounds A[i]
663-
isgoodzero(op,x) && return x
664-
end
665-
end
666-
return v
667-
end
668-
669612
"""
670613
maximum(f, itr; [init])
671614

test/reduce.jl

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,19 +299,37 @@ end
299299
arr = zeros(N)
300300
@test minimum(arr) === 0.0
301301
@test maximum(arr) === 0.0
302+
@test minimum(abs, arr) === 0.0
303+
@test maximum(abs, arr) === 0.0
304+
@test minimum(-, arr) === -0.0
305+
@test maximum(-, arr) === -0.0
302306

303307
arr[i] = -0.0
304308
@test minimum(arr) === -0.0
305309
@test maximum(arr) === 0.0
310+
@test minimum(abs, arr) === 0.0
311+
@test maximum(abs, arr) === 0.0
312+
@test minimum(-, arr) === -0.0
313+
@test maximum(-, arr) === 0.0
306314

307315
arr = -zeros(N)
308316
@test minimum(arr) === -0.0
309317
@test maximum(arr) === -0.0
318+
@test minimum(abs, arr) === 0.0
319+
@test maximum(abs, arr) === 0.0
320+
@test minimum(-, arr) === 0.0
321+
@test maximum(-, arr) === 0.0
310322
arr[i] = 0.0
311323
@test minimum(arr) === -0.0
312-
@test maximum(arr) === 0.0
324+
@test maximum(arr) === 0.0
325+
@test minimum(abs, arr) === 0.0
326+
@test maximum(abs, arr) === 0.0
327+
@test minimum(-, arr) === -0.0
328+
@test maximum(-, arr) === 0.0
313329
end
314330
end
331+
332+
@test minimum(abs, fill(-0.0, 16)) === mapreduce(abs, (x,y)->min(x,y), fill(-0.0, 16)) === 0.0
315333
end
316334

317335
@testset "maximum works on generic order #30320" begin

0 commit comments

Comments
 (0)