[libc] Provide isnan, isnanf and isnanl functions #96008
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@llvm/pr-subscribers-libc Author: Petr Hosek (petrhosek) ChangesWhile C99 defines type generic isnan macro, BSD provided isnan, isnanf and isnanl in prior C standards and existing code still relies on these. Full diff: https://github.com/llvm/llvm-project/pull/96008.diff 13 Files Affected:
diff --git a/libc/config/baremetal/arm/entrypoints.txt b/libc/config/baremetal/arm/entrypoints.txt
index 7fb82c60a1bb8..9e6999fccc1ed 100644
--- a/libc/config/baremetal/arm/entrypoints.txt
+++ b/libc/config/baremetal/arm/entrypoints.txt
@@ -261,6 +261,9 @@ set(TARGET_LIBM_ENTRYPOINTS
libc.src.math.ilogb
libc.src.math.ilogbf
libc.src.math.ilogbl
+ libc.src.math.isnan
+ libc.src.math.isnanf
+ libc.src.math.isnanl
libc.src.math.ldexp
libc.src.math.ldexpf
libc.src.math.ldexpl
diff --git a/libc/config/baremetal/riscv/entrypoints.txt b/libc/config/baremetal/riscv/entrypoints.txt
index b769b43f03a2c..dffc6af88929e 100644
--- a/libc/config/baremetal/riscv/entrypoints.txt
+++ b/libc/config/baremetal/riscv/entrypoints.txt
@@ -260,6 +260,9 @@ set(TARGET_LIBM_ENTRYPOINTS
libc.src.math.ilogb
libc.src.math.ilogbf
libc.src.math.ilogbl
+ libc.src.math.isnan
+ libc.src.math.isnanf
+ libc.src.math.isnanl
libc.src.math.ldexp
libc.src.math.ldexpf
libc.src.math.ldexpl
diff --git a/libc/include/llvm-libc-macros/math-macros.h b/libc/include/llvm-libc-macros/math-macros.h
index 47838969d59ae..30e826e769306 100644
--- a/libc/include/llvm-libc-macros/math-macros.h
+++ b/libc/include/llvm-libc-macros/math-macros.h
@@ -54,6 +54,5 @@
// TODO: Move generic functional math macros to a separate header file.
#define isfinite(x) __builtin_isfinite(x)
#define isinf(x) __builtin_isinf(x)
-#define isnan(x) __builtin_isnan(x)
#endif // LLVM_LIBC_MACROS_MATH_MACROS_H
diff --git a/libc/include/math.h.def b/libc/include/math.h.def
index 454b8f2980514..3ec52e77856d4 100644
--- a/libc/include/math.h.def
+++ b/libc/include/math.h.def
@@ -17,4 +17,6 @@
%%public_api()
+#define isnan(x) __builtin_isnan(x)
+
#endif // LLVM_LIBC_MATH_H
diff --git a/libc/spec/bsd_ext.td b/libc/spec/bsd_ext.td
index 50ca8b919ff2c..4d33313521735 100644
--- a/libc/spec/bsd_ext.td
+++ b/libc/spec/bsd_ext.td
@@ -1,4 +1,16 @@
def BsdExtensions : StandardSpec<"BSDExtensions"> {
+ HeaderSpec Math = HeaderSpec<
+ "math.h",
+ [], // Macros
+ [], // Types
+ [], // Enumerations
+ [
+ FunctionSpec<"isnan", RetValSpec<IntType>, [ArgSpec<DoubleType>]>,
+ FunctionSpec<"isnanf", RetValSpec<IntType>, [ArgSpec<FloatType>]>,
+ FunctionSpec<"isnanl", RetValSpec<IntType>, [ArgSpec<LongDoubleType>]>,
+ ]
+ >;
+
HeaderSpec String = HeaderSpec<
"string.h",
[], // Macros
@@ -67,6 +79,7 @@ def BsdExtensions : StandardSpec<"BSDExtensions"> {
>;
let Headers = [
+ Math,
String,
Strings,
SysWait,
diff --git a/libc/src/math/CMakeLists.txt b/libc/src/math/CMakeLists.txt
index 4472367d6c073..ca833f3a872f4 100644
--- a/libc/src/math/CMakeLists.txt
+++ b/libc/src/math/CMakeLists.txt
@@ -219,6 +219,10 @@ add_math_entrypoint_object(ilogbl)
add_math_entrypoint_object(ilogbf16)
add_math_entrypoint_object(ilogbf128)
+add_math_entrypoint_object(isnan)
+add_math_entrypoint_object(isnanf)
+add_math_entrypoint_object(isnanl)
+
add_math_entrypoint_object(llogb)
add_math_entrypoint_object(llogbf)
add_math_entrypoint_object(llogbl)
diff --git a/libc/src/math/generic/CMakeLists.txt b/libc/src/math/generic/CMakeLists.txt
index aa0069d821d0d..9d99879488be5 100644
--- a/libc/src/math/generic/CMakeLists.txt
+++ b/libc/src/math/generic/CMakeLists.txt
@@ -2684,6 +2684,36 @@ add_entrypoint_object(
-O3
)
+add_entrypoint_object(
+ isnan
+ SRCS
+ isnan.cpp
+ HDRS
+ ../isnan.h
+ COMPILE_OPTIONS
+ -O3
+)
+
+add_entrypoint_object(
+ isnanf
+ SRCS
+ isnanf.cpp
+ HDRS
+ ../isnanf.h
+ COMPILE_OPTIONS
+ -O3
+)
+
+add_entrypoint_object(
+ isnanl
+ SRCS
+ isnanl.cpp
+ HDRS
+ ../isnanl.h
+ COMPILE_OPTIONS
+ -O3
+)
+
add_entrypoint_object(
nan
SRCS
diff --git a/libc/src/math/generic/isnan.cpp b/libc/src/math/generic/isnan.cpp
new file mode 100644
index 0000000000000..5406a69f68217
--- /dev/null
+++ b/libc/src/math/generic/isnan.cpp
@@ -0,0 +1,18 @@
+//===-- Implementation of isnan function ----------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/math/isnan.h"
+#include "src/__support/common.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(int, isnan, (double x)) {
+ return __builtin_isnan(x);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/math/generic/isnanf.cpp b/libc/src/math/generic/isnanf.cpp
new file mode 100644
index 0000000000000..2d9339360713b
--- /dev/null
+++ b/libc/src/math/generic/isnanf.cpp
@@ -0,0 +1,18 @@
+//===-- Implementation of isnanf function ---------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/math/isnanf.h"
+#include "src/__support/common.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(int, isnanf, (float x)) {
+ return __builtin_isnan(x);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/math/generic/isnanl.cpp b/libc/src/math/generic/isnanl.cpp
new file mode 100644
index 0000000000000..a700b636d0149
--- /dev/null
+++ b/libc/src/math/generic/isnanl.cpp
@@ -0,0 +1,18 @@
+//===-- Implementation of isnanl function ---------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "src/math/isnanl.h"
+#include "src/__support/common.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(int, isnanl, (long double x)) {
+ return __builtin_isnan(x);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/math/isnan.h b/libc/src/math/isnan.h
new file mode 100644
index 0000000000000..eda8e7eb30f39
--- /dev/null
+++ b/libc/src/math/isnan.h
@@ -0,0 +1,18 @@
+//===-- Implementation header for isnan -------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_MATH_ISNAN_H
+#define LLVM_LIBC_SRC_MATH_ISNAN_H
+
+namespace LIBC_NAMESPACE {
+
+int isnan(double x);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_MATH_ISNAN_H
diff --git a/libc/src/math/isnanf.h b/libc/src/math/isnanf.h
new file mode 100644
index 0000000000000..a12d39ee5af97
--- /dev/null
+++ b/libc/src/math/isnanf.h
@@ -0,0 +1,18 @@
+//===-- Implementation header for isnanf ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_MATH_ISNANF_H
+#define LLVM_LIBC_SRC_MATH_ISNANF_H
+
+namespace LIBC_NAMESPACE {
+
+int isnanf(float x);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_MATH_ISNANF_H
diff --git a/libc/src/math/isnanl.h b/libc/src/math/isnanl.h
new file mode 100644
index 0000000000000..9fbfca03cb15e
--- /dev/null
+++ b/libc/src/math/isnanl.h
@@ -0,0 +1,18 @@
+//===-- Implementation header for isnanl ------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_LIBC_SRC_MATH_ISNANL_H
+#define LLVM_LIBC_SRC_MATH_ISNANL_H
+
+namespace LIBC_NAMESPACE {
+
+int isnanl(long double x);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_MATH_ISNANL_H
|
petrhosek
commented
Jun 18, 2024
libc/include/math.h.def
Outdated
| @@ -17,4 +17,6 @@ | |||
|
|
|||
| %%public_api() | |||
|
|
|||
| #define isnan(x) __builtin_isnan(x) | |||
There was a problem hiding this comment.
Should we put this macro into its own header? Do you have any suggestion for the name?
There was a problem hiding this comment.
If possible I'd like to leave this in math-macros but if it needs to be moved out then I'd say it should be defined in isnan-macro.h inside include/llvm-libc-macros/
There was a problem hiding this comment.
Do we expect every consumer of this header to have this builtin? We could possible do if defined(__has_builtin) && __has_builtin(__builtin_isnan)) or omething.
There was a problem hiding this comment.
Does it mean that we still define isnan macro after defining all the isnan function?
If so then I think it makes sense to do the TODO in math-macros.h, that is to move isinf, isfinite, isnan macro definitions to math-function-macros.h, including it after %%public_api in here, and commenting about the reason we have to include it after all the function definitions.
There was a problem hiding this comment.
Do we expect every consumer of this header to have this builtin?
Yes. All versions of clang and gcc that we claim to support have this builtin. Therefor, we should not guard it with __has_builtin (same for other builtins).
There was a problem hiding this comment.
that is to move isinf, isfinite, isnan
So isnan has isnan, isnanf, and isnanl.
isinf has isinf, isinff, isinfl,
finite has finite, finitef, finitel. Note that finite is not isfinite{f|l}, so it lacks the symmetry that isinf and isnan have. Stated another way, isfinite (the type-generic macro) does not conflict with the finite, finitef, and finitel functions (like isnan and isinf do).
There was a problem hiding this comment.
For longer term maintainability I'd like to go with a design like what @lntue proposed where the primary math macros are included before %%public_api and the macros that might also be functions are defined in a separate header after.
nickdesaulniers
requested changes
Jun 20, 2024
There was a problem hiding this comment.
isnanf and isnanl LGTM.
But I think we may need to be more careful here with isnan.
isnan is generally a macro or a function? That needs to be explored more carefully I think.
cc @enh-google @frobtech for folks that might understand this problem better.
a macro, because realistically you want this to just be an inlined instruction or two via (callers that want the function [on systems where it exists] need to use the |
|
C standard introduced |
|
Got it, thanks for the context @enh-google . In that case, @petrhosek please add tests, but for isnan it sounds like we should test the function AND the macro? |
| @@ -260,6 +260,9 @@ set(TARGET_LIBM_ENTRYPOINTS | |||
| libc.src.math.ilogb | |||
| libc.src.math.ilogbf | |||
| libc.src.math.ilogbl | |||
| libc.src.math.isnan | |||
| libc.src.math.isnanf | |||
| libc.src.math.isnanl | |||
There was a problem hiding this comment.
We don't test baremetal continuously; please add these entrypoints to at least libc/config/linux/x86_64/entrypoints.txt, if not the rest of the linux targets.
strictly, i don't think any of them (except bionic, for
|
|
(for completeness, i looked at FreeBSD/NetBSD/OpenBSD, and see that -- unsurprisingly, like macOS -- FreeBSD doesn't declare any of these functions, but NetBSD+OpenBSD both have declarations for isnanf()/isinff() [but not isnanl()/isinfl()].) |
|
and glibc does have all the declarations --- it just hides them behind macros that declare functions :-) |
hubot
pushed a commit
to aosp-mirror/platform_bionic
that referenced
this pull request
Jun 21, 2024
isnan() and isinf() are type-generic macros, not usually declared as functions. Remove the function declarations and the big comment talking about how we shouldn't expose them. isnanl() and isinfl() are exposed as functions by glibc (but not the BSDs), and we've always had the definitions, so we can increase source compatibility there trivially. See the discussion on llvm/llvm-project#96008 for more background. Change-Id: I798153f710794870bb4864f2ae8b98147f9d8763
While C99 defines type generic isnan macro, BSD provided isnan, isnanf and isnanl in prior C standards and existing code still relies on these.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
lntue
approved these changes
Jul 3, 2024
|
Please fix formatting. |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/71/builds/1776 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/131/builds/1744 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/179/builds/1742 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/171/builds/1730 Here is the relevant piece of the build log for the reference: |
petrhosek
added a commit
to petrhosek/llvm-project
that referenced
this pull request
Jul 10, 2024
This was accidentally omitted in llvm#96008.
petrhosek
added a commit
that referenced
this pull request
Jul 10, 2024
This was accidentally omitted in #96008.
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
While C99 defines type generic isnan macro, BSD provided isnan, isnanf and isnanl in prior C standards and existing code still relies on these.
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
This was accidentally omitted in llvm#96008.
tejas101k
pushed a commit
to ProjectInfinity-X/bionic
that referenced
this pull request
Dec 14, 2024
isnan() and isinf() are type-generic macros, not usually declared as functions. Remove the function declarations and the big comment talking about how we shouldn't expose them. isnanl() and isinfl() are exposed as functions by glibc (but not the BSDs), and we've always had the definitions, so we can increase source compatibility there trivially. See the discussion on llvm/llvm-project#96008 for more background. Change-Id: I798153f710794870bb4864f2ae8b98147f9d8763
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While C99 defines type generic isnan macro, BSD provided isnan, isnanf and isnanl in prior C standards and existing code still relies on these.