From 97e234ac74b153f994a410de2a8adab0814a1eb1 Mon Sep 17 00:00:00 2001 From: Wyatt Hepler Date: Fri, 9 Feb 2024 19:15:52 +0000 Subject: [PATCH] pw_string: Make IntToString constexpr Move code from the .cc file to the header to enable compile-time evaluation. Change-Id: I3e997ca92e3111c32ca440fb65b5eecd11a19310 Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/190457 Reviewed-by: Alexei Frolov Commit-Queue: Auto-Submit Pigweed-Auto-Submit: Wyatt Hepler Presubmit-Verified: CQ Bot Account --- pw_string/BUILD.gn | 2 +- pw_string/CMakeLists.txt | 1 - pw_string/public/pw_string/type_to_string.h | 123 ++++++++++++++++++-- pw_string/type_to_string.cc | 116 +----------------- pw_string/type_to_string_test.cc | 22 +++- 5 files changed, 136 insertions(+), 128 deletions(-) diff --git a/pw_string/BUILD.gn b/pw_string/BUILD.gn index ef3fda62ce..fd10ba83f4 100644 --- a/pw_string/BUILD.gn +++ b/pw_string/BUILD.gn @@ -113,10 +113,10 @@ pw_source_set("to_string") { ":config", ":format", ":util", + "$dir_pw_third_party/fuchsia:stdcompat", dir_pw_span, dir_pw_status, ] - deps = [ "$dir_pw_third_party/fuchsia:stdcompat" ] # TODO: b/259746255 - Remove this when everything compiles with -Wconversion. configs = [ "$dir_pw_build:conversion_warnings" ] diff --git a/pw_string/CMakeLists.txt b/pw_string/CMakeLists.txt index 8299bfab25..17b4c1ea3d 100644 --- a/pw_string/CMakeLists.txt +++ b/pw_string/CMakeLists.txt @@ -85,7 +85,6 @@ pw_add_library(pw_string.to_string STATIC pw_string.format pw_string.util pw_status - PRIVATE_DEPS pw_third_party.fuchsia.stdcompat SOURCES type_to_string.cc diff --git a/pw_string/public/pw_string/type_to_string.h b/pw_string/public/pw_string/type_to_string.h index b461ed0076..1266d4d48e 100644 --- a/pw_string/public/pw_string/type_to_string.h +++ b/pw_string/public/pw_string/type_to_string.h @@ -21,6 +21,7 @@ #include #include +#include "lib/stdcompat/bit.h" #include "pw_span/span.h" #include "pw_status/status_with_size.h" #include "pw_string/util.h" @@ -29,7 +30,7 @@ namespace pw::string { // Returns the number of digits in the decimal representation of the provided // non-negative integer. Returns 1 for 0 or 1 + log base 10 for other numbers. -uint_fast8_t DecimalDigitCount(uint64_t integer); +constexpr uint_fast8_t DecimalDigitCount(uint64_t integer); // Returns the number of digits in the hexadecimal representation of the // provided non-negative integer. @@ -57,7 +58,7 @@ constexpr uint_fast8_t HexDigitCount(uint64_t integer) { // sites pass their arguments directly and casting instructions are shared. // template -StatusWithSize IntToString(T value, span buffer) { +constexpr StatusWithSize IntToString(T value, span buffer) { if constexpr (std::is_signed_v) { return IntToString(value, buffer); } else { @@ -65,12 +66,6 @@ StatusWithSize IntToString(T value, span buffer) { } } -template <> -StatusWithSize IntToString(uint64_t value, span buffer); - -template <> -StatusWithSize IntToString(int64_t value, span buffer); - // Writes an integer as a hexadecimal string. Semantics match IntToString. The // output is lowercase without a leading 0x. min_width adds leading zeroes such // that the final string is at least the specified number of characters wide. @@ -160,4 +155,116 @@ inline StatusWithSize CopyEntireStringOrNull(const char* value, template StatusWithSize UnknownTypeToString(const T& value, span buffer); +// Implementations +namespace internal { + +// Powers of 10 (except 0) as an array. This table is fairly large (160 B), but +// avoids having to recalculate these values for each DecimalDigitCount call. +inline constexpr std::array kPowersOf10{ + 0ull, + 10ull, // 10^1 + 100ull, // 10^2 + 1000ull, // 10^3 + 10000ull, // 10^4 + 100000ull, // 10^5 + 1000000ull, // 10^6 + 10000000ull, // 10^7 + 100000000ull, // 10^8 + 1000000000ull, // 10^9 + 10000000000ull, // 10^10 + 100000000000ull, // 10^11 + 1000000000000ull, // 10^12 + 10000000000000ull, // 10^13 + 100000000000000ull, // 10^14 + 1000000000000000ull, // 10^15 + 10000000000000000ull, // 10^16 + 100000000000000000ull, // 10^17 + 1000000000000000000ull, // 10^18 + 10000000000000000000ull, // 10^19 +}; + +constexpr StatusWithSize HandleExhaustedBuffer(span buffer) { + if (!buffer.empty()) { + buffer[0] = '\0'; + } + return StatusWithSize::ResourceExhausted(); +} + +} // namespace internal + +constexpr uint_fast8_t DecimalDigitCount(uint64_t integer) { + // This fancy piece of code takes the log base 2, then approximates the + // change-of-base formula by multiplying by 1233 / 4096. + const uint_fast8_t log_10 = static_cast( + (64 - cpp20::countl_zero(integer | 1)) * 1233 >> 12); + + // Adjust the estimated log base 10 by comparing against the power of 10. + return static_cast( + log_10 + (integer < internal::kPowersOf10[log_10] ? 0u : 1u)); +} + +// std::to_chars is available for integers in recent versions of GCC. I looked +// into switching to std::to_chars instead of this implementation. std::to_chars +// increased binary size by 160 B on an -Os build (even after removing +// DecimalDigitCount and its table). I didn't measure performance, but I don't +// think std::to_chars will be faster, so I kept this implementation for now. +template <> +constexpr StatusWithSize IntToString(uint64_t value, span buffer) { + constexpr uint32_t base = 10; + constexpr uint32_t max_uint32_base_power = 1'000'000'000; + constexpr uint_fast8_t max_uint32_base_power_exponent = 9; + + const uint_fast8_t total_digits = DecimalDigitCount(value); + + if (total_digits >= buffer.size()) { + return internal::HandleExhaustedBuffer(buffer); + } + + buffer[total_digits] = '\0'; + + uint_fast8_t remaining = total_digits; + while (remaining > 0u) { + uint32_t lower_digits = 0; // the value of the lower digits to write + uint_fast8_t digit_count = 0; // the number of lower digits to write + + // 64-bit division is slow on 32-bit platforms, so print large numbers in + // 32-bit chunks to minimize the number of 64-bit divisions. + if (value <= std::numeric_limits::max()) { + lower_digits = static_cast(value); + digit_count = remaining; + } else { + lower_digits = static_cast(value % max_uint32_base_power); + digit_count = max_uint32_base_power_exponent; + value /= max_uint32_base_power; + } + + // Write the specified number of digits, with leading 0s. + for (uint_fast8_t i = 0; i < digit_count; ++i) { + buffer[--remaining] = static_cast(lower_digits % base + '0'); + lower_digits /= base; + } + } + return StatusWithSize(total_digits); +} + +template <> +constexpr StatusWithSize IntToString(int64_t value, span buffer) { + if (value >= 0) { + return IntToString(static_cast(value), buffer); + } + + // Write as an unsigned number, but leave room for the leading minus sign. + // Do not use std::abs since it fails for the minimum value integer. + const uint64_t absolute_value = -static_cast(value); + auto result = IntToString( + absolute_value, buffer.empty() ? buffer : buffer.subspan(1)); + + if (result.ok()) { + buffer[0] = '-'; + return StatusWithSize(result.size() + 1); + } + + return internal::HandleExhaustedBuffer(buffer); +} + } // namespace pw::string diff --git a/pw_string/type_to_string.cc b/pw_string/type_to_string.cc index cbe9cfecda..1f433aa225 100644 --- a/pw_string/type_to_string.cc +++ b/pw_string/type_to_string.cc @@ -23,96 +23,6 @@ #include "lib/stdcompat/bit.h" namespace pw::string { -namespace { - -// Powers of 10 (except 0) as an array. This table is fairly large (160 B), but -// avoids having to recalculate these values for each DecimalDigitCount call. -constexpr std::array kPowersOf10{ - 0ull, - 10ull, // 10^1 - 100ull, // 10^2 - 1000ull, // 10^3 - 10000ull, // 10^4 - 100000ull, // 10^5 - 1000000ull, // 10^6 - 10000000ull, // 10^7 - 100000000ull, // 10^8 - 1000000000ull, // 10^9 - 10000000000ull, // 10^10 - 100000000000ull, // 10^11 - 1000000000000ull, // 10^12 - 10000000000000ull, // 10^13 - 100000000000000ull, // 10^14 - 1000000000000000ull, // 10^15 - 10000000000000000ull, // 10^16 - 100000000000000000ull, // 10^17 - 1000000000000000000ull, // 10^18 - 10000000000000000000ull, // 10^19 -}; - -StatusWithSize HandleExhaustedBuffer(span buffer) { - if (!buffer.empty()) { - buffer[0] = '\0'; - } - return StatusWithSize::ResourceExhausted(); -} - -} // namespace - -uint_fast8_t DecimalDigitCount(uint64_t integer) { - // This fancy piece of code takes the log base 2, then approximates the - // change-of-base formula by multiplying by 1233 / 4096. - const uint_fast8_t log_10 = static_cast( - (64 - cpp20::countl_zero(integer | 1)) * 1233 >> 12); - - // Adjust the estimated log base 10 by comparing against the power of 10. - return static_cast(log_10 + - (integer < kPowersOf10[log_10] ? 0u : 1u)); -} - -// std::to_chars is available for integers in recent versions of GCC. I looked -// into switching to std::to_chars instead of this implementation. std::to_chars -// increased binary size by 160 B on an -Os build (even after removing -// DecimalDigitCount and its table). I didn't measure performance, but I don't -// think std::to_chars will be faster, so I kept this implementation for now. -template <> -StatusWithSize IntToString(uint64_t value, span buffer) { - constexpr uint32_t base = 10; - constexpr uint32_t max_uint32_base_power = 1'000'000'000; - constexpr uint_fast8_t max_uint32_base_power_exponent = 9; - - const uint_fast8_t total_digits = DecimalDigitCount(value); - - if (total_digits >= buffer.size()) { - return HandleExhaustedBuffer(buffer); - } - - buffer[total_digits] = '\0'; - - uint_fast8_t remaining = total_digits; - while (remaining > 0u) { - uint32_t lower_digits; // the value of the lower digits to write - uint_fast8_t digit_count; // the number of lower digits to write - - // 64-bit division is slow on 32-bit platforms, so print large numbers in - // 32-bit chunks to minimize the number of 64-bit divisions. - if (value <= std::numeric_limits::max()) { - lower_digits = static_cast(value); - digit_count = remaining; - } else { - lower_digits = static_cast(value % max_uint32_base_power); - digit_count = max_uint32_base_power_exponent; - value /= max_uint32_base_power; - } - - // Write the specified number of digits, with leading 0s. - for (uint_fast8_t i = 0; i < digit_count; ++i) { - buffer[--remaining] = static_cast(lower_digits % base + '0'); - lower_digits /= base; - } - } - return StatusWithSize(total_digits); -} StatusWithSize IntToHexString(uint64_t value, span buffer, @@ -120,7 +30,7 @@ StatusWithSize IntToHexString(uint64_t value, const uint_fast8_t digits = std::max(HexDigitCount(value), min_width); if (digits >= buffer.size()) { - return HandleExhaustedBuffer(buffer); + return internal::HandleExhaustedBuffer(buffer); } for (int i = static_cast(digits) - 1; i >= 0; --i) { @@ -132,26 +42,6 @@ StatusWithSize IntToHexString(uint64_t value, return StatusWithSize(digits); } -template <> -StatusWithSize IntToString(int64_t value, span buffer) { - if (value >= 0) { - return IntToString(static_cast(value), buffer); - } - - // Write as an unsigned number, but leave room for the leading minus sign. - // Do not use std::abs since it fails for the minimum value integer. - const uint64_t absolute_value = -static_cast(value); - auto result = IntToString( - absolute_value, buffer.empty() ? buffer : buffer.subspan(1)); - - if (result.ok()) { - buffer[0] = '-'; - return StatusWithSize(result.size() + 1); - } - - return HandleExhaustedBuffer(buffer); -} - StatusWithSize FloatAsIntToString(float value, span buffer) { // If it's finite and fits in an int64_t, print it as a rounded integer. if (std::isfinite(value) && @@ -171,7 +61,7 @@ StatusWithSize FloatAsIntToString(float value, span buffer) { return StatusWithSize(written); } - return HandleExhaustedBuffer(buffer); + return internal::HandleExhaustedBuffer(buffer); } StatusWithSize BoolToString(bool value, span buffer) { @@ -188,7 +78,7 @@ StatusWithSize PointerToString(const void* pointer, span buffer) { StatusWithSize CopyEntireStringOrNull(const std::string_view& value, span buffer) { if (value.size() >= buffer.size()) { - return HandleExhaustedBuffer(buffer); + return internal::HandleExhaustedBuffer(buffer); } std::memcpy(buffer.data(), value.data(), value.size()); diff --git a/pw_string/type_to_string_test.cc b/pw_string/type_to_string_test.cc index ef46fa3414..3afe39914f 100644 --- a/pw_string/type_to_string_test.cc +++ b/pw_string/type_to_string_test.cc @@ -90,13 +90,13 @@ TEST(Digits, HexDigits_16) { } } +constexpr char kStartingString[] = "!@#$%^&*()!@#$%^&*()"; +constexpr char kUint64Max[] = "18446744073709551615"; +constexpr char kInt64Min[] = "-9223372036854775808"; +constexpr char kInt64Max[] = "9223372036854775807"; + class TestWithBuffer : public ::testing::Test { protected: - static constexpr char kStartingString[] = "!@#$%^&*()!@#$%^&*()"; - static constexpr char kUint64Max[] = "18446744073709551615"; - static constexpr char kInt64Min[] = "-9223372036854775808"; - static constexpr char kInt64Max[] = "9223372036854775807"; - static_assert(sizeof(kStartingString) == sizeof(kUint64Max)); TestWithBuffer() { std::memcpy(buffer_, kStartingString, sizeof(buffer_)); } @@ -104,6 +104,18 @@ class TestWithBuffer : public ::testing::Test { char buffer_[sizeof(kUint64Max)]; }; +static_assert([] { + char buffer[sizeof(kUint64Max)] = {}; + auto result = IntToString(std::numeric_limits::max(), buffer); + return result.ok() && std::string_view(buffer, result.size()) == kUint64Max; +}()); + +static_assert([] { + char buffer[sizeof(kInt64Min)] = {}; + auto result = IntToString(std::numeric_limits::min(), buffer); + return result.ok() && std::string_view(buffer, result.size()) == kInt64Min; +}()); + class IntToStringTest : public TestWithBuffer {}; TEST_F(IntToStringTest, Unsigned_EmptyBuffer_WritesNothing) {