[libc-commits] [libc] 7706945 - [libc] Work around incorrect fmin/fmax results for +/-x (#83158)

via libc-commits libc-commits at lists.llvm.org
Tue Feb 27 09:57:51 PST 2024


Author: Joseph Huber
Date: 2024-02-27T11:57:47-06:00
New Revision: 770694539bde0f1b706c70d87dd70777c94f178f

URL: https://github.com/llvm/llvm-project/commit/770694539bde0f1b706c70d87dd70777c94f178f
DIFF: https://github.com/llvm/llvm-project/commit/770694539bde0f1b706c70d87dd70777c94f178f.diff

LOG: [libc] Work around incorrect fmin/fmax results for +/-x (#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.

Added: 
    

Modified: 
    libc/src/math/amdgpu/fmax.cpp
    libc/src/math/amdgpu/fmaxf.cpp
    libc/src/math/amdgpu/fmin.cpp
    libc/src/math/amdgpu/fminf.cpp
    libc/src/math/nvptx/fmax.cpp
    libc/src/math/nvptx/fmaxf.cpp
    libc/src/math/nvptx/fmin.cpp
    libc/src/math/nvptx/fminf.cpp
    libc/test/src/math/smoke/CMakeLists.txt

Removed: 
    


################################################################################
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