-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Make minmax faster for Float32/64 #41709
Changes from all commits
5936574
3e39c57
04f07fb
39d6878
18f5a7c
b64fddb
563ed8c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -758,17 +758,34 @@ end | |
atan(y::Real, x::Real) = atan(promote(float(y),float(x))...) | ||
atan(y::T, x::T) where {T<:AbstractFloat} = Base.no_op_err("atan", T) | ||
|
||
max(x::T, y::T) where {T<:AbstractFloat} = ifelse((y > x) | (signbit(y) < signbit(x)), | ||
ifelse(isnan(x), x, y), ifelse(isnan(y), y, x)) | ||
|
||
|
||
min(x::T, y::T) where {T<:AbstractFloat} = ifelse((y < x) | (signbit(y) > signbit(x)), | ||
ifelse(isnan(x), x, y), ifelse(isnan(y), y, x)) | ||
_isless(x::T, y::T) where {T<:AbstractFloat} = (x < y) || (signbit(x) > signbit(y)) | ||
min(x::T, y::T) where {T<:AbstractFloat} = isnan(x) || ~isnan(y) && _isless(x, y) ? x : y | ||
max(x::T, y::T) where {T<:AbstractFloat} = isnan(x) || ~isnan(y) && _isless(y, x) ? x : y | ||
minmax(x::T, y::T) where {T<:AbstractFloat} = min(x, y), max(x, y) | ||
|
||
_isless(x::Float16, y::Float16) = signbit(widen(x) - widen(y)) | ||
|
||
function min(x::T, y::T) where {T<:Union{Float32,Float64}} | ||
diff = x - y | ||
argmin = ifelse(signbit(diff), x, y) | ||
anynan = isnan(x)|isnan(y) | ||
ifelse(anynan, diff, argmin) | ||
end | ||
|
||
minmax(x::T, y::T) where {T<:AbstractFloat} = | ||
ifelse(isnan(x) | isnan(y), ifelse(isnan(x), (x,x), (y,y)), | ||
ifelse((y > x) | (signbit(x) > signbit(y)), (x,y), (y,x))) | ||
function max(x::T, y::T) where {T<:Union{Float32,Float64}} | ||
diff = x - y | ||
argmax = ifelse(signbit(diff), y, x) | ||
anynan = isnan(x)|isnan(y) | ||
ifelse(anynan, diff, argmax) | ||
end | ||
|
||
function minmax(x::T, y::T) where {T<:Union{Float32,Float64}} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this detailed definition necessary? It seems like a more generic There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My local bench does show some performance difference. julia> a = randn(1024); b = randn(1024); z = min.(a,b); zz = minmax.(a,b);
julia> using BenchmarkTools
[ Info: Precompiling BenchmarkTools [6e4b80f9-dd63-53aa-95a3-0cdb28fa8baf]
julia> @benchmark $zz .= minmax.($a, $b)
BenchmarkTools.Trial: 10000 samples with 198 evaluations.
Range (min … max): 440.404 ns … 1.189 μs ┊ GC (min … max): 0.00% … 0.00%
Time (median): 442.424 ns ┊ GC (median): 0.00%
Time (mean ± σ): 457.627 ns ± 45.207 ns ┊ GC (mean ± σ): 0.00% ± 0.00%
█
█▂▃▁▅▃▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▁
440 ns Histogram: frequency by time 702 ns <
Memory estimate: 0 bytes, allocs estimate: 0.
julia> f(x, y) = min(x, y), max(x, y)
f (generic function with 1 method)
julia> @benchmark $zz .= f.($a, $b)
BenchmarkTools.Trial: 10000 samples with 194 evaluations.
Range (min … max): 497.423 ns … 3.276 μs ┊ GC (min … max): 0.00% … 0.00%
Time (median): 498.969 ns ┊ GC (median): 0.00%
Time (mean ± σ): 515.506 ns ± 55.169 ns ┊ GC (mean ± σ): 0.00% ± 0.00%
█▄▂ ▆▅▄▂▁ ▁
██████████▇▆▆▅▄▃▆▆▆▅▄▅▄▄▅▃▅▅▃▃▃▂▄▃▃▃▄▃▄▄▄▄▄▄▄▄▄▄▄▅▄▅▄▅▄▅▄▄▃▄ █
497 ns Histogram: log(frequency) by time 759 ns <
Memory estimate: 0 bytes, allocs estimate: 0. Their LLVM IR only have order difference. I'm not sure why this matters though. |
||
diff = x - y | ||
sdiff = signbit(diff) | ||
min, max = ifelse(sdiff, x, y), ifelse(sdiff, y, x) | ||
anynan = isnan(x)|isnan(y) | ||
ifelse(anynan, diff, min), ifelse(anynan, diff, max) | ||
end | ||
|
||
""" | ||
ldexp(x, n) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the new
min
,max
,minmax
signatures be expanded to includeFloat16
? Could either add it to the list or use the definedIEEEFloat
union.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using
T<:IEEEFloat
everywhere that now usesT<:Union{Float32,Float64}
is helpful. Some systems have onchip support for Float16s, so this is a win there. The systems that emulate Float16s support min, max, minmax, so the processing is at worst unchanged there.