From eb793d2b8231cb554d651b11efd7fbf9a50260f7 Mon Sep 17 00:00:00 2001 From: Marco Nilsson Date: Sun, 13 Sep 2026 11:33:42 +0200 Subject: [PATCH] Help the compiler optimize unaligned_type. (#1598) * Help the compiler optimize unaligned_type. This fixes a regression in 266b0f99 when the type was made constexpr. When switching to a loop to construct the value the compiler was unable to detect what was going on. We now unroll the loop at compile time into what's essentially a hand rolled shift and OR operation which the compiler detects and replaces with a byte swap. Tested on gcc (x86+arm) and clang (x86). Introduce a static value_from which can be used to convert a value from an existing buffer. There were previously three options for this, all bad or awkward in different ways: - val = etl::be_uint32_t(address) Copies to temporary storage. - val = *reinterpret_cast(address); Unreadable. - etl::be_uint32_t::copy_store_to_value(address, output); Requires an output parameter. The new addition is: - val = etl::be_uint32_t::value_from(address); Additionally, update the documentation with value_from and the missing, existing constructors. * Please clang-format. --------- Co-authored-by: John Wellbelove --- docs/types/unaligned-type.md | 66 +++++++++++++++-- include/etl/unaligned_type.h | 135 +++++++++++++++++++++++------------ test/test_unaligned_type.cpp | 105 +++++++++++++++++++++++++++ 3 files changed, 256 insertions(+), 50 deletions(-) diff --git a/docs/types/unaligned-type.md b/docs/types/unaligned-type.md index 0f6689a9..28c9f85f 100644 --- a/docs/types/unaligned-type.md +++ b/docs/types/unaligned-type.md @@ -166,6 +166,50 @@ Constructs with the supplied value. --- +```cpp +unaligned_type(const void* address) +``` +**Description** +Constructs from the `Size` bytes at the address. +From: `20.39.5` + +--- + +```cpp +unaligned_type(const void* address, size_t buffer_size) +``` +**Description** +Constructs from the `Size` bytes at the address. +`buffer_size` must be greater than or equal to `Size`. This is a precondition, +checked by `ETL_ASSERT`. If checks are disabled then a smaller buffer size +results in a read beyond the end of the buffer. +From: `20.39.5` + +--- + +```cpp +ETL_CONSTEXPR14 unaligned_type(const unsigned char* address) +``` +**Description** +Constructs from the `Size` bytes at the address. +Unlike the `const void*` overload, this requires no `reinterpret_cast` and so +may be used in a constexpr context. +From: `20.48.1` + +--- + +```cpp +ETL_CONSTEXPR14 unaligned_type(const unsigned char* address, size_t buffer_size) +``` +**Description** +Constructs from the `Size` bytes at the address. +`buffer_size` must be greater than or equal to `Size`. This is a precondition, +checked by `ETL_ASSERT`. If checks are disabled then a smaller buffer size +results in a read beyond the end of the buffer. +From: `20.48.1` + +--- + ```cpp template unaligned_type(const unaligned_type& other) @@ -176,10 +220,11 @@ The endianness is converted, if necessary. ## Member types ```cpp -using pointer = char*; -using const_pointer = const char*; -using iterator = char*; -using const_iterator = const char*; +using storage_type = unsigned char; +using pointer = unsigned char*; +using const_pointer = const unsigned char*; +using iterator = unsigned char*; +using const_iterator = const unsigned char*; using reverse_iterator = etl::reverse_iterator; using const_reverse_iterator = etl::reverse_iterator; ``` @@ -306,7 +351,7 @@ Const reverse iterator to the end of the storage. --- ```cpp -char& operator[](int i) +storage_type& operator[](int i) ``` **Description** Index operator. @@ -314,7 +359,7 @@ Index operator. --- ```cpp -ETL_CONSTEXPR14 const char& operator[](int i) const +ETL_CONSTEXPR14 const storage_type& operator[](int i) const ``` **Description** Const index operator. @@ -345,6 +390,15 @@ Gets the value. --- +```cpp +static ETL_CONSTEXPR14 T value_from(const_pointer address) +``` +**Description** +Gets the value of the `Size` bytes at the address. +Unlike construction from an address, the bytes are not copied to storage first. + +--- + ```cpp template ETL_CONSTEXPR14 unaligned_type& operator =(const unaligned_type& other) diff --git a/include/etl/unaligned_type.h b/include/etl/unaligned_type.h index cba1874f..6d8ef74c 100644 --- a/include/etl/unaligned_type.h +++ b/include/etl/unaligned_type.h @@ -335,6 +335,48 @@ namespace etl return ((Endian == ETL_ENDIAN_LITTLE) ? index : (Size - 1U - index)) * 8U; } + //************************************************************************* + /// Gathers the bytes of a store into a value, and scatters a value back out. + /// Unrolled at source level, so byte swap recognition sees no loop. + //************************************************************************* + template + struct gather_bytes + { + static ETL_CONSTEXPR14 TValue from(const unsigned char* store) + { + return static_cast(gather_bytes::from(store) + | (static_cast(store[Index]) << byte_shift(Index))); + } + }; + + template + struct gather_bytes + { + static ETL_CONSTEXPR14 TValue from(const unsigned char* store) + { + return static_cast(static_cast(store[0]) << byte_shift(0)); + } + }; + + template + struct scatter_bytes + { + static ETL_CONSTEXPR14 void into(TValue value, unsigned char* store) + { + scatter_bytes::into(value, store); + store[Index] = static_cast(value >> byte_shift(Index)); + } + }; + + template + struct scatter_bytes + { + static ETL_CONSTEXPR14 void into(TValue value, unsigned char* store) + { + store[0] = static_cast(value >> byte_shift(0)); + } + }; + #if ETL_USING_BUILTIN_BIT_CAST //************************************************************************* /// Unsigned integer type of a given byte size, used as a constexpr-capable @@ -402,28 +444,23 @@ namespace etl // latter is not defined for 'bool'. typedef typename etl::unsigned_type::type unsigned_t; - unsigned_t uvalue = static_cast(value); + private_unaligned_type::scatter_bytes::into(static_cast(value), store); + } - for (size_t i = 0UL; i < Size_; ++i) - { - store[i] = static_cast(uvalue >> private_unaligned_type::byte_shift(i)); - } + //******************************* + template + static ETL_CONSTEXPR14 T value_from(const_pointer store) + { + typedef typename etl::unsigned_type::type unsigned_t; + + return static_cast(private_unaligned_type::gather_bytes::from(store)); } //******************************* template static ETL_CONSTEXPR14 void copy_store_to_value(const_pointer store, T & value) { - typedef typename etl::unsigned_type::type unsigned_t; - - unsigned_t uvalue = unsigned_t(0); - - for (size_t i = 0UL; i < Size_; ++i) - { - uvalue = static_cast(uvalue | (static_cast(store[i]) << private_unaligned_type::byte_shift(i))); - } - - value = static_cast(uvalue); + value = value_from(store); } //******************************* @@ -462,12 +499,9 @@ namespace etl { typedef typename private_unaligned_type::uint_of_size::type uint_t; - uint_t uvalue = etl::bit_cast(value); + const uint_t uvalue = etl::bit_cast(value); - for (size_t i = 0UL; i < Size_; ++i) - { - store[i] = static_cast(uvalue >> private_unaligned_type::byte_shift(i)); - } + private_unaligned_type::scatter_bytes::into(uvalue, store); } #endif @@ -510,12 +544,7 @@ namespace etl { typedef typename private_unaligned_type::uint_of_size::type uint_t; - uint_t uvalue = uint_t(0); - - for (size_t i = 0UL; i < Size_; ++i) - { - uvalue = static_cast(uvalue | (static_cast(store[i]) << private_unaligned_type::byte_shift(i))); - } + const uint_t uvalue = private_unaligned_type::gather_bytes::from(store); value = etl::bit_cast(uvalue); } @@ -553,6 +582,22 @@ namespace etl do_copy_store_to_value(store, value, use_bit_cast_t()); } + //******************************* + template + static +#if ETL_USING_BUILTIN_BIT_CAST + ETL_CONSTEXPR14 +#endif + T + value_from(const_pointer store) + { + T value = T(); + + copy_store_to_value(store, value); + + return value; + } + //******************************* // This is pure byte manipulation (copy + optional reversal), with no // floating point arithmetic involved, so it is always constexpr-capable @@ -714,11 +759,7 @@ namespace etl //************************************************************************* ETL_CONSTEXPR14 operator T() const { - T value = T(); - - unaligned_copy::copy_store_to_value(this->storage, value); - - return value; + return value_from(this->storage); } //************************************************************************* @@ -726,11 +767,16 @@ namespace etl //************************************************************************* ETL_CONSTEXPR14 T value() const { - T value = T(); + return value_from(this->storage); + } - unaligned_copy::copy_store_to_value(this->storage, value); - - return value; + //************************************************************************* + /// Get the value directly from a byte buffer. + /// Unlike construction from an address, the bytes are not copied to storage. + //************************************************************************* + static ETL_CONSTEXPR14 T value_from(const_pointer address) + { + return unaligned_copy::template value_from(address); } }; ETL_END_PACKED @@ -898,11 +944,7 @@ namespace etl //************************************************************************* operator T() const { - T value = T(); - - unaligned_copy::copy_store_to_value(this->storage, value); - - return value; + return value_from(this->storage); } //************************************************************************* @@ -910,11 +952,16 @@ namespace etl //************************************************************************* T value() const { - T value = T(); + return value_from(this->storage); + } - unaligned_copy::copy_store_to_value(this->storage, value); - - return value; + //************************************************************************* + /// Get the value directly from a byte buffer. + /// Unlike construction from an address, the bytes are not copied to storage. + //************************************************************************* + static T value_from(const_pointer address) + { + return unaligned_copy::template value_from(address); } //************************************************************************* diff --git a/test/test_unaligned_type.cpp b/test/test_unaligned_type.cpp index ad1e50c9..578d0cfb 100644 --- a/test/test_unaligned_type.cpp +++ b/test/test_unaligned_type.cpp @@ -282,6 +282,92 @@ namespace CHECK_THROW(etl::be_uint8_t be_byte_empty(buffer.data(), 0U), etl::unaligned_type_buffer_size); } + //************************************************************************* + // 'value_from' reads a value straight out of a byte buffer, without first + // copying the bytes into an unaligned_type's own storage. + //************************************************************************* + TEST(test_value_from_integral) + { + const std::array buffer = {0x12, 0x34, 0x56, 0x78, 0x9A, 0xBC, 0xDE, 0xF0}; + + CHECK_EQUAL(uint16_t(0x1234), etl::be_uint16_t::value_from(buffer.data())); + CHECK_EQUAL(uint16_t(0x3412), etl::le_uint16_t::value_from(buffer.data())); + + CHECK_EQUAL(uint32_t(0x12345678), etl::be_uint32_t::value_from(buffer.data())); + CHECK_EQUAL(uint32_t(0x78563412), etl::le_uint32_t::value_from(buffer.data())); + + CHECK_EQUAL(uint64_t(0x123456789ABCDEF0), etl::be_uint64_t::value_from(buffer.data())); + CHECK_EQUAL(uint64_t(0xF0DEBC9A78563412), etl::le_uint64_t::value_from(buffer.data())); + } + + //************************************************************************* + /// The address need not be aligned for the value's type. + //************************************************************************* + TEST(test_value_from_unaligned_address) + { + const std::array buffer = {0xFF, 0x12, 0x34, 0x56, 0x78, 0xFF}; + + CHECK_EQUAL(uint32_t(0x12345678), etl::be_uint32_t::value_from(buffer.data() + 1)); + CHECK_EQUAL(uint32_t(0x78563412), etl::le_uint32_t::value_from(buffer.data() + 1)); + } + + //************************************************************************* + /// 'value_from' must agree with constructing from the same address and + /// then reading the value back out. + //************************************************************************* + TEST(test_value_from_matches_construction) + { + const std::array buffer = {0xEE, 0x23, 0x45, 0x67, 0x89, 0xAB, 0xCD, 0xEF}; + + CHECK_EQUAL(etl::be_uint16_t(buffer.data()).value(), etl::be_uint16_t::value_from(buffer.data())); + CHECK_EQUAL(etl::le_uint16_t(buffer.data()).value(), etl::le_uint16_t::value_from(buffer.data())); + CHECK_EQUAL(etl::be_uint32_t(buffer.data()).value(), etl::be_uint32_t::value_from(buffer.data())); + CHECK_EQUAL(etl::le_uint32_t(buffer.data()).value(), etl::le_uint32_t::value_from(buffer.data())); + CHECK_EQUAL(etl::be_uint64_t(buffer.data()).value(), etl::be_uint64_t::value_from(buffer.data())); + CHECK_EQUAL(etl::le_uint64_t(buffer.data()).value(), etl::le_uint64_t::value_from(buffer.data())); + } + + //************************************************************************* + /// Boundary values must survive the round trip through the store. + //************************************************************************* + TEST(test_value_from_boundary_values) + { + const std::array zeroes = {0x00, 0x00, 0x00, 0x00}; + const std::array ones = {0xFF, 0xFF, 0xFF, 0xFF}; + + CHECK_EQUAL(uint32_t(0), etl::be_uint32_t::value_from(zeroes.data())); + CHECK_EQUAL(uint32_t(0), etl::le_uint32_t::value_from(zeroes.data())); + + CHECK_EQUAL(etl::integral_limits::max, etl::be_uint32_t::value_from(ones.data())); + CHECK_EQUAL(etl::integral_limits::max, etl::le_uint32_t::value_from(ones.data())); + } + + //************************************************************************* + /// Signed types are reconstructed through their unsigned representation. + //************************************************************************* + TEST(test_value_from_negative_values) + { + const std::array buffer = {0xFF, 0xFF, 0xFF, 0xFE}; + + CHECK_EQUAL(int32_t(-2), etl::be_int32_t::value_from(buffer.data())); + CHECK_EQUAL(int16_t(-1), etl::be_int16_t::value_from(buffer.data())); + } + + //************************************************************************* + /// The floating point specialisation provides 'value_from' too. + //************************************************************************* + TEST(test_value_from_float) + { + const float f = 3.1415927f; + const double d = 2.718281828459045; + + etl::be_float_t be_f(f); + etl::le_double_t le_d(d); + + CHECK_EQUAL(f, etl::be_float_t::value_from(be_f.data())); + CHECK_EQUAL(d, etl::le_double_t::value_from(le_d.data())); + } + //************************************************************************* // The following tests demonstrate the 'decode' direction: given a raw byte // buffer (e.g. as received from a file, network socket or memory-mapped @@ -625,6 +711,25 @@ namespace CHECK_EQUAL(0x12345678U, be_v.value()); } + //************************************************************************* + /// 'value_from' is usable at compile time for the same reason: it reads + /// the buffer through the 'const unsigned char*' path. + //************************************************************************* + TEST(test_constexpr_value_from_integral) + { + static ETL_CONSTANT unsigned char le_buffer[4] = {0x78, 0x56, 0x34, 0x12}; + static ETL_CONSTANT unsigned char be_buffer[4] = {0x12, 0x34, 0x56, 0x78}; + + constexpr uint32_t le_v = etl::le_uint32_t::value_from(le_buffer); + constexpr uint32_t be_v = etl::be_uint32_t::value_from(be_buffer); + + static_assert(le_v == 0x12345678U, "le_uint32_t constexpr value_from"); + static_assert(be_v == 0x12345678U, "be_uint32_t constexpr value_from"); + + CHECK_EQUAL(0x12345678U, le_v); + CHECK_EQUAL(0x12345678U, be_v); + } + #if ETL_USING_BUILTIN_BIT_CAST //************************************************************************* TEST(test_constexpr_decode_buffer_float)