From 9d624ab047d9cee0e85b1d45563843bca3d03316 Mon Sep 17 00:00:00 2001 From: Allan Renucci Date: Thu, 22 Oct 2026 03:31:39 -0700 Subject: [PATCH] Deprecate `tsl::strings::LegacyPrecision` in favor of `absl::HighPrecision`. `LegacyPrecision(float)` and `LegacyPrecision(double)` now forward to `absl::HighPrecision`, and the custom `FpToBuffer` implementation is removed. This is a value-preserving but not byte-identical change in formatting: - float: the old implementation used %.6g and fell back to %.9g. absl uses the shortest of 6-9 significant digits that round-trips, e.g. 1.17549435e-38f is now printed as "1.1754944e-38". This matches what the header comment already documented (%.6g, %.7g, %.8g, or %.9g). - double: unchanged (%.15g, falling back to %.17g). - NaN with the sign bit set is now printed as "nan" instead of "-nan". All outputs still round-trip to the exact same value. PiperOrigin-RevId: 1004642809 --- tsl/platform/BUILD | 1 - tsl/platform/numbers.cc | 79 ------------------------------------ tsl/platform/numbers.h | 40 +++++------------- tsl/platform/numbers_test.cc | 16 +------- 4 files changed, 12 insertions(+), 124 deletions(-) diff --git a/tsl/platform/BUILD b/tsl/platform/BUILD index 948214982..3f328d139 100644 --- a/tsl/platform/BUILD +++ b/tsl/platform/BUILD @@ -175,7 +175,6 @@ cc_library( srcs = ["numbers.cc"], hdrs = ["numbers.h"], deps = [ - ":stringpiece", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", diff --git a/tsl/platform/numbers.cc b/tsl/platform/numbers.cc index 99fc4c457..76af9db56 100644 --- a/tsl/platform/numbers.cc +++ b/tsl/platform/numbers.cc @@ -18,7 +18,6 @@ limitations under the License. #include #include #include -#include #include #include #include // NOLINT @@ -98,84 +97,6 @@ size_t FastUInt64ToBufferLeft(uint64_t i, char* buffer) { return FastUIntToBufferLeft(i, buffer); } -namespace { - -constexpr int NumDecimalDigits(int n) { - int count = 0; - do { - ++count; - n /= 10; - } while (n != 0); - return count; -} - -template -size_t FpToBuffer(T value, char* buffer) { - using strings_internal::kFastToBufferSize; - // Out of an abundance of caution, we ensure that the buffer is large enough - // to hold the worst-case formatting of any floating-point number. - constexpr size_t kMaxExponentDigits10 = - std::max(NumDecimalDigits(std::numeric_limits::max_exponent10), - NumDecimalDigits(std::numeric_limits::min_exponent10)); - constexpr size_t kMaxCharsWritten = - 1 + // sign bit - std::numeric_limits::max_digits10 + // decimal digits - 1 + // decimal point - 1 + // exponent character - 1 + // exponent sign - kMaxExponentDigits10; // exponent digits - static_assert(kMaxCharsWritten < kFastToBufferSize); - if (std::isnan(value)) { - int snprintf_result = absl::SNPrintF(buffer, kFastToBufferSize, "%snan", - std::signbit(value) ? "-" : ""); - // Paranoid check to ensure we don't overflow the buffer. - DCHECK(snprintf_result > 0 && snprintf_result < kFastToBufferSize); - return snprintf_result; - } - - int snprintf_result = absl::SNPrintF(buffer, kFastToBufferSize, "%.*g", - std::numeric_limits::digits10, value); - - // The snprintf should never overflow because the buffer is significantly - // larger than the precision we asked for. - DCHECK(snprintf_result > 0 && snprintf_result <= kMaxCharsWritten); - - if (auto parsed_value = AsciiToFp(buffer); parsed_value != value) { - // Round-trip conversion failed, so we need to use full precision - // formatting. - snprintf_result = - absl::SNPrintF(buffer, kFastToBufferSize, "%.*g", - std::numeric_limits::max_digits10, value); - - // Should never overflow; see above. - DCHECK(snprintf_result > 0 && snprintf_result <= kMaxCharsWritten); - } - - return snprintf_result; -} - -} // namespace - -size_t DoubleToBuffer(double value, char* buffer) { - return FpToBuffer(value, buffer); -} - -size_t FloatToBuffer(float value, char* buffer) { - return FpToBuffer(value, buffer); -} - -strings_internal::AlphaNumBuffer LegacyPrecision(double d) { - strings_internal::AlphaNumBuffer result; - result.size = DoubleToBuffer(d, result.data.data()); - return result; -} - -strings_internal::AlphaNumBuffer LegacyPrecision(float f) { - strings_internal::AlphaNumBuffer result; - result.size = FloatToBuffer(f, result.data.data()); - return result; -} - std::string FpToString(uint64_t fp) { return absl::StrCat(absl::Hex(fp, absl::kZeroPad16)); } diff --git a/tsl/platform/numbers.h b/tsl/platform/numbers.h index 34ba2a629..1ae68f046 100644 --- a/tsl/platform/numbers.h +++ b/tsl/platform/numbers.h @@ -16,44 +16,18 @@ limitations under the License. #ifndef TENSORFLOW_TSL_PLATFORM_NUMBERS_H_ #define TENSORFLOW_TSL_PLATFORM_NUMBERS_H_ -#include -#include #include #include #include #include "absl/base/macros.h" #include "absl/strings/numbers.h" +#include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" -#include "tsl/platform/stringpiece.h" namespace tsl { namespace strings { -namespace strings_internal { -// Previously documented minimums -- the buffers provided must be at least this -// long, though these numbers are subject to change: -// Int32, UInt32: 12 bytes -// Int64, UInt64, Int, Uint: 22 bytes -// Time: 30 bytes -// Use kFastToBufferSize rather than hardcoding constants. -inline constexpr int kFastToBufferSize = 32; - -// AlphaNumBuffer allows a way to pass a string to absl::StrCat without having -// to do memory allocation. It is simply a pair of a fixed-size character -// array, and a size. Please don't use outside of the "strings" package. -struct AlphaNumBuffer { - std::array data; - size_t size; - - // Support for absl::StrCat() etc. - template - friend void AbslStringify(Sink& sink, const AlphaNumBuffer& buffer) { - sink.Append(absl::string_view(buffer.data.data(), buffer.size)); - } -}; -} // namespace strings_internal - // Helper function for legacy google formatting. template const T& LegacyPrecision(const T& t) { @@ -66,13 +40,21 @@ const T& LegacyPrecision(const T& t) { // Helper function for the old strings::StrCat default "float" format, which was // either %.6g, %.7g, %.8g, or %.9g, basically the smallest string that would // round-trip back to the original float. This is fast. -strings_internal::AlphaNumBuffer LegacyPrecision(float f); +[[deprecated( + "Use absl::HighPrecision instead")]] ABSL_REFACTOR_INLINE inline auto +LegacyPrecision(float f) { + return absl::HighPrecision(f); +} // Helper function for the old strings::StrCat default "double" format, which // was either %.15g or %.17g, depending on whether the %.15g format would // round-trip back to the original double. This is approx. 20-30x slower than // the others. -strings_internal::AlphaNumBuffer LegacyPrecision(double d); +[[deprecated( + "Use absl::HighPrecision instead")]] ABSL_REFACTOR_INLINE inline auto +LegacyPrecision(double d) { + return absl::HighPrecision(d); +} // Convert a 64-bit fingerprint value to an ASCII representation. std::string FpToString(uint64_t fp); diff --git a/tsl/platform/numbers_test.cc b/tsl/platform/numbers_test.cc index a01d1ad62..e2d3f9af0 100644 --- a/tsl/platform/numbers_test.cc +++ b/tsl/platform/numbers_test.cc @@ -31,8 +31,6 @@ limitations under the License. namespace tsl { namespace strings { -using strings_internal::kFastToBufferSize; - // NOTE: most of the routines in numbers.h are tested indirectly through // strcat_test.cc in this directory. @@ -302,12 +300,6 @@ TEST(safe_strtof, Float) { EXPECT_FALSE(absl::SimpleAtof("-infinity is awesome", &result)); - // Make sure we exit cleanly if the string is too long - char test_str[2 * kFastToBufferSize]; - for (int i = 0; i < 2 * kFastToBufferSize; ++i) test_str[i] = 'a'; - test_str[kFastToBufferSize + 1] = '\0'; - EXPECT_FALSE(absl::SimpleAtof(test_str, &result)); - EXPECT_TRUE(absl::SimpleAtof("-inf", &result)); EXPECT_EQ(-std::numeric_limits::infinity(), result); @@ -340,12 +332,6 @@ TEST(safe_strtod, Double) { EXPECT_EQ(0.1234567890123, result); EXPECT_FALSE(absl::SimpleAtod("0.1234567890123abc", &result)); - // Make sure we exit cleanly if the string is too long - char test_str[2 * kFastToBufferSize]; - for (int i = 0; i < 2 * kFastToBufferSize; ++i) test_str[i] = 'a'; - test_str[kFastToBufferSize + 1] = '\0'; - EXPECT_FALSE(absl::SimpleAtod(test_str, &result)); - // Overflow to infinity, underflow to 0. EXPECT_TRUE(absl::SimpleAtod("1e310", &result)); EXPECT_EQ(std::numeric_limits::infinity(), result); @@ -474,7 +460,7 @@ TEST(LegacyPrecision, SimpleFloatToString) { EXPECT_EQ("1e+23", FloatToString(1e23f)); EXPECT_EQ("47.8", FloatToString(47.8f)); EXPECT_EQ("1000.2", FloatToString(1000.2f)); - EXPECT_EQ("1.17549435e-38", FloatToString(1.17549435e-38f)); + EXPECT_EQ("1.1754944e-38", FloatToString(1.17549435e-38f)); } void TestFloatRoundTrip(float value) {