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

Tue Ly via Phabricator via libc-commits libc-commits at lists.llvm.org
Wed Aug 26 06:21:59 PDT 2020

lntue added inline comments.

Comment at: libc/test/src/math/sqrt_test.cpp:23
+double nan = FPBits::buildNaN(1);
+double inf = FPBits::inf();
hungptit wrote:
> NIT: constexpr?
constexpr does not work with floating point yet.

Comment at: libc/test/src/math/sqrtf_test.cpp:33
+  ASSERT_EQ(-0.0f, __llvm_libc::sqrtf(-0.0f));
+  ASSERT_EQ(nan, __llvm_libc::sqrtf(-1.0f));
+  ASSERT_EQ(1.0f, __llvm_libc::sqrtf(1.0f));
hungptit wrote:
> Do we plan to raise [[ https://en.cppreference.com/w/cpp/numeric/fenv/FE_exceptions | FE_INVALID ]]?
Floating point exceptions will be implemented in the future.  Currently we prioritize expanding functionality first, unless there are immediate requests/needs.

Comment at: libc/utils/FPUtil/SqrtLongDoubleX86.h:129
+    // Extract output
+    FPBits<T> out(0.0L);
+    out.exponent = x_exp;
hungptit wrote:
> Do these clang-tidy go away if we rename this variable to extractedOutput? If we give this variable a better name then the above comment is redundant.
I think clang-tidy wants the variables' initials to be capitalized;  it's a bit noisy

  rG LLVM Github Monorepo



More information about the libc-commits mailing list