Skip to content

Conversation

makslevental
Copy link
Contributor

@makslevental makslevental commented Sep 20, 2025

This test fails under -Werror=implicit-float-conversion (like in our pre-merge CI) because dsqrtl returns a double

LIBC_INLINE static constexpr double dsqrtl(long double x) {

Fixes #159932

@llvmbot
Copy link
Member

llvmbot commented Sep 20, 2025

@llvm/pr-subscribers-libc

Author: Maksim Levental (makslevental)

Changes

This test fails under -Werror=implicit-float-conversion (like in our pre-merge CI) because dsqrtl returns a double

LIBC_INLINE static constexpr double dsqrtl(long double x) {

Fixes #159932


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

1 Files Affected:

  • (modified) libc/test/shared/shared_math_test.cpp (+1-1)
diff --git a/libc/test/shared/shared_math_test.cpp b/libc/test/shared/shared_math_test.cpp
index 9e4d4d52e7f5e..bfff5ecb8cf81 100644
--- a/libc/test/shared/shared_math_test.cpp
+++ b/libc/test/shared/shared_math_test.cpp
@@ -57,7 +57,7 @@ TEST(LlvmLibcSharedMathTest, AllFloat) {
   EXPECT_FP_EQ(0x1p+0f, LIBC_NAMESPACE::shared::cosf(0.0f));
   EXPECT_FP_EQ(0x1p+0f, LIBC_NAMESPACE::shared::coshf(0.0f));
   EXPECT_FP_EQ(0x1p+0f, LIBC_NAMESPACE::shared::cospif(0.0f));
-  EXPECT_FP_EQ(0x0p+0f, LIBC_NAMESPACE::shared::dsqrtl(0.0f));
+  EXPECT_FP_EQ(0x0p+0f, (float)LIBC_NAMESPACE::shared::dsqrtl(0.0f));
   EXPECT_FP_EQ(0x0p+0f, LIBC_NAMESPACE::shared::erff(0.0f));
   EXPECT_FP_EQ(0x1p+0f, LIBC_NAMESPACE::shared::exp10f(0.0f));
   EXPECT_FP_EQ(0x1p+0f, LIBC_NAMESPACE::shared::expf(0.0f));

@makslevental
Copy link
Contributor Author

Note, I'm not sure if this is the right - the alternative could be

EXPECT_FP_EQ(0x0p+0, LIBC_NAMESPACE::shared::dsqrtl(0.0));

but all the values in this test are single 🤷.

@@ -57,7 +57,7 @@ TEST(LlvmLibcSharedMathTest, AllFloat) {
EXPECT_FP_EQ(0x1p+0f, LIBC_NAMESPACE::shared::cosf(0.0f));
EXPECT_FP_EQ(0x1p+0f, LIBC_NAMESPACE::shared::coshf(0.0f));
EXPECT_FP_EQ(0x1p+0f, LIBC_NAMESPACE::shared::cospif(0.0f));
EXPECT_FP_EQ(0x0p+0f, LIBC_NAMESPACE::shared::dsqrtl(0.0f));
EXPECT_FP_EQ(0x0p+0f, (float)LIBC_NAMESPACE::shared::dsqrtl(0.0f));
Copy link
Contributor

Choose a reason for hiding this comment

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

Either just use the double constant without the f or use a C++ cast.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@jhuber6 jhuber6 Sep 20, 2025

Choose a reason for hiding this comment

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

If it fixes the warning without the f constant then that's probably the better solution, since technically the double result could be outisde a float's ULP and get rounded to zero. Though the chances of that happening are quite slim.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@makslevental makslevental force-pushed the users/makslevental/fix-libc branch from f40bc6f to 44882b2 Compare September 20, 2025 16:41
@makslevental makslevental force-pushed the users/makslevental/fix-libc branch from 44882b2 to 2db3b36 Compare September 20, 2025 16:54
… 'double' to 'float' in shared_math_test.cpp
@makslevental makslevental force-pushed the users/makslevental/fix-libc branch from 2db3b36 to e6bc6f8 Compare September 20, 2025 16:55
@jhuber6 jhuber6 enabled auto-merge (squash) September 20, 2025 16:56
@jhuber6 jhuber6 merged commit c1d838e into main Sep 20, 2025
19 checks passed
@jhuber6 jhuber6 deleted the users/makslevental/fix-libc branch September 20, 2025 17:07
@lntue
Copy link
Contributor

lntue commented Sep 21, 2025

Note, I'm not sure if this is the right - the alternative could be

EXPECT_FP_EQ(0x0p+0, LIBC_NAMESPACE::shared::dsqrtl(0.0));

but all the values in this test are single 🤷.

The input should be 0.0L and the output should be 0.0

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.

[libc]: error: implicit conversion loses floating-point precision: 'double' to 'float'
4 participants