-
Notifications
You must be signed in to change notification settings - Fork 12.4k
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] Fix typo in long double negative block #68243
[libc] Fix typo in long double negative block #68243
Conversation
The long double version of float to string's get_negative_block had a bug in table mode. In table mode, one of the tables is named "MIN_BLOCK_2" and it stores the number of blocks that are all zeroes before the digits start for a given index. The check for long doubles was incorrectly "block_index <= MIN_BLOCK_2[idx]" when it should be "block_index < MIN_BLOCK_2[idx]" (without the equal sign). This bug caused an off-by-one error for some long double values. This patch fixes the bug and adds tests to ensure it doesn't regress.
@llvm/pr-subscribers-libc ChangesThe long double version of float to string's get_negative_block had a Full diff: https://github.com/llvm/llvm-project/pull/68243.diff 2 Files Affected:
diff --git a/libc/src/__support/float_to_string.h b/libc/src/__support/float_to_string.h
index d3fcec7a652faba..eb06cd9c08af287 100644
--- a/libc/src/__support/float_to_string.h
+++ b/libc/src/__support/float_to_string.h
@@ -708,7 +708,7 @@ FloatToString<long double>::get_negative_block(int block_index) {
const int32_t SHIFT_CONST = TABLE_SHIFT_CONST;
// if the requested block is zero
- if (block_index <= MIN_BLOCK_2[idx]) {
+ if (block_index < MIN_BLOCK_2[idx]) {
return 0;
}
const uint32_t p = POW10_OFFSET_2[idx] + block_index - MIN_BLOCK_2[idx];
diff --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index 8b9c919bed203d4..63de6f94dc1ab5e 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -2757,6 +2757,17 @@ TEST_F(LlvmLibcSPrintfTest, FloatAutoConv) {
written = LIBC_NAMESPACE::sprintf(buff, "%.10g", 0x1.0p-1074);
ASSERT_STREQ_LEN(written, buff, "4.940656458e-324");
+ written = LIBC_NAMESPACE::sprintf(buff, "%g", 0xa.aaaaaaaaaaaaaabp-7);
+ ASSERT_STREQ_LEN(written, buff, "0.0833333");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%Lg", 0xa.aaaaaaaaaaaaaabp-7L);
+ ASSERT_STREQ_LEN(written, buff, "0.0833333");
+
+ written = LIBC_NAMESPACE::sprintf(buff, "%.60Lg", 0xa.aaaaaaaaaaaaaabp-7L);
+ ASSERT_STREQ_LEN(
+ written, buff,
+ "0.0833333333333333333355920878593448009041821933351457118988037");
+
// Long double precision tests.
// These are currently commented out because they require long double support
// that isn't ready yet.
|
libc/test/src/stdio/sprintf_test.cpp
Outdated
@@ -2757,6 +2757,17 @@ TEST_F(LlvmLibcSPrintfTest, FloatAutoConv) { | |||
written = LIBC_NAMESPACE::sprintf(buff, "%.10g", 0x1.0p-1074); | |||
ASSERT_STREQ_LEN(written, buff, "4.940656458e-324"); | |||
|
|||
written = LIBC_NAMESPACE::sprintf(buff, "%g", 0xa.aaaaaaaaaaaaaabp-7); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these tests be guarded in the ifdefs
below?
The long double version of float to string's get_negative_block had a
bug in table mode. In table mode, one of the tables is named
"MIN_BLOCK_2" and it stores the number of blocks that are all zeroes
before the digits start for a given index. The check for long doubles
was incorrectly "block_index <= MIN_BLOCK_2[idx]" when it should be
"block_index < MIN_BLOCK_2[idx]" (without the equal sign). This bug
caused an off-by-one error for some long double values. This patch fixes
the bug and adds tests to ensure it doesn't regress.