[libc-commits] [PATCH] D84726: [libc] Add implementations for sqrt, sqrtf, and sqrtl.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added a comment.

I need more time to read through the patch. But I have left few comments about the structuring of the code.



================
Comment at: libc/utils/FPUtil/Sqrt.h:22
+template <typename T>
+void normalize(int &exponent, typename FPBits<T>::UIntType &mantissa);
+
----------------
Make all functions implemented in header files `static inline`.


================
Comment at: libc/utils/FPUtil/Sqrt.h:63
+
+#ifndef LLVM_LIBC_UTILS_FPUTIL_LONG_DOUBLE_BITS_X86_H
+template <> void normalize<long double>(int &exponent, __uint128_t &mantissa) {
----------------
Instead of this, use: `#if !(defined(__x86_64__) || defined(__i386__))`



================
Comment at: libc/utils/FPUtil/Sqrt.h:176
+}
+
+} // namespace fputil
----------------
To keep FPUtil utility like, would it make sense to conditionally include `SqrtLongDoubleX86.h`. This will also avoid the conditional include in `sqrtl.cpp` and FPUtil/Sqrt.h will be the only file to include in other places where the square root function will probably be used (say in `hypot`).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84726/new/

https://reviews.llvm.org/D84726