[libc][math][c23] add c23 floating point fmaximum and fminimum functions. #86016
Conversation
|
@llvm/pr-subscribers-backend-amdgpu @llvm/pr-subscribers-libc Author: Job Henandez Lara (Jobhdez) ChangesHello, I have added fminimum and fmaximum functions. I built it using I ran the tests as follows: Please let me know of anything I need to change. I will be available. Thanks. Patch is 161.81 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86016.diff 141 Files Affected:
diff --git a/libc/config/linux/aarch64/entrypoints.txt b/libc/config/linux/aarch64/entrypoints.txt
index 43c9e81f17833e..56fb490104f67b 100644
--- a/libc/config/linux/aarch64/entrypoints.txt
+++ b/libc/config/linux/aarch64/entrypoints.txt
@@ -363,6 +363,30 @@ set(TARGET_LIBM_ENTRYPOINTS
libc.src.math.fmin
libc.src.math.fminf
libc.src.math.fminl
+ libc.src.math.fmaximum
+ libc.src.math.fmaximumf
+ libc.src.math.fmaximuml
+ libc.src.math.fmaximum_num
+ libc.src.math.fmaximum_numf
+ libc.src.math.fmaximum_numl
+ libc.src.math.fmaximum_mag
+ libc.src.math.fmaximum_magf
+ libc.src.math.fmaximum_magl
+ libc.src.math.fmaximum_mag_num
+ libc.src.math.fmaximum_mag_numf
+ libc.src.math.fmaximum_mag_numl
+ libc.src.math.fminimum
+ libc.src.math.fminimumf
+ libc.src.math.fminimuml
+ libc.src.math.fminimum_num
+ libc.src.math.fminimum_numf
+ libc.src.math.fminimum_numl
+ libc.src.math.fminimum_mag
+ libc.src.math.fminimum_magf
+ libc.src.math.fminimum_magl
+ libc.src.math.fminimum_mag_num
+ libc.src.math.fminimum_mag_numf
+ libc.src.math.fminimum_mag_numl
libc.src.math.fmod
libc.src.math.fmodf
libc.src.math.fmodl
diff --git a/libc/config/linux/arm/entrypoints.txt b/libc/config/linux/arm/entrypoints.txt
index bf1559b2f02369..70a3ed55a69a29 100644
--- a/libc/config/linux/arm/entrypoints.txt
+++ b/libc/config/linux/arm/entrypoints.txt
@@ -234,6 +234,30 @@ set(TARGET_LIBM_ENTRYPOINTS
libc.src.math.fmin
libc.src.math.fminf
libc.src.math.fminl
+ libc.src.math.fmaximum
+ libc.src.math.fmaximumf
+ libc.src.math.fmaximuml
+ libc.src.math.fmaximum_num
+ libc.src.math.fmaximum_numf
+ libc.src.math.fmaximum_numl
+ libc.src.math.fmaximum_mag
+ libc.src.math.fmaximum_magf
+ libc.src.math.fmaximum_magl
+ libc.src.math.fmaximum_mag_num
+ libc.src.math.fmaximum_mag_numf
+ libc.src.math.fmaximum_mag_numl
+ libc.src.math.fminimum
+ libc.src.math.fminimumf
+ libc.src.math.fminimuml
+ libc.src.math.fminimum_num
+ libc.src.math.fminimum_numf
+ libc.src.math.fminimum_numl
+ libc.src.math.fminimum_mag
+ libc.src.math.fminimum_magf
+ libc.src.math.fminimum_magl
+ libc.src.math.fminimum_mag_num
+ libc.src.math.fminimum_mag_numf
+ libc.src.math.fminimum_mag_numl
libc.src.math.fmod
libc.src.math.fmodf
libc.src.math.frexp
diff --git a/libc/config/linux/riscv/entrypoints.txt b/libc/config/linux/riscv/entrypoints.txt
index 99ef84d3f73974..456baff250e101 100644
--- a/libc/config/linux/riscv/entrypoints.txt
+++ b/libc/config/linux/riscv/entrypoints.txt
@@ -371,6 +371,30 @@ set(TARGET_LIBM_ENTRYPOINTS
libc.src.math.fmax
libc.src.math.fmaxf
libc.src.math.fmaxl
+ libc.src.math.fmaximum
+ libc.src.math.fmaximumf
+ libc.src.math.fmaximuml
+ libc.src.math.fmaximum_num
+ libc.src.math.fmaximum_numf
+ libc.src.math.fmaximum_numl
+ libc.src.math.fmaximum_mag
+ libc.src.math.fmaximum_magf
+ libc.src.math.fmaximum_magl
+ libc.src.math.fmaximum_mag_num
+ libc.src.math.fmaximum_mag_numf
+ libc.src.math.fmaximum_mag_numl
+ libc.src.math.fminimum
+ libc.src.math.fminimumf
+ libc.src.math.fminimuml
+ libc.src.math.fminimum_num
+ libc.src.math.fminimum_numf
+ libc.src.math.fminimum_numl
+ libc.src.math.fminimum_mag
+ libc.src.math.fminimum_magf
+ libc.src.math.fminimum_magl
+ libc.src.math.fminimum_mag_num
+ libc.src.math.fminimum_mag_numf
+ libc.src.math.fminimum_mag_numl
libc.src.math.fmod
libc.src.math.fmodf
libc.src.math.fmodl
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index 99182e7f92ac09..65f95a92a538ff 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -374,6 +374,30 @@ set(TARGET_LIBM_ENTRYPOINTS
libc.src.math.fmax
libc.src.math.fmaxf
libc.src.math.fmaxl
+ libc.src.math.fmaximum
+ libc.src.math.fmaximumf
+ libc.src.math.fmaximuml
+ libc.src.math.fmaximum_num
+ libc.src.math.fmaximum_numf
+ libc.src.math.fmaximum_numl
+ libc.src.math.fmaximum_mag
+ libc.src.math.fmaximum_magf
+ libc.src.math.fmaximum_magl
+ libc.src.math.fmaximum_mag_num
+ libc.src.math.fmaximum_mag_numf
+ libc.src.math.fmaximum_mag_numl
+ libc.src.math.fminimum
+ libc.src.math.fminimumf
+ libc.src.math.fminimuml
+ libc.src.math.fminimum_num
+ libc.src.math.fminimum_numf
+ libc.src.math.fminimum_numl
+ libc.src.math.fminimum_mag
+ libc.src.math.fminimum_magf
+ libc.src.math.fminimum_magl
+ libc.src.math.fminimum_mag_num
+ libc.src.math.fminimum_mag_numf
+ libc.src.math.fminimum_mag_numl
libc.src.math.fmod
libc.src.math.fmodf
libc.src.math.fmodl
diff --git a/libc/config/windows/entrypoints.txt b/libc/config/windows/entrypoints.txt
index d6227a427afe2b..1a1d92b1f2afe7 100644
--- a/libc/config/windows/entrypoints.txt
+++ b/libc/config/windows/entrypoints.txt
@@ -1,4 +1,4 @@
-set(TARGET_LIBC_ENTRYPOINTS
+fset(TARGET_LIBC_ENTRYPOINTS
# ctype.h entrypoints
libc.src.ctype.isalnum
libc.src.ctype.isalpha
@@ -153,6 +153,30 @@ set(TARGET_LIBM_ENTRYPOINTS
libc.src.math.fmax
libc.src.math.fmaxf
libc.src.math.fmaxl
+ libc.src.math.fmaximum
+ libc.src.math.fmaximumf
+ libc.src.math.fmaximuml
+ libc.src.math.fmaximum_num
+ libc.src.math.fmaximum_numf
+ libc.src.math.fmaximum_numl
+ libc.src.math.fmaximum_mag
+ libc.src.math.fmaximum_magf
+ libc.src.math.fmaximum_magl
+ libc.src.math.fmaximum_mag_num
+ libc.src.math.fmaximum_mag_numf
+ libc.src.math.fmaximum_mag_numl
+ libc.src.math.fminimum
+ libc.src.math.fminimumf
+ libc.src.math.fminimuml
+ libc.src.math.fminimum_num
+ libc.src.math.fminimum_numf
+ libc.src.math.fminimum_numl
+ libc.src.math.fminimum_mag
+ libc.src.math.fminimum_magf
+ libc.src.math.fminimum_magl
+ libc.src.math.fminimum_mag_num
+ libc.src.math.fminimum_mag_numf
+ libc.src.math.fminimum_mag_numl
libc.src.math.fmod
libc.src.math.fmodf
libc.src.math.fmodl
diff --git a/libc/spec/stdc.td b/libc/spec/stdc.td
index 84d28cc3350304..f35cf06a12ca0d 100644
--- a/libc/spec/stdc.td
+++ b/libc/spec/stdc.td
@@ -400,6 +400,46 @@ def StdC : StandardSpec<"stdc"> {
FunctionSpec<"fmaxf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
FunctionSpec<"fmaxl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
GuardedFunctionSpec<"fmaxf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT128">,
+
+ FunctionSpec<"fmaximum", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
+ FunctionSpec<"fmaximumf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
+ FunctionSpec<"fmaximuml", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
+ GuardedFunctionSpec<"fmaximumf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT128">,
+
+ FunctionSpec<"fmaximum_num", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
+ FunctionSpec<"fmaximum_numf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
+ FunctionSpec<"fmaximum_numl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
+ GuardedFunctionSpec<"fmaximum_numf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT128">,
+
+ FunctionSpec<"fmaximum_mag", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
+ FunctionSpec<"fmaximum_magf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
+ FunctionSpec<"fmaximum_magl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
+ GuardedFunctionSpec<"fmaximum_magf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT128">,
+
+ FunctionSpec<"fmaximum_mag_num", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
+ FunctionSpec<"fmaximum_mag_numf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
+ FunctionSpec<"fmaximum_mag_numl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
+ GuardedFunctionSpec<"fmaximum_mag_numf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT128">,
+
+ FunctionSpec<"fminimum", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
+ FunctionSpec<"fminimumf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
+ FunctionSpec<"fminimuml", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
+ GuardedFunctionSpec<"fminimumf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT128">,
+
+ FunctionSpec<"fminimum_num", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
+ FunctionSpec<"fminimum_numf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
+ FunctionSpec<"fmaximum_numl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
+ GuardedFunctionSpec<"fminimum_numf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT128">,
+
+ FunctionSpec<"fminimum_mag", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
+ FunctionSpec<"fminimum_magf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
+ FunctionSpec<"fminimum_magl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
+ GuardedFunctionSpec<"fminimum_magf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT128">,
+
+ FunctionSpec<"fminimum_mag_num", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
+ FunctionSpec<"fminimum_mag_numf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>]>,
+ FunctionSpec<"fminimum_mag_numl", RetValSpec<LongDoubleType>, [ArgSpec<LongDoubleType>, ArgSpec<LongDoubleType>]>,
+ GuardedFunctionSpec<"fminimum_mag_numf128", RetValSpec<Float128Type>, [ArgSpec<Float128Type>, ArgSpec<Float128Type>], "LIBC_TYPES_HAS_FLOAT128">,
FunctionSpec<"fma", RetValSpec<DoubleType>, [ArgSpec<DoubleType>, ArgSpec<DoubleType>, ArgSpec<DoubleType>]>,
FunctionSpec<"fmaf", RetValSpec<FloatType>, [ArgSpec<FloatType>, ArgSpec<FloatType>, ArgSpec<FloatType>]>,
diff --git a/libc/src/__support/FPUtil/BasicOperations.h b/libc/src/__support/FPUtil/BasicOperations.h
index a19d6d0bef08ff..de04f6d0a353b5 100644
--- a/libc/src/__support/FPUtil/BasicOperations.h
+++ b/libc/src/__support/FPUtil/BasicOperations.h
@@ -58,6 +58,130 @@ LIBC_INLINE T fmax(T x, T y) {
}
}
+template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
+LIBC_INLINE T fmaximum(T x, T y) {
+ FPBits<T> bitx(x), bity(y);
+
+ if (bitx.is_nan()) {
+ return x;
+ } else if (bity.is_nan()) {
+ return y;
+ } else if (bitx.sign() != bity.sign()) {
+ // To make sure that fmax(+0, -0) == +0 == fmax(-0, +0), whenever x and
+ // y has different signs and both are not NaNs, we return the number
+ // with positive sign.
+ return (bitx.is_neg() ? y : x);
+ } else {
+ return (x > y ? x : y);
+ }
+}
+
+template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
+LIBC_INLINE T fminimum(T x, T y) {
+ const FPBits<T> bitx(x), bity(y);
+
+ if (bitx.is_nan()) {
+ return x;
+ } else if (bity.is_nan()) {
+ return y;
+ } else if (bitx.sign() != bity.sign()) {
+ // To make sure that fmin(+0, -0) == -0 == fmin(-0, +0), whenever x and
+ // y has different signs and both are not NaNs, we return the number
+ // with negative sign.
+ return (bitx.is_neg()) ? x : y;
+ } else {
+ return (x < y ? x : y);
+ }
+}
+
+template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
+LIBC_INLINE T fmaximum_num(T x, T y) {
+ FPBits<T> bitx(x), bity(y);
+
+ if (bitx.is_nan()) {
+ return y;
+ } else if (bity.is_nan()) {
+ return x;
+ } else if (bitx.sign() != bity.sign()) {
+ // To make sure that fmax(+0, -0) == +0 == fmax(-0, +0), whenever x and
+ // y has different signs and both are not NaNs, we return the number
+ // with positive sign.
+ return (bitx.is_neg() ? y : x);
+ } else {
+ return (x > y ? x : y);
+ }
+}
+
+template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
+LIBC_INLINE T fminimum_num(T x, T y) {
+ const FPBits<T> bitx(x), bity(y);
+
+ if (bitx.is_nan()) {
+ return y;
+ } else if (bity.is_nan()) {
+ return x;
+ } else if (bitx.sign() != bity.sign()) {
+ // To make sure that fmin(+0, -0) == -0 == fmin(-0, +0), whenever x and
+ // y has different signs and both are not NaNs, we return the number
+ // with negative sign.
+ return (bitx.is_neg()) ? x : y;
+ } else {
+ return (x < y ? x : y);
+ }
+}
+
+template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
+LIBC_INLINE T fmaximum_mag(T x, T y) {
+ FPBits<T> bitx(x), bity(y);
+
+ if (abs(x) > abs(y)) {
+ return x;
+ } else if (abs(y) > abs(x)) {
+ return y;
+ } else {
+ return fmaximum(x, y);
+ }
+}
+
+template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
+LIBC_INLINE T fminimum_mag(T x, T y) {
+ FPBits<T> bitx(x), bity(y);
+
+ if (abs(x) < abs(y)) {
+ return x;
+ } else if (abs(y) < abs(x)) {
+ return y;
+ } else {
+ return fminimum(x, y);
+ }
+}
+
+template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
+LIBC_INLINE T fmaximum_mag_num(T x, T y) {
+ FPBits<T> bitx(x), bity(y);
+
+ if (abs(x) > abs(y)) {
+ return x;
+ } else if (abs(y) > abs(x)) {
+ return y;
+ } else {
+ return fmaximum_num(x, y);
+ }
+}
+
+template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
+LIBC_INLINE T fminimum_mag_num(T x, T y) {
+ FPBits<T> bitx(x), bity(y);
+
+ if (abs(x) < abs(y)) {
+ return x;
+ } else if (abs(y) < abs(x)) {
+ return y;
+ } else {
+ return fminimum_num(x, y);
+ }
+}
+
template <typename T, cpp::enable_if_t<cpp::is_floating_point_v<T>, int> = 0>
LIBC_INLINE T fdim(T x, T y) {
FPBits<T> bitx(x), bity(y);
diff --git a/libc/src/math/CMakeLists.txt b/libc/src/math/CMakeLists.txt
index 5e2e6e699d0e0c..bdfd2a51346307 100644
--- a/libc/src/math/CMakeLists.txt
+++ b/libc/src/math/CMakeLists.txt
@@ -117,6 +117,46 @@ add_math_entrypoint_object(fminf)
add_math_entrypoint_object(fminl)
add_math_entrypoint_object(fminf128)
+add_math_entrypoint_object(fmaximum)
+add_math_entrypoint_object(fmaximumf)
+add_math_entrypoint_object(fmaximuml)
+add_math_entrypoint_object(fmaximumf128)
+
+add_math_entrypoint_object(fmaximum_num)
+add_math_entrypoint_object(fmaximum_numf)
+add_math_entrypoint_object(fmaximum_numl)
+add_math_entrypoint_object(fmaximum_numf128)
+
+add_math_entrypoint_object(fmaximum_mag)
+add_math_entrypoint_object(fmaximum_magf)
+add_math_entrypoint_object(fmaximum_magl)
+add_math_entrypoint_object(fmaximum_magf128)
+
+add_math_entrypoint_object(fmaximum_mag_num)
+add_math_entrypoint_object(fmaximum_mag_numf)
+add_math_entrypoint_object(fmaximum_mag_numl)
+add_math_entrypoint_object(fmaximum_mag_numf128)
+
+add_math_entrypoint_object(fminimum)
+add_math_entrypoint_object(fminimumf)
+add_math_entrypoint_object(fminimuml)
+add_math_entrypoint_object(fminimumf128)
+
+add_math_entrypoint_object(fminimum_num)
+add_math_entrypoint_object(fminimum_numf)
+add_math_entrypoint_object(fminimum_numl)
+add_math_entrypoint_object(fminimum_numf128)
+
+add_math_entrypoint_object(fminimum_mag)
+add_math_entrypoint_object(fminimum_magf)
+add_math_entrypoint_object(fminimum_magl)
+add_math_entrypoint_object(fminimum_magf128)
+
+add_math_entrypoint_object(fminimum_mag_num)
+add_math_entrypoint_object(fminimum_mag_numf)
+add_math_entrypoint_object(fminimum_mag_numl)
+add_math_entrypoint_object(fminimum_mag_numf128)
+
add_math_entrypoint_object(fmod)
add_math_entrypoint_object(fmodf)
add_math_entrypoint_object(fmodl)
diff --git a/libc/src/math/amdgpu/fmaximum.cpp b/libc/src/math/amdgpu/fmaximum.cpp
new file mode 100644
index 00000000000000..a121feb79addce
--- /dev/null
+++ b/libc/src/math/amdgpu/fmaximum.cpp
@@ -0,0 +1,21 @@
+//===-- Implementation of the fmaximum function for GPU -----------------------===//
+//
+// 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/fmaximum.h"
+
+#include "src/__support/CPP/bit.h"
+#include "src/__support/common.h"
+#include "src/__support/macros/optimization.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(double, fmaximum, (double x, double y)) {
+ return __builtin_fmaximum(x, y);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/math/amdgpu/fmaximum_mag.cpp b/libc/src/math/amdgpu/fmaximum_mag.cpp
new file mode 100644
index 00000000000000..99e277f108e02e
--- /dev/null
+++ b/libc/src/math/amdgpu/fmaximum_mag.cpp
@@ -0,0 +1,21 @@
+//===-- Implementation of the fmaximum_mag function for GPU -----------------------===//
+//
+// 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/fmaximum_mag.h"
+
+#include "src/__support/CPP/bit.h"
+#include "src/__support/common.h"
+#include "src/__support/macros/optimization.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(double, fmaximum_mag, (double x, double y)) {
+ return __builtin_fmaximum_mag(x, y);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/math/amdgpu/fmaximum_mag_num.cpp b/libc/src/math/amdgpu/fmaximum_mag_num.cpp
new file mode 100644
index 00000000000000..bde427903ad60b
--- /dev/null
+++ b/libc/src/math/amdgpu/fmaximum_mag_num.cpp
@@ -0,0 +1,21 @@
+//===-- Implementation of the fmaximum_mag_num function for GPU -----------------------===//
+//
+// 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/fmaximum_mag_num.h"
+
+#include "src/__support/CPP/bit.h"
+#include "src/__support/common.h"
+#include "src/__support/macros/optimization.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(double, fmaximum_mag_num, (double x, double y)) {
+ return __builtin_fmaximum_mag_num(x, y);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/math/amdgpu/fmaximum_mag_numf.cpp b/libc/src/math/amdgpu/fmaximum_mag_numf.cpp
new file mode 100644
index 00000000000000..52f7d1413a1cc2
--- /dev/null
+++ b/libc/src/math/amdgpu/fmaximum_mag_numf.cpp
@@ -0,0 +1,21 @@
+//===-- Implementation of the fmaximum_mag_numf function for GPU -----------------------===//
+//
+// 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/fmaximum_mag_numf.h"
+
+#include "src/__support/CPP/bit.h"
+#include "src/__support/common.h"
+#include "src/__support/macros/optimization.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(float, fmaximum_mag_numf, (float x, float y)) {
+ return __builtin_fmaximum_mag_num(x, y);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/math/amdgpu/fmaximum_mag_numl.cpp b/libc/sr...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
Thanks for the patch! Try |
libc/config/windows/entrypoints.txt
Outdated
| @@ -1,4 +1,4 @@ | |||
| set(TARGET_LIBC_ENTRYPOINTS | |||
| fset(TARGET_LIBC_ENTRYPOINTS | |||
Do I just do just that. Do I Have to amend the commit? |
|
|
||
| if (bitx.is_nan()) { | ||
| return x; | ||
| } else if (bity.is_nan()) { |
There was a problem hiding this comment.
Please use the form:
if (bitx. ...)
return ...
if (bity. ...)
return
if (bitx.sign ...)
return
return x > y ? x : y;
Here and below. When the body of the if statement returns, there's no need for the else.
There was a problem hiding this comment.
ok I will fix it. is it ok after I make my changes to do
$ git add
$ git commit --amend
$ git push --force
So I can keep it at two commits?
There was a problem hiding this comment.
It's ok for you to keep multiple commits, don't need to --amend.
I'd just commit the result and push it so that you have more than one commit. |
|
There was an open bug for this, yeah? If you put |
| LIBC_INLINE T fmaximum(T x, T y) { | ||
| FPBits<T> bitx(x), bity(y); | ||
|
|
||
| if (bitx.is_nan()) { |
There was a problem hiding this comment.
nit: You don't need else here, since you always return if the condition is true.
This could be
if(foo)
return foo;
if(bar)
return bar;
...
libc/config/windows/entrypoints.txt
Outdated
| @@ -1,4 +1,4 @@ | |||
| set(TARGET_LIBC_ENTRYPOINTS | |||
| fset(TARGET_LIBC_ENTRYPOINTS | |||
There was a problem hiding this comment.
I think you may have had a typo adding an f here.
libc/src/math/amdgpu/fmaximum.cpp
Outdated
| namespace LIBC_NAMESPACE { | ||
|
|
||
| LLVM_LIBC_FUNCTION(double, fmaximum, (double x, double y)) { | ||
| return __builtin_fmaximum(x, y); |
There was a problem hiding this comment.
do the GPUs need these builtins right now or would it be easier to add this support later and just add the generic implementations for now. This patch is already rather large so shrinking the scope would be helpful for review.
There was a problem hiding this comment.
Let me know. If these need to be removed I will remove them.
There was a problem hiding this comment.
Let's remove all the specialization in src/math/amdgpu folder for now.
There was a problem hiding this comment.
@lntue so I need to delete the fmaximum.cpp and so on functions in this directory?
It’s for this issue here #85496 |
I just did that and the checks failed again. |
libc/src/math/amdgpu/fmaximum.cpp
Outdated
| //===-- Implementation of the fmaximum function for GPU | ||
| //-----------------------===// |
There was a problem hiding this comment.
Make it fit in 1 line, and exact 80-char. Same with other files.
There was a problem hiding this comment.
same with other files in the directory amdgpu? or everywhere else?
There was a problem hiding this comment.
Everywhere, the headers should all look like the other ones.
|
You should also try with full build mode: start with a clean build folder: And you should be able to test them individually: |
lntue
changed the title
|
tag @jhuber6 for AMDGPU builtins, building & testing. |
|
@lntue I will address your reviews; thanks! |
|
@arsenm I assume you mean for the GPU implementation? The generic implementation looks correct already since we have to support both clang and GCC. |
So what's happening is that the comment on the first line of a few files (see report) is too long. When you run the automated formatter, it's wrapping the line (and appears to do so in a way that doesn't reach a fixed point if the formatter is rerun). Rather than commit changes suggested by the formatter where it's wrapping the first line comment, you should reject those changes (or not commit them; |
Thanks. Once I fix the comment do I just run |
The GPU should use For now however I think that it's good to have a C implementation to fall back on. |
So if you have 2 commits |
|
✅ With the latest revision this PR passed the Python code formatter. |
|
@lntue ok I addressed your review |
| LIBC_INLINE T fmaximum_num(T x, T y) { | ||
| FPBits<T> bitx(x), bity(y); | ||
|
|
||
| if (bitx.is_nan()) |
There was a problem hiding this comment.
I'm sorry for a late suggestion, but I just noticed that in section F.10.9.5 in C23 standard, the exceptional handling of f*_num functions is different from their non-num versions:
- if both are
NaN, then a quiet NaN is returned. - if at least one of them is signaling NaN, then
FE_INVALIDis raised.
Do you mind updating fmaximum_num and fminimum_num implementations here to take care of those cases, and updating f*_num_test (4 of them in total) accordingly?
Thanks,
There was a problem hiding this comment.
Yea I’ll get it done tomorrow.
are the other functions correct? I tried to follow the doc you mentioned on the issue and also looked at some gcc docs. I’ll double check tomorrow.
There was a problem hiding this comment.
The others look correct to me.
There was a problem hiding this comment.
Here’s the definition I was basing it on
The fmaximum_num function returns the greater of the two values x and y. If one argument is a number and the other is a NaN, even a signaling NaN, the number is returned. Positive zero is treated as greater than negative zero.
I took this from here https://www.gnu.org/software/libc/manual/html_node/Misc-FP-Arithmetic.html
There was a problem hiding this comment.
@lntue hello, I changed the code as you suggested.
can you please give an idea of how to test the exception? In the codebase I have found examples of EXPECT_FP_EQ_WITH_EXCEPTION
But not sure how to use it
There was a problem hiding this comment.
Something like EXPECT_FP_EQ_WITH_EXCEPTION(aNaN, func(aNaN, sNaN), FE_INVALID); should work. And you'll neet to test with all combinations of NaNs.
There was a problem hiding this comment.
Is my handling of the case associated with raising the exception correct? Because I used you example and I’m getting which is 0 to be greater or equal to 4. I also tried to return a quiet nan but same thing happened
There was a problem hiding this comment.
I figured it out. i found the bug
ghost
requested a review
from
| EXPECT_FP_EQ(inf, func(FPBits::quiet_nan().get_val(), inf)); | ||
| EXPECT_FP_EQ(inf, func(FPBits::signaling_nan().get_val(), inf)); |
There was a problem hiding this comment.
aNaN and sNaN could be used in this file instead of these. They are just the same.
https://github.com/llvm/llvm-project/blob/main/libc/test/UnitTest/FPMatcher.h#L99
There was a problem hiding this comment.
aNaN corresponds to quiet nans and sNaN corresponds to signaling nan right?
There was a problem hiding this comment.
And the same for FMaximumNumTest.h
There was a problem hiding this comment.
Ditto. For all the tests, just use aNaN and sNaN consistently, and only use FPBits::quiet_nan( ... ) when you need to set something special in the payloads.
| EXPECT_FP_EQ(-0.0, func(-0.0, FPBits::signaling_nan().get_val())); | ||
| EXPECT_FP_EQ(T(-1.2345), func(FPBits::quiet_nan().get_val(), T(-1.2345))); | ||
| EXPECT_FP_EQ(T(1.2345), func(T(1.2345), FPBits::quiet_nan().get_val())); | ||
| EXPECT_FP_EQ(T(-1.2345), |
There was a problem hiding this comment.
Change this to EXPECT_FP_EQ_WITH_EXCEPTION( ... , FE_INVALID);
There was a problem hiding this comment.
You’re highlighting 4 lines. Which do I change? Do I change all the lines that you highlighted?
There was a problem hiding this comment.
Sorry, I meant that change all the EXPECT_FP_EQ to EXPECT_FP_EQ_WITH_EXCEPTION if one of the input is signaling nan.
There was a problem hiding this comment.
So do I need to change the logic in BasicOperations? Because in my code I only raise an exception when one argument is a nan and the other is a signaling nan
There was a problem hiding this comment.
Yes, section F.10.9.5 from https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf states that:
- If one input is a number, and the other input is a NaN (quiet or signaling), then return the number.
- If both inputs are NaN (quiet or signaling), then return a quiet NaN.
- If any of the inputs is a signaling NaN, the invalid floating point exception is raised (even if the other input is a number)
So for updating the logic of _num functions, it might be easier to treat the case when neither of the inputs are NaNs first. Then you can deal with all the NaN cases afterward.
And FPBits::is_nan() function will return true for both quiet and signaling NaNs.
There was a problem hiding this comment.
Yes, section F.10.9.5 from https://www.open-std.org/jtc1/sc22/wg14/www/docs/n3096.pdf states that:
* If one input is a number, and the other input is a NaN (quiet or signaling), then return the number. * If both inputs are NaN (quiet or signaling), then return a quiet NaN. * If any of the inputs is a signaling NaN, the invalid floating point exception is raised (even if the other input is a number)So for updating the logic of
_numfunctions, it might be easier to treat the case when neither of the inputs are NaNs first. Then you can deal with all the NaN cases afterward.And
FPBits::is_nan()function will return true for both quiet and signaling NaNs.
ok Ill try to address all of your review in the next two hours.
There was a problem hiding this comment.
Just take your time, don't need to rush. And thanks for bearing with me all the time!
There was a problem hiding this comment.
No problem. I just finished addressing your review! I’m looking forward to merge this! Thanks
| EXPECT_FP_EQ(inf, func(FPBits::signaling_nan().get_val(), inf)); | ||
| EXPECT_FP_EQ(neg_inf, func(neg_inf, FPBits::quiet_nan().get_val())); | ||
| EXPECT_FP_EQ(neg_inf, func(neg_inf, FPBits::signaling_nan().get_val())); | ||
| EXPECT_FP_EQ(FPBits::quiet_nan().get_val(), func(aNaN, aNaN)); |
There was a problem hiding this comment.
Change this to `EXPECT_EQ(FPBits(aNaN).uintval(), FPBits(func(aNaN, aNaN)).uintval());
| func(aNaN, FPBits::signaling_nan().get_val()), FE_INVALID); | ||
| EXPECT_FP_IS_NAN_WITH_EXCEPTION( | ||
| func(FPBits::signaling_nan().get_val(), aNaN), FE_INVALID); | ||
| } |
There was a problem hiding this comment.
Can you also add the following tests:
EXPECT_EQ(FPBits(aNaN).uintval(), FPBits(func(aNaN, sNaN)).uintval());
EXPECT_EQ(FPBits(aNaN).uintval(), FPBits(func(sNaN, aNaN)).uintval());
EXPECT_EQ(FPBits(aNaN).uintval(), FPBits(func(sNaN, sNaN)).uintval());
| if (bitx.is_signaling_nan()) { | ||
| fputil::raise_except_if_required(FE_INVALID); | ||
| if (bity.is_nan()) | ||
| return FPBits<T>::quiet_nan().get_val(); | ||
| return y; | ||
| } | ||
| if (bity.is_signaling_nan()) { | ||
| fputil::raise_except_if_required(FE_INVALID); | ||
| if (bitx.is_nan()) | ||
| return FPBits<T>::quiet_nan().get_val(); | ||
| return x; | ||
| } | ||
| if (bitx.is_quiet_nan()) | ||
| return y; | ||
| if (bity.is_quiet_nan()) | ||
| return x; |
There was a problem hiding this comment.
Sorry for the last request: this could be simplify a bit to:
if (bitx.is_nan()) {
if (bitx.is_signaling_nan())
fputil::raise_except_if_required(FE_INVALID)
if (bity.is_nan())
return FPBits<T>::quiet_nan().get_val();
return y;
}
if (bity.is_nan()) {
if (bity.is_signaling_nan())
fputil::raise_except_if_required(FE_INVALID)
// x is not NaN.
return x;
}
The same for fminimum_num.
There was a problem hiding this comment.
I used this code but now the NaN tests are failing. Do i go back to what I had?
There was a problem hiding this comment.
Sorry the logic I posted above is not correct for fmaximum_num(aNaN, sNaN). The corrected version is:
if (bitx.is_signaling_nan() || bity.is_signaling_nan()) {
fputil::raise_except_if_required(FE_INVALID);
if (bitx.is_nan() && bity.is_nan())
return FPBits<T>::quiet_nan().get_val();
}
if (bitx.is_nan())
return y;
if (bity.is_nan())
return x;
if (bitx.sign() != bity.sign())
return (bitx.is_neg() ? y : x);
return x > y ? x : y;
fminimum_num can be updated similarly.
Thank you! |
No, these are different operations. Not sure if we have a wired up builtin for these |
Without this, you get an error "Do not know how to soften the result of this operator!" when compiling for a soft float target. The libcall names match those defined in glibc <https://www.gnu.org/software/libc/manual/html_node/Misc-FP-Arithmetic.html> and more recently added to LLVM's libc <llvm#86016>.
Without this, you get an error "Do not know how to soften the result of this operator!" when compiling for a soft float target. The libcall names match those defined in glibc <https://www.gnu.org/software/libc/manual/html_node/Misc-FP-Arithmetic.html> and more recently added to LLVM's libc <#86016>.
Without this, you get an error "Do not know how to soften the result of this operator!" when compiling for a soft float target. The libcall names match those defined in glibc <https://www.gnu.org/software/libc/manual/html_node/Misc-FP-Arithmetic.html> and more recently added to LLVM's libc <llvm#86016>.
Hello,
This PR addresses this issue #85496.
I have added fminimum and fmaximum functions.
I built it using
I ran the tests as follows:
Please let me know of anything I need to change. I will be available.
Thanks.