[libc-commits] [libc] [libc][stdfix] Implement sqrtfx for remaining fixed point types (PR #214741)

[via libc-commits]

================
@@ -181,31 +200,67 @@ sqrt_core(typename Config::Type x_frac) {
 
 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);
-
-  // Compute sqrt(x_frac) using Newton-method.
-  FracType r = sqrt_core<internal::SqrtConfig<T>>(x_frac);
-
-  // Re-scaling
-  r >>= EXP_ADJUSTMENT - (x_exp >> 1);
-
-  // Return result.
-  return cpp::bit_cast<T>(r);
+  if constexpr (FXRep<T>::SIGN_LEN > 0) {
+    LIBC_CRASH_ON_VALUE(x < FXRep<T>::ZERO(), true);
+    return static_cast<T>(
+        sqrt(static_cast<typename internal::FXUnsigned<T>::Type>(x)));
+  } else {
----------------
sohail103 wrote:

I tried this and it doesn't compile without the else. I think the if constexpr only removes the code that is inside the if/else bodies and doesn't handle early returns like this. Because of this everything that used to be inside the else gets instantiated for every type and because there's no specialization for SqrtConfig for signed types it fails to compile.

Also I was wondering if the signed functions are even necessary. Since there's no special handling for negative inputs anyway we could just keep the unsigned entrypoints and signed negative inputs would be UB anyway. Or do we want all the entrypoints for the sake of completeness?

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