From 83c149d5c97dc9ba5e9b416aa6db487db45f2c9b Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Fri, 13 Dec 2024 14:55:02 -0800 Subject: [PATCH] Migrate some TSL code over to ABSL equivalents No functional change is intended. PiperOrigin-RevId: 706010982 --- tsl/platform/BUILD | 4 + tsl/platform/numbers.cc | 215 +++-------------------------------- tsl/platform/numbers.h | 53 +++++---- tsl/platform/numbers_test.cc | 17 +-- 4 files changed, 59 insertions(+), 230 deletions(-) diff --git a/tsl/platform/BUILD b/tsl/platform/BUILD index f160011f6..1cc83c02f 100644 --- a/tsl/platform/BUILD +++ b/tsl/platform/BUILD @@ -229,6 +229,8 @@ cc_library( ":stringpiece", ":stringprintf", ":types", + "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/strings", "@double_conversion//:double-conversion", ], ) @@ -1720,6 +1722,8 @@ tsl_cc_test( ":numbers", ":test", ":test_main", + ":types", + "@com_google_absl//absl/strings", ], ) diff --git a/tsl/platform/numbers.cc b/tsl/platform/numbers.cc index 7239e6fff..a675403f4 100644 --- a/tsl/platform/numbers.cc +++ b/tsl/platform/numbers.cc @@ -20,14 +20,16 @@ limitations under the License. #include #include -#include +#include #include #include #include +#include +#include // NOLINT #include -#include "double-conversion/double-conversion.h" -#include "tsl/platform/str_util.h" +#include "absl/strings/str_cat.h" +#include "absl/strings/string_view.h" #include "tsl/platform/logging.h" #include "tsl/platform/macros.h" #include "tsl/platform/stringprintf.h" @@ -114,17 +116,6 @@ T locale_independent_strtonum(const char* str, const char** endptr) { return result; } -static inline const double_conversion::StringToDoubleConverter& -StringToFloatConverter() { - static const double_conversion::StringToDoubleConverter converter( - double_conversion::StringToDoubleConverter::ALLOW_LEADING_SPACES | - double_conversion::StringToDoubleConverter::ALLOW_HEX | - double_conversion::StringToDoubleConverter::ALLOW_TRAILING_SPACES | - double_conversion::StringToDoubleConverter::ALLOW_CASE_INSENSIBILITY, - 0., 0., "inf", "nan"); - return converter; -} - } // namespace namespace strings { @@ -219,154 +210,6 @@ size_t DoubleToBuffer(double value, char* buffer) { return snprintf_result; } -namespace { -char SafeFirstChar(absl::string_view str) { - if (str.empty()) return '\0'; - return str[0]; -} -void SkipSpaces(absl::string_view* str) { - while (isspace(SafeFirstChar(*str))) str->remove_prefix(1); -} -} // namespace - -bool safe_strto64(absl::string_view str, int64_t* value) { - SkipSpaces(&str); - - int64_t vlimit = kint64max; - int sign = 1; - if (absl::ConsumePrefix(&str, "-")) { - sign = -1; - // Different limit for positive and negative integers. - vlimit = kint64min; - } - - if (!isdigit(SafeFirstChar(str))) return false; - - int64_t result = 0; - if (sign == 1) { - do { - int digit = SafeFirstChar(str) - '0'; - if ((vlimit - digit) / 10 < result) { - return false; - } - result = result * 10 + digit; - str.remove_prefix(1); - } while (isdigit(SafeFirstChar(str))); - } else { - do { - int digit = SafeFirstChar(str) - '0'; - if ((vlimit + digit) / 10 > result) { - return false; - } - result = result * 10 - digit; - str.remove_prefix(1); - } while (isdigit(SafeFirstChar(str))); - } - - SkipSpaces(&str); - if (!str.empty()) return false; - - *value = result; - return true; -} - -bool safe_strtou64(absl::string_view str, uint64_t* value) { - SkipSpaces(&str); - if (!isdigit(SafeFirstChar(str))) return false; - - uint64_t result = 0; - do { - int digit = SafeFirstChar(str) - '0'; - if ((kuint64max - digit) / 10 < result) { - return false; - } - result = result * 10 + digit; - str.remove_prefix(1); - } while (isdigit(SafeFirstChar(str))); - - SkipSpaces(&str); - if (!str.empty()) return false; - - *value = result; - return true; -} - -bool safe_strto32(absl::string_view str, int32_t* value) { - SkipSpaces(&str); - - int64_t vmax = kint32max; - int sign = 1; - if (absl::ConsumePrefix(&str, "-")) { - sign = -1; - // Different max for positive and negative integers. - ++vmax; - } - - if (!isdigit(SafeFirstChar(str))) return false; - - int64_t result = 0; - do { - result = result * 10 + SafeFirstChar(str) - '0'; - if (result > vmax) { - return false; - } - str.remove_prefix(1); - } while (isdigit(SafeFirstChar(str))); - - SkipSpaces(&str); - - if (!str.empty()) return false; - - *value = static_cast(result * sign); - return true; -} - -bool safe_strtou32(absl::string_view str, uint32_t* value) { - SkipSpaces(&str); - if (!isdigit(SafeFirstChar(str))) return false; - - int64_t result = 0; - do { - result = result * 10 + SafeFirstChar(str) - '0'; - if (result > kuint32max) { - return false; - } - str.remove_prefix(1); - } while (isdigit(SafeFirstChar(str))); - - SkipSpaces(&str); - if (!str.empty()) return false; - - *value = static_cast(result); - return true; -} - -bool safe_strtof(absl::string_view str, float* value) { - int processed_characters_count = -1; - auto len = str.size(); - - // If string length exceeds buffer size or int max, fail. - if (len >= kFastToBufferSize) return false; - if (len > std::numeric_limits::max()) return false; - - *value = StringToFloatConverter().StringToFloat( - str.data(), static_cast(len), &processed_characters_count); - return processed_characters_count > 0; -} - -bool safe_strtod(absl::string_view str, double* value) { - int processed_characters_count = -1; - auto len = str.size(); - - // If string length exceeds buffer size or int max, fail. - if (len >= kFastToBufferSize) return false; - if (len > std::numeric_limits::max()) return false; - - *value = StringToFloatConverter().StringToDouble( - str.data(), static_cast(len), &processed_characters_count); - return processed_characters_count > 0; -} - size_t FloatToBuffer(float value, char* buffer) { // FLT_DIG is 6 for IEEE-754 floats, which are used on almost all // platforms these days. Just in case some system exists where FLT_DIG @@ -401,51 +244,21 @@ size_t FloatToBuffer(float value, char* buffer) { } std::string FpToString(Fprint fp) { - char buf[17]; - snprintf(buf, sizeof(buf), "%016llx", static_cast(fp)); - return std::string(buf); + return absl::StrCat(absl::Hex(fp, absl::kZeroPad16)); } -bool StringToFp(const std::string& s, Fprint* fp) { - char junk; - uint64_t result; - if (sscanf(s.c_str(), "%" SCNx64 "%c", &result, &junk) == 1) { - *fp = result; - return true; - } else { +bool HexStringToUint64(absl::string_view s, uint64_t* result) { + auto end_ptr = s.data() + s.size(); + uint64_t parsed_result; + auto [ptr, ec] = + std::from_chars(s.data(), end_ptr, parsed_result, /*base=*/16); + if (ec != std::errc{}) { return false; } -} - -absl::string_view Uint64ToHexString(uint64_t v, char* buf) { - static const char* hexdigits = "0123456789abcdef"; - const int num_byte = 16; - buf[num_byte] = '\0'; - for (int i = num_byte - 1; i >= 0; i--) { - buf[i] = hexdigits[v & 0xf]; - v >>= 4; - } - return absl::string_view(buf, num_byte); -} - -bool HexStringToUint64(const absl::string_view& s, uint64_t* result) { - uint64_t v = 0; - if (s.empty()) { + if (ptr != end_ptr) { return false; } - for (size_t i = 0; i < s.size(); i++) { - char c = s[i]; - if (c >= '0' && c <= '9') { - v = (v << 4) + (c - '0'); - } else if (c >= 'a' && c <= 'f') { - v = (v << 4) + 10 + (c - 'a'); - } else if (c >= 'A' && c <= 'F') { - v = (v << 4) + 10 + (c - 'A'); - } else { - return false; - } - } - *result = v; + *result = parsed_result; return true; } diff --git a/tsl/platform/numbers.h b/tsl/platform/numbers.h index 0d62f4253..ab21c23db 100644 --- a/tsl/platform/numbers.h +++ b/tsl/platform/numbers.h @@ -16,9 +16,12 @@ limitations under the License. #ifndef TENSORFLOW_TSL_PLATFORM_NUMBERS_H_ #define TENSORFLOW_TSL_PLATFORM_NUMBERS_H_ +#include #include #include +#include "absl/base/macros.h" +#include "absl/strings/numbers.h" #include "tsl/platform/stringpiece.h" #include "tsl/platform/types.h" @@ -46,7 +49,7 @@ namespace strings { // Int64, UInt64, Int, Uint: 22 bytes // Time: 30 bytes // Use kFastToBufferSize rather than hardcoding constants. -static const int kFastToBufferSize = 32; +inline constexpr int kFastToBufferSize = 32; // ---------------------------------------------------------------------- // FastInt32ToBufferLeft() @@ -77,52 +80,60 @@ size_t FloatToBuffer(float value, char* buffer); // Convert a 64-bit fingerprint value to an ASCII representation. std::string FpToString(Fprint fp); -// Attempt to parse a fingerprint in the form encoded by FpToString. If -// successful, stores the fingerprint in *fp and returns true. Otherwise, -// returns false. -bool StringToFp(const std::string& s, Fprint* fp); - -// Convert a 64-bit fingerprint value to an ASCII representation that -// is terminated by a '\0'. -// Buf must point to an array of at least kFastToBufferSize characters -absl::string_view Uint64ToHexString(uint64_t v, char* buf); - -// Attempt to parse a uint64 in the form encoded by FastUint64ToHexString. If -// successful, stores the value in *v and returns true. Otherwise, -// returns false. -bool HexStringToUint64(const absl::string_view& s, uint64_t* result); +// Attempt to parse a `uint64_t` in the form encoded by +// `absl::StrCat(absl::Hex(*result))`. If successful, stores the value in +// `result` and returns true. Otherwise, returns false. +bool HexStringToUint64(absl::string_view s, uint64_t* result); // Convert strings to 32bit integer values. // Leading and trailing spaces are allowed. // Return false with overflow or invalid input. -bool safe_strto32(absl::string_view str, int32_t* value); +ABSL_DEPRECATE_AND_INLINE() +inline bool safe_strto32(absl::string_view str, int32_t* value) { + return absl::SimpleAtoi(str, value); +} // Convert strings to unsigned 32bit integer values. // Leading and trailing spaces are allowed. // Return false with overflow or invalid input. -bool safe_strtou32(absl::string_view str, uint32_t* value); +ABSL_DEPRECATE_AND_INLINE() +inline bool safe_strtou32(absl::string_view str, uint32_t* value) { + return absl::SimpleAtoi(str, value); +} // Convert strings to 64bit integer values. // Leading and trailing spaces are allowed. // Return false with overflow or invalid input. -bool safe_strto64(absl::string_view str, int64_t* value); +ABSL_DEPRECATE_AND_INLINE() +inline bool safe_strto64(absl::string_view str, int64_t* value) { + return absl::SimpleAtoi(str, value); +} // Convert strings to unsigned 64bit integer values. // Leading and trailing spaces are allowed. // Return false with overflow or invalid input. -bool safe_strtou64(absl::string_view str, uint64_t* value); +ABSL_DEPRECATE_AND_INLINE() +inline bool safe_strtou64(absl::string_view str, uint64_t* value) { + return absl::SimpleAtoi(str, value); +} // Convert strings to floating point values. // Leading and trailing spaces are allowed. // Values may be rounded on over- and underflow. // Returns false on invalid input or if `strlen(value) >= kFastToBufferSize`. -bool safe_strtof(absl::string_view str, float* value); +ABSL_DEPRECATE_AND_INLINE() +inline bool safe_strtof(absl::string_view str, float* value) { + return absl::SimpleAtof(str, value); +} // Convert strings to double precision floating point values. // Leading and trailing spaces are allowed. // Values may be rounded on over- and underflow. // Returns false on invalid input or if `strlen(value) >= kFastToBufferSize`. -bool safe_strtod(absl::string_view str, double* value); +ABSL_DEPRECATE_AND_INLINE() +inline bool safe_strtod(absl::string_view str, double* value) { + return absl::SimpleAtod(str, value); +} inline bool ProtoParseNumeric(absl::string_view s, int32_t* value) { return safe_strto32(s, value); diff --git a/tsl/platform/numbers_test.cc b/tsl/platform/numbers_test.cc index 0ce574e59..69590ba9d 100644 --- a/tsl/platform/numbers_test.cc +++ b/tsl/platform/numbers_test.cc @@ -18,7 +18,9 @@ limitations under the License. #include #include +#include "absl/strings/str_cat.h" #include "tsl/platform/test.h" +#include "tsl/platform/types.h" namespace tsl { namespace strings { @@ -26,29 +28,28 @@ namespace strings { // NOTE: most of the routines in numbers.h are tested indirectly through // strcat_test.cc in this directory. -// Test StrCat of ints and longs of various sizes and signdedness. +// Test StrCat of ints and longs of various sizes and signedness. TEST(FpToString, Ints) { for (int s = 0; s < 64; s++) { for (int delta = -1; delta <= 1; delta++) { uint64 fp = (1ull << s) + delta; string s = FpToString(fp); uint64 fp2; - EXPECT_TRUE(StringToFp(s, &fp2)); + EXPECT_TRUE(HexStringToUint64(s, &fp2)); EXPECT_EQ(fp, fp2); } } Fprint dummy; - EXPECT_FALSE(StringToFp("", &dummy)); - EXPECT_FALSE(StringToFp("xyz", &dummy)); - EXPECT_FALSE(StringToFp("0000000000000000xyz", &dummy)); + EXPECT_FALSE(HexStringToUint64("", &dummy)); + EXPECT_FALSE(HexStringToUint64("xyz", &dummy)); + EXPECT_FALSE(HexStringToUint64("0000000000000000xyz", &dummy)); } TEST(Uint64ToHexString, Ints) { for (int s = 0; s < 64; s++) { for (int delta = -1; delta <= 1; delta++) { uint64 fp = (1ull << s) + delta; - char buf[kFastToBufferSize]; - absl::string_view s = Uint64ToHexString(fp, buf); + std::string s = absl::StrCat(absl::Hex(fp, absl::kZeroPad16)); uint64 fp2; EXPECT_TRUE(HexStringToUint64(s, &fp2)); EXPECT_EQ(fp, fp2) << s; @@ -353,7 +354,7 @@ TEST(safe_strtod, Double) { EXPECT_TRUE(safe_strtod("\t82.0\t ", &result)); EXPECT_EQ(82.0, result); - EXPECT_FALSE(safe_strtod("infinity", &result)); + EXPECT_TRUE(safe_strtod("infinity", &result)); EXPECT_TRUE(safe_strtod("-inf", &result)); EXPECT_EQ(-std::numeric_limits::infinity(), result);