Skip to content

hardware independent isinf/isnan/isfinite #19008

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

Closed
wants to merge 3 commits into from
Closed

hardware independent isinf/isnan/isfinite #19008

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0ceff46
hardware indep. isinf for hardware floats
musm Oct 18, 2016
ed845a4
Add isnan functions
musm Oct 19, 2016
f02dd4e
Add hard indep isfinite
musm Oct 19, 2016
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
18 changes: 16 additions & 2 deletions base/float.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

## floating point traits ##

typealias IEEEFloat Union{Float16,Float32,Float64}

"""
Inf16

Expand Down Expand Up @@ -494,17 +496,29 @@ abs(x::Float64) = box(Float64,abs_float(unbox(Float64,x)))

Test whether a floating point number is not a number (NaN).
"""
isnan{T<:IEEEFloat}(x::T) = (reinterpret(Unsigned, x) & ~sign_mask(T)) > exponent_mask(T)
Copy link
Member

@vchuravy vchuravy Oct 19, 2016

Choose a reason for hiding this comment

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

Cool did not know reinterpret(Unsigned, x) is a thing.

Copy link
Member

@simonbyrne simonbyrne Oct 19, 2016

Choose a reason for hiding this comment

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

Is this more efficient than x != x?

Copy link
Contributor

@p-zubieta p-zubieta Oct 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

It is not

# x != x
julia> @code_native isnan(NaN)
    .text
Filename: float.jl
    pushq   %rbp
    movq    %rsp, %rbp
Source line: 355
    ucomisd %xmm0, %xmm0
    setp    %al
    popq    %rbp
    retq
    nopl    (%rax)

# This PR
julia> @code_native isnan2(NaN)
    .text
Filename: REPL[3]
    pushq   %rbp
    movq    %rsp, %rbp
Source line: 1
    movd    %xmm0, %rax
    movabsq $9223372036854775807, %rcx # imm = 0x7FFFFFFFFFFFFFFF
    andq    %rax, %rcx
    movabsq $9218868437227405312, %rax # imm = 0x7FF0000000000000
    cmpq    %rax, %rcx
    seta    %al
    popq    %rbp
    retq
    nopl    (%rax,%rax)

I don't think we should sacrifice losing this hardware instructions when available.

Copy link
Contributor Author

@musm musm Oct 19, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

See https://gist.github.com/musm/8150451b04120ef27974d8270dedbf6f
for more comprehensive tests (the gist is we are comparing performance in the 10-100 picosecond range...) So basically these instructions are so cheap and fast that it really doesn't matter on modern processors. In other words, the changes are just as fast as what we already have.

For isnan I agree, unfortunately we still require special handling for Float16 which prompted this pr, which I thought would be good to unify all the exceptional case handling to be independent of floating point hardware instructions so that Float16 (F128/F80) would be handled all the same. But the problem currently is that cpu support for small floats is not great (but I think now intel is investing more on this side due to demand).

I'll leave this to @vchuravy expertise on the issue on whether these proposed changes are positive or not, for now I am closing this.

For the denormals even though x - x will not work. It's only on the x87 chips where exceptional handling are very bad (some x87 sandybridge too?), where one may want to disable or flush them to zero.

But disabling denormals is not recommended, so the issue will probably never crop up; someone would have to go out of their way to disable them in Julia anyways.

So perhaps only for isinf and isfinite the changes are positive, but at that point might as well just use what we already have.

isnan(x::AbstractFloat) = x != x
isnan(x::Float16) = reinterpret(UInt16,x)&0x7fff > 0x7c00
isnan(x::Real) = false

"""
isfinite(f) -> Bool
Copy link
Contributor

@tkelman tkelman Oct 19, 2016

Choose a reason for hiding this comment

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

should this be removed from helpdb/Base.jl if it's also there and now duplicated?


Test whether a floating point number is finite, i.e. that `f` is neither infinite or not a number.
"""
isfinite{T<:IEEEFloat}(x::T) = (reinterpret(Unsigned, x) & exponent_mask(T)) != exponent_mask(T)
isfinite(x::AbstractFloat) = x - x == 0
isfinite(x::Float16) = reinterpret(UInt16,x)&0x7c00 != 0x7c00
isfinite(x::Real) = decompose(x)[3] != 0
isfinite(x::Integer) = true

"""
isinf(f) -> Bool

Test whether a floating point number is an infinity value (positive infinity or negative infinity).
"""
isinf{T<:IEEEFloat}(x::T) = (reinterpret(Unsigned, x) & ~sign_mask(T)) == exponent_mask(T)
Copy link
Member

@simonbyrne simonbyrne Oct 19, 2016

Choose a reason for hiding this comment

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

Again, is this more efficient?

isinf(x::Real) = !isnan(x) & !isfinite(x)


Copy link
Member

@vchuravy vchuravy Oct 19, 2016

Choose a reason for hiding this comment

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

unnecessary newline

## hashing small, built-in numeric types ##

hx(a::UInt64, b::Float64, h::UInt) = hash_uint64((3a + reinterpret(UInt64,b)) - h)
Expand Down