[libc-commits] [libc] [llvm] [libc][math] Do not use float16 basic operations in hypotf16. (PR #155177)

[via libc-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-libc

Author: None (lntue)

<details>
<summary>Changes</summary>

Compiler runtimes for float16 basic operations might not be correctly rounded for all rounding modes.

---
Full diff: https://github.com/llvm/llvm-project/pull/155177.diff


4 Files Affected:

- (modified) libc/src/__support/FPUtil/CMakeLists.txt (+1) 
- (modified) libc/src/__support/FPUtil/Hypot.h (+12-2) 
- (modified) libc/src/math/generic/hypotf16.cpp (+6-7) 
- (modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (+1) 


``````````diff
diff --git a/libc/src/__support/FPUtil/CMakeLists.txt b/libc/src/__support/FPUtil/CMakeLists.txt
index 37520eadba005..e8fc539fd32e8 100644
--- a/libc/src/__support/FPUtil/CMakeLists.txt
+++ b/libc/src/__support/FPUtil/CMakeLists.txt
@@ -231,6 +231,7 @@ add_header_library(
     Hypot.h
   DEPENDS
     .basic_operations
+    .cast
     .fenv_impl
     .fp_bits
     .rounding_mode
diff --git a/libc/src/__support/FPUtil/Hypot.h b/libc/src/__support/FPUtil/Hypot.h
index 94da259cd42f0..227ff5e49372e 100644
--- a/libc/src/__support/FPUtil/Hypot.h
+++ b/libc/src/__support/FPUtil/Hypot.h
@@ -12,6 +12,7 @@
 #include "BasicOperations.h"
 #include "FEnvImpl.h"
 #include "FPBits.h"
+#include "cast.h"
 #include "rounding_mode.h"
 #include "src/__support/CPP/bit.h"
 #include "src/__support/CPP/type_traits.h"
@@ -133,8 +134,17 @@ LIBC_INLINE T hypot(T x, T y) {
   uint16_t a_exp = a_bits.get_biased_exponent();
   uint16_t b_exp = b_bits.get_biased_exponent();
 
-  if ((a_exp - b_exp >= FPBits_t::FRACTION_LEN + 2) || (x == 0) || (y == 0))
-    return x_abs.get_val() + y_abs.get_val();
+  if ((a_exp - b_exp >= FPBits_t::FRACTION_LEN + 2) || (x == 0) || (y == 0)) {
+    if constexpr (cpp::is_same_v<T, float16>) {
+      // Compiler runtime for basic operations of float16 might not be correctly
+      // rounded for all rounding modes.
+      float af = fputil::cast<float>(x_abs.get_val());
+      float bf = fputil::cast<float>(y_abs.get_val());
+      return fputil::cast<float16>(af + bf);
+    } else {
+      return x_abs.get_val() + y_abs.get_val();
+    }
+  }
 
   uint64_t out_exp = a_exp;
   StorageType a_mant = a_bits.get_mantissa();
diff --git a/libc/src/math/generic/hypotf16.cpp b/libc/src/math/generic/hypotf16.cpp
index d782c2687cdb6..fa90069f9ff0d 100644
--- a/libc/src/math/generic/hypotf16.cpp
+++ b/libc/src/math/generic/hypotf16.cpp
@@ -48,16 +48,15 @@ LLVM_LIBC_FUNCTION(float16, hypotf16, (float16 x, float16 y)) {
     return a_bits.get_val();
   }
 
-  // TODO: Investigate why replacing the return line below with:
-  //   return x_bits.get_val() + y_bits.get_val();
-  // fails the hypotf16 smoke tests.
+  float af = fputil::cast<float>(a_bits.get_val());
+  float bf = fputil::cast<float>(b_bits.get_val());
+
+  // Compiler runtime basic operations for float16 might not be correctly
+  // rounded for all rounding modes.
   if (LIBC_UNLIKELY(a_u - b_u >=
                     static_cast<uint16_t>((FPBits::FRACTION_LEN + 2)
                                           << FPBits::FRACTION_LEN)))
-    return a_bits.get_val() + b_bits.get_val();
-
-  float af = fputil::cast<float>(a_bits.get_val());
-  float bf = fputil::cast<float>(b_bits.get_val());
+    return fputil::cast<float16>(af + bf);
 
   // These squares are exact.
   float a_sq = af * af;
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index a4a2e39c74fe0..bfda5385f012b 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -1230,6 +1230,7 @@ libc_support_library(
         ":__support_cpp_bit",
         ":__support_cpp_type_traits",
         ":__support_fputil_basic_operations",
+        ":__support_fputil_cast",
         ":__support_fputil_fenv_impl",
         ":__support_fputil_fp_bits",
         ":__support_fputil_rounding_mode",

``````````

</details>


https://github.com/llvm/llvm-project/pull/155177