Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc][stdfix] Add sqrt for fixed point types. #83042

Merged
merged 4 commits into from
Feb 27, 2024
Merged

[libc][stdfix] Add sqrt for fixed point types. #83042

merged 4 commits into from
Feb 27, 2024

Conversation

lntue
Copy link
Contributor

@lntue lntue commented Feb 26, 2024

No description provided.

@lntue lntue marked this pull request as ready for review February 26, 2024 19:27
@lntue lntue requested a review from PiJoules February 26, 2024 19:28
@llvmbot llvmbot added the libc label Feb 26, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 26, 2024

@llvm/pr-subscribers-libc

Author: None (lntue)

Changes

Patch is 25.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/83042.diff

23 Files Affected:

  • (modified) libc/config/linux/x86_64/entrypoints.txt (+6)
  • (modified) libc/docs/math/stdfix.rst (+1-1)
  • (modified) libc/spec/stdc_ext.td (+8)
  • (modified) libc/src/__support/fixed_point/CMakeLists.txt (+13)
  • (added) libc/src/__support/fixed_point/sqrt.h (+139)
  • (modified) libc/src/stdfix/CMakeLists.txt (+15)
  • (added) libc/src/stdfix/sqrtuhk.cpp (+19)
  • (added) libc/src/stdfix/sqrtuhk.h (+20)
  • (added) libc/src/stdfix/sqrtuhr.cpp (+19)
  • (added) libc/src/stdfix/sqrtuhr.h (+20)
  • (added) libc/src/stdfix/sqrtuk.cpp (+19)
  • (added) libc/src/stdfix/sqrtuk.h (+20)
  • (added) libc/src/stdfix/sqrtulr.cpp (+19)
  • (added) libc/src/stdfix/sqrtulr.h (+20)
  • (added) libc/src/stdfix/sqrtur.cpp (+19)
  • (added) libc/src/stdfix/sqrtur.h (+20)
  • (modified) libc/test/src/stdfix/CMakeLists.txt (+22)
  • (added) libc/test/src/stdfix/SqrtTest.h (+66)
  • (added) libc/test/src/stdfix/sqrtuhk_test.cpp (+13)
  • (added) libc/test/src/stdfix/sqrtuhr_test.cpp (+13)
  • (added) libc/test/src/stdfix/sqrtuk_test.cpp (+13)
  • (added) libc/test/src/stdfix/sqrtulr_test.cpp (+13)
  • (added) libc/test/src/stdfix/sqrtur_test.cpp (+13)
diff --git a/libc/config/linux/x86_64/entrypoints.txt b/libc/config/linux/x86_64/entrypoints.txt
index f2a224d45bbae7..cd44b5c52b58cd 100644
--- a/libc/config/linux/x86_64/entrypoints.txt
+++ b/libc/config/linux/x86_64/entrypoints.txt
@@ -473,6 +473,12 @@ if(LIBC_COMPILER_HAS_FIXED_POINT)
     libc.src.stdfix.roundur
     libc.src.stdfix.roundulk
     libc.src.stdfix.roundulr
+    libc.src.stdfix.sqrtuhk
+    libc.src.stdfix.sqrtuhr
+    libc.src.stdfix.sqrtuk
+    libc.src.stdfix.sqrtur
+    # libc.src.stdfix.sqrtulk
+    libc.src.stdfix.sqrtulr
   )
 endif()
 
diff --git a/libc/docs/math/stdfix.rst b/libc/docs/math/stdfix.rst
index 080066e53bd2f1..79f499e61f121c 100644
--- a/libc/docs/math/stdfix.rst
+++ b/libc/docs/math/stdfix.rst
@@ -78,7 +78,7 @@ Fixed-point Arithmetics
 +---------------+----------------+-------------+---------------+------------+----------------+-------------+----------------+-------------+---------------+------------+----------------+-------------+
 | round         | |check|        | |check|     | |check|       | |check|    | |check|        | |check|     | |check|        | |check|     | |check|       | |check|    | |check|        | |check|     |
 +---------------+----------------+-------------+---------------+------------+----------------+-------------+----------------+-------------+---------------+------------+----------------+-------------+
-| sqrt          |                |             |               |            |                |             |                |             |               |            |                |             |
+| sqrt          | |check|        |             | |check|       |            | |check|        |             | |check|        |             | |check|       |            |                |             |
 +---------------+----------------+-------------+---------------+------------+----------------+-------------+----------------+-------------+---------------+------------+----------------+-------------+
 
 ================== =========
diff --git a/libc/spec/stdc_ext.td b/libc/spec/stdc_ext.td
index 6620142146c471..be1e6d4ba2fcdc 100644
--- a/libc/spec/stdc_ext.td
+++ b/libc/spec/stdc_ext.td
@@ -47,6 +47,14 @@ def StdcExt : StandardSpec<"stdc_ext"> {
           GuardedFunctionSpec<"rounduhk", RetValSpec<UnsignedShortAccumType>, [ArgSpec<UnsignedShortAccumType>, ArgSpec<IntType>], "LIBC_COMPILER_HAS_FIXED_POINT">,
           GuardedFunctionSpec<"rounduk", RetValSpec<UnsignedAccumType>, [ArgSpec<UnsignedAccumType>, ArgSpec<IntType>], "LIBC_COMPILER_HAS_FIXED_POINT">,
           GuardedFunctionSpec<"roundulk", RetValSpec<UnsignedLongAccumType>, [ArgSpec<UnsignedLongAccumType>, ArgSpec<IntType>], "LIBC_COMPILER_HAS_FIXED_POINT">,
+
+          GuardedFunctionSpec<"sqrtuhr", RetValSpec<UnsignedShortFractType>, [ArgSpec<UnsignedShortFractType>], "LIBC_COMPILER_HAS_FIXED_POINT">,
+          GuardedFunctionSpec<"sqrtur", RetValSpec<UnsignedFractType>, [ArgSpec<UnsignedFractType>], "LIBC_COMPILER_HAS_FIXED_POINT">,
+          GuardedFunctionSpec<"sqrtulr", RetValSpec<UnsignedLongFractType>, [ArgSpec<UnsignedLongFractType>], "LIBC_COMPILER_HAS_FIXED_POINT">,
+
+          GuardedFunctionSpec<"sqrtuhk", RetValSpec<UnsignedShortAccumType>, [ArgSpec<UnsignedShortAccumType>], "LIBC_COMPILER_HAS_FIXED_POINT">,
+          GuardedFunctionSpec<"sqrtuk", RetValSpec<UnsignedAccumType>, [ArgSpec<UnsignedAccumType>], "LIBC_COMPILER_HAS_FIXED_POINT">,
+          GuardedFunctionSpec<"sqrtulk", RetValSpec<UnsignedLongAccumType>, [ArgSpec<UnsignedLongAccumType>], "LIBC_COMPILER_HAS_FIXED_POINT">,
       ]
   >;
 
diff --git a/libc/src/__support/fixed_point/CMakeLists.txt b/libc/src/__support/fixed_point/CMakeLists.txt
index 64f9dacc7ba5f0..0ed118f2408849 100644
--- a/libc/src/__support/fixed_point/CMakeLists.txt
+++ b/libc/src/__support/fixed_point/CMakeLists.txt
@@ -21,3 +21,16 @@ add_header_library(
     libc.src.__support.CPP.bit
     libc.src.__support.math_extras
 )
+
+add_header_library(
+  sqrt
+  HDRS
+    sqrt.h
+  DEPENDS
+    .fx_rep
+    libc.include.llvm-libc-macros.stdfix_macros
+    libc.src.__support.macros.attributes
+    libc.src.__support.macros.optimization
+    libc.src.__support.CPP.bit
+    libc.src.__support.CPP.type_traits
+)
diff --git a/libc/src/__support/fixed_point/sqrt.h b/libc/src/__support/fixed_point/sqrt.h
new file mode 100644
index 00000000000000..dcf2c715497130
--- /dev/null
+++ b/libc/src/__support/fixed_point/sqrt.h
@@ -0,0 +1,139 @@
+//===-- Calculate square root of fixed point numbers. -----*- 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___SUPPORT_FIXEDPOINT_SQRT_H
+#define LLVM_LIBC_SRC___SUPPORT_FIXEDPOINT_SQRT_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+#include "src/__support/CPP/bit.h"
+#include "src/__support/CPP/type_traits.h"
+#include "src/__support/macros/attributes.h"   // LIBC_INLINE
+#include "src/__support/macros/optimization.h" // LIBC_UNLIKELY
+
+#include "fx_rep.h"
+
+#ifdef LIBC_COMPILER_HAS_FIXED_POINT
+
+namespace LIBC_NAMESPACE::fixed_point {
+
+namespace internal {
+
+template <typename T> struct SqrtConfig;
+
+template <> struct SqrtConfig<unsigned short fract> {
+  using Type = unsigned short fract;
+  static constexpr int EXTRA_STEPS = 0;
+};
+
+template <> struct SqrtConfig<unsigned fract> {
+  using Type = unsigned fract;
+  static constexpr int EXTRA_STEPS = 1;
+};
+
+template <> struct SqrtConfig<unsigned long fract> {
+  using Type = unsigned long fract;
+  static constexpr int EXTRA_STEPS = 2;
+};
+
+template <>
+struct SqrtConfig<unsigned short accum> : SqrtConfig<unsigned fract> {};
+
+template <>
+struct SqrtConfig<unsigned accum> : SqrtConfig<unsigned long fract> {};
+
+// TODO: unsigned long accum type is 64-bit, and will need 64-bit fract type.
+// Probably we will use DyadicFloat<64> for intermediate computations instead.
+
+// Linear approximation for the initial values, with errors bounded by:
+//   max(1.5 * 2^-11, eps)
+// Generated with Sollya:
+// > for i from 4 to 15 do {
+//     P = fpminimax(sqrt(x), 1, [|8, 8|], [i * 2^-4, (i + 1)*2^-4],
+//                   fixed, absolute);
+//     print("{", coeff(P, 1), "uhr,", coeff(P, 0), "uhr},");
+//   };
+// Clang is having issues with this array:
+// static constexpr unsigned short fract SQRT_FIRST_APPROX[12][2] = {
+//     {0x1.e8p-1uhr, 0x1.0cp-2uhr}, {0x1.bap-1uhr, 0x1.28p-2uhr},
+//     {0x1.94p-1uhr, 0x1.44p-2uhr}, {0x1.74p-1uhr, 0x1.6p-2uhr},
+//     {0x1.6p-1uhr, 0x1.74p-2uhr},  {0x1.4ep-1uhr, 0x1.88p-2uhr},
+//     {0x1.3ep-1uhr, 0x1.9cp-2uhr}, {0x1.32p-1uhr, 0x1.acp-2uhr},
+//     {0x1.22p-1uhr, 0x1.c4p-2uhr}, {0x1.18p-1uhr, 0x1.d4p-2uhr},
+//     {0x1.08p-1uhr, 0x1.fp-2uhr},  {0x1.04p-1uhr, 0x1.f8p-2uhr},
+// };
+// We are using their storage type instead.
+// TODO(https://github.com/llvm/llvm-project/issues/83050): Use fixed point
+// array when the bug is fixed.
+static constexpr uint8_t SQRT_FIRST_APPROX[12][2] = {
+    {244, 67},  {221, 74},  {202, 81},  {186, 88},  {176, 93},  {167, 98},
+    {159, 103}, {153, 107}, {145, 113}, {140, 117}, {132, 124}, {130, 126},
+};
+
+} // namespace internal
+
+template <typename T>
+LIBC_INLINE constexpr cpp::enable_if_t<cpp::is_fixed_point_v<T>, T> sqrt(T x) {
+  using BitType = typename FXRep<T>::StorageType;
+  BitType x_bit = cpp::bit_cast<BitType>(x);
+
+  if (LIBC_UNLIKELY(x_bit == 0))
+    return FXRep<T>::ZERO();
+
+  int leading_zeros = cpp::countl_zero(x_bit);
+  constexpr int STORAGE_LENGTH = sizeof(BitType) * CHAR_BIT;
+  constexpr int EXP_ADJUSTMENT = STORAGE_LENGTH - FXRep<T>::FRACTION_LEN - 1;
+  // x_exp is the real exponent of the leading bit of x.
+  int x_exp = EXP_ADJUSTMENT - leading_zeros;
+  int shift = EXP_ADJUSTMENT - 1 - (x_exp & (~1));
+  // Normalize.
+  x_bit <<= shift;
+  using FracType = typename internal::SqrtConfig<T>::Type;
+  FracType x_frac = cpp::bit_cast<FracType>(x_bit);
+
+  // Use use Newton method to approximate sqrt(a):
+  //   x_{n + 1} = 1/2 (x_n + a / x_n)
+  // For the initial values, we choose x_0
+
+  // Use the leading 4 bits to do look up for sqrt(x).
+  int index = (x_bit >> (STORAGE_LENGTH - 4)) - 4;
+  FracType a = static_cast<FracType>(cpp::bit_cast<unsigned short fract>(
+      internal::SQRT_FIRST_APPROX[index][0]));
+  FracType b = static_cast<FracType>(cpp::bit_cast<unsigned short fract>(
+      internal::SQRT_FIRST_APPROX[index][1]));
+
+  // Initial approximation step.
+  // Estimated error bounds: | r - sqrt(x_frac) | < max(1.5 * 2^-11, eps).
+  FracType r = a * x_frac + b;
+
+  // Further Newton-method iterations for square-root:
+  //   x_{n + 1} = 0.5 * (x_n + a / x_n)
+  // We distribute and do the multiplication by 0.5 first to avoid overflow.
+  // TODO: Investigate the performance and accuracy of using division-free
+  // iterations from:
+  //   Blanchard, J. D. and Chamberland, M., "Newton's Method Without Division",
+  //   The American Mathematical Monthly (2023).
+  //   https://chamberland.math.grinnell.edu/papers/newton.pdf
+  for (int i = 0; i < internal::SqrtConfig<T>::EXTRA_STEPS; ++i) {
+    r = (r >> 1) + (x_frac >> 1) / r;
+  }
+
+  int r_exp = (x_exp >> 1);
+  int shift_back = EXP_ADJUSTMENT - r_exp;
+
+  // Re-scaling
+  r >>= shift_back;
+
+  // Return result.
+  return cpp::bit_cast<T>(r);
+}
+
+} // namespace LIBC_NAMESPACE::fixed_point
+
+#endif // LIBC_COMPILER_HAS_FIXED_POINT
+
+#endif // LLVM_LIBC_SRC___SUPPORT_FIXEDPOINT_SQRT_H
diff --git a/libc/src/stdfix/CMakeLists.txt b/libc/src/stdfix/CMakeLists.txt
index 6e2ed1bdfeafe7..cb2134fe33cf9c 100644
--- a/libc/src/stdfix/CMakeLists.txt
+++ b/libc/src/stdfix/CMakeLists.txt
@@ -17,6 +17,21 @@ foreach(suffix IN ITEMS hr r lr hk k lk)
   )
 endforeach()
 
+foreach(suffix IN ITEMS uhr ur ulr uhk uk)
+  add_entrypoint_object(
+    sqrt${suffix}
+    HDRS
+      sqrt${suffix}.h
+    SRCS
+      sqrt${suffix}.cpp
+    COMPILE_OPTIONS
+      -O3
+      -ffixed-point
+    DEPENDS
+      libc.src.__support.fixed_point.sqrt
+  )
+endforeach()
+
 foreach(suffix IN ITEMS hr r lr hk k lk uhr ur ulr uhk uk ulk)
   add_entrypoint_object(
     round${suffix}
diff --git a/libc/src/stdfix/sqrtuhk.cpp b/libc/src/stdfix/sqrtuhk.cpp
new file mode 100644
index 00000000000000..e8dc842c8a9980
--- /dev/null
+++ b/libc/src/stdfix/sqrtuhk.cpp
@@ -0,0 +1,19 @@
+//===-- Implementation of sqrtuhk 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 "sqrtuhk.h"
+#include "src/__support/common.h"
+#include "src/__support/fixed_point/sqrt.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(unsigned short accum, sqrtuhk, (unsigned short accum x)) {
+  return fixed_point::sqrt(x);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdfix/sqrtuhk.h b/libc/src/stdfix/sqrtuhk.h
new file mode 100644
index 00000000000000..80000a0079696d
--- /dev/null
+++ b/libc/src/stdfix/sqrtuhk.h
@@ -0,0 +1,20 @@
+//===-- Implementation header for sqrtuhk -----------------------*- 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_STDFIX_SQRTUHK_H
+#define LLVM_LIBC_SRC_STDFIX_SQRTUHK_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+
+namespace LIBC_NAMESPACE {
+
+unsigned short accum sqrtuhk(unsigned short accum x);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_STDFIX_SQRTUHK_H
diff --git a/libc/src/stdfix/sqrtuhr.cpp b/libc/src/stdfix/sqrtuhr.cpp
new file mode 100644
index 00000000000000..6bba07aa20d595
--- /dev/null
+++ b/libc/src/stdfix/sqrtuhr.cpp
@@ -0,0 +1,19 @@
+//===-- Implementation of sqrtuhr 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 "sqrtuhr.h"
+#include "src/__support/common.h"
+#include "src/__support/fixed_point/sqrt.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(unsigned short fract, sqrtuhr, (unsigned short fract x)) {
+  return fixed_point::sqrt(x);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdfix/sqrtuhr.h b/libc/src/stdfix/sqrtuhr.h
new file mode 100644
index 00000000000000..fd95f0924e8d48
--- /dev/null
+++ b/libc/src/stdfix/sqrtuhr.h
@@ -0,0 +1,20 @@
+//===-- Implementation header for sqrtuhr -----------------------*- 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_STDFIX_SQRTUHR_H
+#define LLVM_LIBC_SRC_STDFIX_SQRTUHR_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+
+namespace LIBC_NAMESPACE {
+
+unsigned short fract sqrtuhr(unsigned short fract x);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_STDFIX_SQRTUHR_H
diff --git a/libc/src/stdfix/sqrtuk.cpp b/libc/src/stdfix/sqrtuk.cpp
new file mode 100644
index 00000000000000..6e5d8118c83b73
--- /dev/null
+++ b/libc/src/stdfix/sqrtuk.cpp
@@ -0,0 +1,19 @@
+//===-- Implementation of sqrtuk 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 "sqrtuk.h"
+#include "src/__support/common.h"
+#include "src/__support/fixed_point/sqrt.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(unsigned accum, sqrtuk, (unsigned accum x)) {
+  return fixed_point::sqrt(x);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdfix/sqrtuk.h b/libc/src/stdfix/sqrtuk.h
new file mode 100644
index 00000000000000..04d0adadde9ad2
--- /dev/null
+++ b/libc/src/stdfix/sqrtuk.h
@@ -0,0 +1,20 @@
+//===-- Implementation header for sqrtuk ------------------------*- 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_STDFIX_SQRTUK_H
+#define LLVM_LIBC_SRC_STDFIX_SQRTUK_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+
+namespace LIBC_NAMESPACE {
+
+unsigned accum sqrtuk(unsigned accum x);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_STDFIX_SQRTUK_H
diff --git a/libc/src/stdfix/sqrtulr.cpp b/libc/src/stdfix/sqrtulr.cpp
new file mode 100644
index 00000000000000..c9e5cd51f66bc5
--- /dev/null
+++ b/libc/src/stdfix/sqrtulr.cpp
@@ -0,0 +1,19 @@
+//===-- Implementation of sqrtulr 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 "sqrtulr.h"
+#include "src/__support/common.h"
+#include "src/__support/fixed_point/sqrt.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(unsigned long fract, sqrtulr, (unsigned long fract x)) {
+  return fixed_point::sqrt(x);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdfix/sqrtulr.h b/libc/src/stdfix/sqrtulr.h
new file mode 100644
index 00000000000000..284adaaf35bf59
--- /dev/null
+++ b/libc/src/stdfix/sqrtulr.h
@@ -0,0 +1,20 @@
+//===-- Implementation header for sqrtulr -----------------------*- 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_STDFIX_SQRTULR_H
+#define LLVM_LIBC_SRC_STDFIX_SQRTULR_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+
+namespace LIBC_NAMESPACE {
+
+unsigned long fract sqrtulr(unsigned long fract x);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_STDFIX_SQRTULR_H
diff --git a/libc/src/stdfix/sqrtur.cpp b/libc/src/stdfix/sqrtur.cpp
new file mode 100644
index 00000000000000..ac5be84910849f
--- /dev/null
+++ b/libc/src/stdfix/sqrtur.cpp
@@ -0,0 +1,19 @@
+//===-- Implementation of sqrtur 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 "sqrtur.h"
+#include "src/__support/common.h"
+#include "src/__support/fixed_point/sqrt.h"
+
+namespace LIBC_NAMESPACE {
+
+LLVM_LIBC_FUNCTION(unsigned fract, sqrtur, (unsigned fract x)) {
+  return fixed_point::sqrt(x);
+}
+
+} // namespace LIBC_NAMESPACE
diff --git a/libc/src/stdfix/sqrtur.h b/libc/src/stdfix/sqrtur.h
new file mode 100644
index 00000000000000..df9dfe5a0bf39e
--- /dev/null
+++ b/libc/src/stdfix/sqrtur.h
@@ -0,0 +1,20 @@
+//===-- Implementation header for sqrtur ------------------------*- 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_STDFIX_SQRTUR_H
+#define LLVM_LIBC_SRC_STDFIX_SQRTUR_H
+
+#include "include/llvm-libc-macros/stdfix-macros.h"
+
+namespace LIBC_NAMESPACE {
+
+unsigned fract sqrtur(unsigned fract x);
+
+} // namespace LIBC_NAMESPACE
+
+#endif // LLVM_LIBC_SRC_STDFIX_SQRTUR_H
diff --git a/libc/test/src/stdfix/CMakeLists.txt b/libc/test/src/stdfix/CMakeLists.txt
index b6e0256bb68800..4140b5b29f3b3c 100644
--- a/libc/test/src/stdfix/CMakeLists.txt
+++ b/libc/test/src/stdfix/CMakeLists.txt
@@ -22,6 +22,28 @@ foreach(suffix IN ITEMS hr r lr hk k lk)
   )
 endforeach()
 
+foreach(suffix IN ITEMS uhr ur ulr uhk uk)
+  add_libc_test(
+    sqrt${suffix}_test
+    SUITE
+      libc-stdfix-tests
+    HDRS
+      SqrtTest.h
+    SRCS
+      sqrt${suffix}_test.cpp
+    COMPILE_OPTIONS
+      -O3
+      -ffixed-point
+    DEPENDS
+      libc.src.stdfix.sqrt${suffix}
+      libc.src.__support.CPP.bit
+      libc.src.__support.fixed_point.fx_rep
+      libc.src.__support.fixed_point.sqrt
+      libc.src.__support.FPUtil.basic_operations
+      libc.src.__support.FPUtil.sqrt
+  )
+endforeach()
+
 foreach(suffix IN ITEMS hr r lr hk k lk uhr ur ulr uhk uk ulk)
   add_libc_test(
     round${suffix}_test
diff --git a/libc/test/src/stdfix/SqrtTest.h b/libc/test/src/stdfix/SqrtTest.h
new file mode 100644
index 00000000000000..628be0deb770fb
--- /dev/null
+++ b/libc/test/src/stdfix/SqrtTest.h
@@ -0,0 +1,66 @@
+//===-- Utility class to test fixed-point sqrt ------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+
+#include "test/UnitTest/Test.h"
+
+#include "src/__support/CPP/bit.h"...
[truncated]

libc/src/__support/fixed_point/sqrt.h Outdated Show resolved Hide resolved
libc/src/__support/fixed_point/sqrt.h Outdated Show resolved Hide resolved
libc/src/__support/fixed_point/sqrt.h Show resolved Hide resolved
libc/src/__support/fixed_point/sqrt.h Outdated Show resolved Hide resolved
constexpr int EXP_ADJUSTMENT = STORAGE_LENGTH - FXRep<T>::FRACTION_LEN - 1;
// x_exp is the real exponent of the leading bit of x.
int x_exp = EXP_ADJUSTMENT - leading_zeros;
int shift = EXP_ADJUSTMENT - 1 - (x_exp & (~1));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that ~1 may have problems on 64 bit types since it's an int by default. Is it worthwhile to make this even in a different way?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is all computed in int, and all the variables and intermediates are in int so I think it should be fine. Another way to do it is (x_exp >> 1) << 1 which I think is clunkier.

libc/src/__support/fixed_point/sqrt.h Show resolved Hide resolved
// The American Mathematical Monthly (2023).
// https://chamberland.math.grinnell.edu/papers/newton.pdf
for (int i = 0; i < internal::SqrtConfig<T>::EXTRA_STEPS; ++i) {
r = (r >> 1) + (x_frac >> 1) / r;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you remove the division, I think this could all be calculated as integer operations, removing the need for a 64 bit fract type.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit tricky if we want to just use integers. Shifts and additions are totally fine, but replacement for division will involve multiplications, and for that we need to keep track of the correct bit positions, which the fixed point types are doing for us internally. Otherwise, the integer multiplications will only retain the least significant bits.

Using DyadicFloat will fit the requirement for fixed point and it is quite convenient for us, but might be a bit inefficient, since DyadicFloat will normalize the bits after every operation. I guess eventually we will implement a fixed point class with all the basic arithmetic operations, similar to FPBits and DyadicFloat, then we can simply use it instead.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the situation, this seems reasonable. I think Making a fixed point type comparable to DyadicFloat is the best path forward.

Comment on lines 70 to 67
// TODO(https://github.com/llvm/llvm-project/issues/83050): Use fixed point
// array when the bug is fixed.
static constexpr uint8_t SQRT_FIRST_APPROX[12][2] = {
{244, 67}, {221, 74}, {202, 81}, {186, 88}, {176, 93}, {167, 98},
{159, 103}, {153, 107}, {145, 113}, {140, 117}, {132, 124}, {130, 126},
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#83071 fixes this. It's not too late to backport to clang-18 (release/18.x), but we claim to support clang-11+ (https://libc.llvm.org/compiler_support.html). So do we want to have 2 different code paths here, or what?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the current predefined detection macro __FRACT_FBIT__ that we use just added to very recently, and basic compiler support for fixed point is still on-going, I think it is ok for us to require clang trunk to build fixed point functions for now. Later on, we might set the requirement to build fixed point functions to be clang-19 once it's released.

Copy link
Contributor

@michaelrj-google michaelrj-google left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

// The American Mathematical Monthly (2023).
// https://chamberland.math.grinnell.edu/papers/newton.pdf
for (int i = 0; i < internal::SqrtConfig<T>::EXTRA_STEPS; ++i) {
r = (r >> 1) + (x_frac >> 1) / r;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given the situation, this seems reasonable. I think Making a fixed point type comparable to DyadicFloat is the best path forward.

@lntue lntue merged commit ded4ea9 into llvm:main Feb 27, 2024
5 checks passed
@lntue lntue deleted the fixsqrt branch February 27, 2024 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants