Skip to content
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

Don't use obsolete isinff and isnanf functions #174

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lantw44
Copy link

@lantw44 lantw44 commented Dec 1, 2019

GNU libc already marks isinff and isnanf functions as obsolete. They
have been replaced by isinf and isnan macros defined by C99. Since we
already requires C99 in this project, it should be nice if we can
switch to standard macros.

Proposed changes:

  • Drop the use of non-standard and obsolete isinff and isnanf functions.

Benchmark results:

  • None

Test suite changes:

  • None

GNU libc already marks isinff and isnanf functions as obsolete. They
have been replaced by isinf and isnan macros defined by C99. Since we
already requires C99 in this project, it should be nice if we can
switch to standard macros.
@lantw44
Copy link
Author

lantw44 commented Dec 1, 2019

It currently fails with GCC __builtin_isinf when -O1 or higher is used. I haven't found out why it fails. It works with Clang __builtin_isinf and FreeBSD libc. GNU libc just defines isinf as __builtin_isinf.

@ebassi
Copy link
Owner

ebassi commented Dec 3, 2019

I initially used isinf() and isnan(), but those ended up breaking when using GCC because it tried to promote floats to doubles, and the results wouldn't match any more—especially when using GCC vectorisation.

It could very well be a bug in GCC.

ebassi pushed a commit that referenced this pull request Jun 6, 2021
Avoids the #ifdef forest and code duplication resulting from it.
There was mismatch in the two code paths, see [1] and [2], and this
commit avoids repeating the same mistake.

While there, use isnan() instead of fpclassify() == FP_NAN for the case
where isnanf() isn't available.

We use isnanf() (if available) instead of isnan() due to [3]:

    I initially used isinf() and isnan(), but those ended up breaking
    when using GCC because it tried to promote floats to doubles, and
    the results wouldn't match any more—especially when using GCC
    vectorisation.
    It could very well be a bug in GCC.

[1] #223
[2] https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/3976
[3] #174 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants