[libc-commits] [libc] [libc] Work around incorrect fmin/fmax results for +/-x (PR #83158)
Joseph Huber via libc-commits
libc-commits at lists.llvm.org
Tue Feb 27 09:43:20 PST 2024
https://github.com/jhuber6 created https://github.com/llvm/llvm-project/pull/83158
Summary:
The IEEE 754 standard as of the 2019 revision states that for fmin -0.0
is always less than 0.0 and for fmax 0.0 is always greater than 0.0.
These are currently not respected by the builtin value and thus cause
the tests to fail. This patch works around it in the implementation for
now by explicitly modifying the sign bit.
>From 691f43d2d5579eecec036fe7d9783beea31355b4 Mon Sep 17 00:00:00 2001
From: Joseph Huber <huberjn at outlook.com>
Date: Tue, 27 Feb 2024 11:36:55 -0600
Subject: [PATCH] [libc] Work around incorrect fmin/fmax results for +/-x
Summary:
The IEEE 754 standard as of the 2019 revision states that for fmin -0.0
is always less than 0.0 and for fmax 0.0 is always greater than 0.0.
These are currently not respected by the builtin value and thus cause
the tests to fail. This patch works around it in the implementation for
now by explicitly modifying the sign bit.
---
libc/src/math/amdgpu/fmax.cpp | 7 +
libc/src/math/amdgpu/fmaxf.cpp | 7 +
libc/src/math/amdgpu/fmin.cpp | 7 +
libc/src/math/amdgpu/fminf.cpp | 7 +
libc/src/math/nvptx/fmax.cpp | 7 +
libc/src/math/nvptx/fmaxf.cpp | 7 +
libc/src/math/nvptx/fmin.cpp | 7 +
libc/src/math/nvptx/fminf.cpp | 7 +
libc/test/src/math/smoke/CMakeLists.txt | 195 ++++++++++++------------
9 files changed, 152 insertions(+), 99 deletions(-)
diff --git a/libc/src/math/amdgpu/fmax.cpp b/libc/src/math/amdgpu/fmax.cpp
index a2c35371d12b6a..09624cc6f092af 100644
--- a/libc/src/math/amdgpu/fmax.cpp
+++ b/libc/src/math/amdgpu/fmax.cpp
@@ -7,11 +7,18 @@
//===----------------------------------------------------------------------===//
#include "src/math/fmax.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, fmax, (double x, double y)) {
+ // FIXME: The builtin function does not correctly handle the +/-0.0 case.
+ if (LIBC_UNLIKELY(x == y))
+ return cpp::bit_cast<double>(cpp::bit_cast<uint64_t>(x) &
+ cpp::bit_cast<uint64_t>(y));
return __builtin_fmax(x, y);
}
diff --git a/libc/src/math/amdgpu/fmaxf.cpp b/libc/src/math/amdgpu/fmaxf.cpp
index 67178b3e273575..f6ed46699a049f 100644
--- a/libc/src/math/amdgpu/fmaxf.cpp
+++ b/libc/src/math/amdgpu/fmaxf.cpp
@@ -7,11 +7,18 @@
//===----------------------------------------------------------------------===//
#include "src/math/fmaxf.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, fmaxf, (float x, float y)) {
+ // FIXME: The builtin function does not correctly handle the +/-0.0 case.
+ if (LIBC_UNLIKELY(x == y))
+ return cpp::bit_cast<float>(cpp::bit_cast<uint32_t>(x) &
+ cpp::bit_cast<uint32_t>(y));
return __builtin_fmaxf(x, y);
}
diff --git a/libc/src/math/amdgpu/fmin.cpp b/libc/src/math/amdgpu/fmin.cpp
index 7303adcd347ee9..8977ff7a066c6b 100644
--- a/libc/src/math/amdgpu/fmin.cpp
+++ b/libc/src/math/amdgpu/fmin.cpp
@@ -7,11 +7,18 @@
//===----------------------------------------------------------------------===//
#include "src/math/fmin.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, fmin, (double x, double y)) {
+ // FIXME: The builtin function does not correctly handle the +/-0.0 case.
+ if (LIBC_UNLIKELY(x == y))
+ return cpp::bit_cast<double>(cpp::bit_cast<uint64_t>(x) |
+ cpp::bit_cast<uint64_t>(y));
return __builtin_fmin(x, y);
}
diff --git a/libc/src/math/amdgpu/fminf.cpp b/libc/src/math/amdgpu/fminf.cpp
index bbf0c677b5e3ae..3be55257f61649 100644
--- a/libc/src/math/amdgpu/fminf.cpp
+++ b/libc/src/math/amdgpu/fminf.cpp
@@ -7,11 +7,18 @@
//===----------------------------------------------------------------------===//
#include "src/math/fminf.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, fminf, (float x, float y)) {
+ // FIXME: The builtin function does not correctly handle the +/-0.0 case.
+ if (LIBC_UNLIKELY(x == y))
+ return cpp::bit_cast<float>(cpp::bit_cast<uint32_t>(x) |
+ cpp::bit_cast<uint32_t>(y));
return __builtin_fminf(x, y);
}
diff --git a/libc/src/math/nvptx/fmax.cpp b/libc/src/math/nvptx/fmax.cpp
index a2c35371d12b6a..09624cc6f092af 100644
--- a/libc/src/math/nvptx/fmax.cpp
+++ b/libc/src/math/nvptx/fmax.cpp
@@ -7,11 +7,18 @@
//===----------------------------------------------------------------------===//
#include "src/math/fmax.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, fmax, (double x, double y)) {
+ // FIXME: The builtin function does not correctly handle the +/-0.0 case.
+ if (LIBC_UNLIKELY(x == y))
+ return cpp::bit_cast<double>(cpp::bit_cast<uint64_t>(x) &
+ cpp::bit_cast<uint64_t>(y));
return __builtin_fmax(x, y);
}
diff --git a/libc/src/math/nvptx/fmaxf.cpp b/libc/src/math/nvptx/fmaxf.cpp
index 67178b3e273575..f6ed46699a049f 100644
--- a/libc/src/math/nvptx/fmaxf.cpp
+++ b/libc/src/math/nvptx/fmaxf.cpp
@@ -7,11 +7,18 @@
//===----------------------------------------------------------------------===//
#include "src/math/fmaxf.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, fmaxf, (float x, float y)) {
+ // FIXME: The builtin function does not correctly handle the +/-0.0 case.
+ if (LIBC_UNLIKELY(x == y))
+ return cpp::bit_cast<float>(cpp::bit_cast<uint32_t>(x) &
+ cpp::bit_cast<uint32_t>(y));
return __builtin_fmaxf(x, y);
}
diff --git a/libc/src/math/nvptx/fmin.cpp b/libc/src/math/nvptx/fmin.cpp
index 7303adcd347ee9..8977ff7a066c6b 100644
--- a/libc/src/math/nvptx/fmin.cpp
+++ b/libc/src/math/nvptx/fmin.cpp
@@ -7,11 +7,18 @@
//===----------------------------------------------------------------------===//
#include "src/math/fmin.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, fmin, (double x, double y)) {
+ // FIXME: The builtin function does not correctly handle the +/-0.0 case.
+ if (LIBC_UNLIKELY(x == y))
+ return cpp::bit_cast<double>(cpp::bit_cast<uint64_t>(x) |
+ cpp::bit_cast<uint64_t>(y));
return __builtin_fmin(x, y);
}
diff --git a/libc/src/math/nvptx/fminf.cpp b/libc/src/math/nvptx/fminf.cpp
index bbf0c677b5e3ae..3be55257f61649 100644
--- a/libc/src/math/nvptx/fminf.cpp
+++ b/libc/src/math/nvptx/fminf.cpp
@@ -7,11 +7,18 @@
//===----------------------------------------------------------------------===//
#include "src/math/fminf.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, fminf, (float x, float y)) {
+ // FIXME: The builtin function does not correctly handle the +/-0.0 case.
+ if (LIBC_UNLIKELY(x == y))
+ return cpp::bit_cast<float>(cpp::bit_cast<uint32_t>(x) |
+ cpp::bit_cast<uint32_t>(y));
return __builtin_fminf(x, y);
}
diff --git a/libc/test/src/math/smoke/CMakeLists.txt b/libc/test/src/math/smoke/CMakeLists.txt
index 7ef87a0dd2eecf..825000e1cb7afe 100644
--- a/libc/test/src/math/smoke/CMakeLists.txt
+++ b/libc/test/src/math/smoke/CMakeLists.txt
@@ -1092,112 +1092,109 @@ add_fp_unittest(
libc.src.__support.FPUtil.fp_bits
)
-# FIXME: These tests are currently broken on the GPU.
-if(NOT LIBC_TARGET_OS_IS_GPU)
- add_fp_unittest(
- fminf_test
- SUITE
- libc-math-smoke-tests
- SRCS
- fminf_test.cpp
- HDRS
- FMinTest.h
- DEPENDS
- libc.src.math.fminf
- libc.src.__support.FPUtil.fp_bits
- )
+add_fp_unittest(
+ fminf_test
+ SUITE
+ libc-math-smoke-tests
+ SRCS
+ fminf_test.cpp
+ HDRS
+ FMinTest.h
+ DEPENDS
+ libc.src.math.fminf
+ libc.src.__support.FPUtil.fp_bits
+)
- add_fp_unittest(
- fmin_test
- SUITE
- libc-math-smoke-tests
- SRCS
- fmin_test.cpp
- HDRS
- FMinTest.h
- DEPENDS
- libc.src.math.fmin
- libc.src.__support.FPUtil.fp_bits
- )
+add_fp_unittest(
+ fmin_test
+ SUITE
+ libc-math-smoke-tests
+ SRCS
+ fmin_test.cpp
+ HDRS
+ FMinTest.h
+ DEPENDS
+ libc.src.math.fmin
+ libc.src.__support.FPUtil.fp_bits
+)
- add_fp_unittest(
- fminl_test
- SUITE
- libc-math-smoke-tests
- SRCS
- fminl_test.cpp
- HDRS
- FMinTest.h
- DEPENDS
- libc.src.math.fminl
- libc.src.__support.FPUtil.fp_bits
- )
+add_fp_unittest(
+ fminl_test
+ SUITE
+ libc-math-smoke-tests
+ SRCS
+ fminl_test.cpp
+ HDRS
+ FMinTest.h
+ DEPENDS
+ libc.src.math.fminl
+ libc.src.__support.FPUtil.fp_bits
+)
- add_fp_unittest(
- fminf128_test
- SUITE
- libc-math-smoke-tests
- SRCS
- fminf128_test.cpp
- HDRS
- FMinTest.h
- DEPENDS
- libc.src.math.fminf128
- libc.src.__support.FPUtil.fp_bits
- )
+add_fp_unittest(
+ fminf128_test
+ SUITE
+ libc-math-smoke-tests
+ SRCS
+ fminf128_test.cpp
+ HDRS
+ FMinTest.h
+ DEPENDS
+ libc.src.math.fminf128
+ libc.src.__support.FPUtil.fp_bits
+)
- add_fp_unittest(
- fmaxf_test
- SUITE
- libc-math-smoke-tests
- SRCS
- fmaxf_test.cpp
- HDRS
- FMaxTest.h
- DEPENDS
- libc.src.math.fmaxf
- libc.src.__support.FPUtil.fp_bits
- )
+add_fp_unittest(
+ fmaxf_test
+ SUITE
+ libc-math-smoke-tests
+ SRCS
+ fmaxf_test.cpp
+ HDRS
+ FMaxTest.h
+ DEPENDS
+ libc.src.math.fmaxf
+ libc.src.__support.FPUtil.fp_bits
+)
- add_fp_unittest(
- fmax_test
- SUITE
- libc-math-smoke-tests
- SRCS
- fmax_test.cpp
- HDRS
- FMaxTest.h
- DEPENDS
- libc.src.math.fmax
- libc.src.__support.FPUtil.fp_bits
- )
+add_fp_unittest(
+ fmax_test
+ SUITE
+ libc-math-smoke-tests
+ SRCS
+ fmax_test.cpp
+ HDRS
+ FMaxTest.h
+ DEPENDS
+ libc.src.math.fmax
+ libc.src.__support.FPUtil.fp_bits
+)
- add_fp_unittest(
- fmaxl_test
- SUITE
- libc-math-smoke-tests
- SRCS
- fmaxl_test.cpp
- HDRS
- FMaxTest.h
- DEPENDS
- libc.src.math.fmaxl
- libc.src.__support.FPUtil.fp_bits
- )
+add_fp_unittest(
+ fmaxl_test
+ SUITE
+ libc-math-smoke-tests
+ SRCS
+ fmaxl_test.cpp
+ HDRS
+ FMaxTest.h
+ DEPENDS
+ libc.src.math.fmaxl
+ libc.src.__support.FPUtil.fp_bits
+)
- add_fp_unittest(
- fmaxf128_test
- SUITE
- libc-math-smoke-tests
- SRCS
- fmaxf128_test.cpp
- HDRS
- FMaxTest.h
- DEPENDS
- libc.src.math.fmaxf128
- libc.src.__support.FPUtil.fp_bits
- )
-endif()
+add_fp_unittest(
+ fmaxf128_test
+ SUITE
+ libc-math-smoke-tests
+ SRCS
+ fmaxf128_test.cpp
+ HDRS
+ FMaxTest.h
+ DEPENDS
+ libc.src.math.fmaxf128
+ libc.src.__support.FPUtil.fp_bits
+)
add_fp_unittest(
sqrtf_test
More information about the libc-commits
mailing list