From 1b4eb1a281c99f2bb772ad0055b3e50e75770673 Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Fri, 6 Sep 2024 18:41:43 -0400 Subject: [PATCH] Use crc32c_read_le.h instead In doing so, I fixed up crc32c_read_le.h a bit: 1. The documentation and tests say the buffers need to be aligned, but they don't. 2. Add a 16-bit version. 3. Remove an unnecessary static_cast. The pointer is already uint8_t. 4. Replace the necessary static_casts with uintN_t{x} per the Google style guide. https://google.github.io/styleguide/cppguide.html#Casting --- src/crc32c_arm64.cc | 39 ++++++++++------------------------ src/crc32c_read_le.h | 34 +++++++++++++++++------------ src/crc32c_read_le_unittest.cc | 17 +++++++++------ 3 files changed, 42 insertions(+), 48 deletions(-) diff --git a/src/crc32c_arm64.cc b/src/crc32c_arm64.cc index 36021c1..701326d 100644 --- a/src/crc32c_arm64.cc +++ b/src/crc32c_arm64.cc @@ -15,6 +15,7 @@ #include #include "./crc32c_internal.h" +#include "./crc32c_read_le.h" #include "crc32c/crc32c_config.h" #if HAVE_ARM64_CRC32C @@ -25,31 +26,13 @@ #define KBYTES 1032 #define SEGMENTBYTES 256 -static uint64_t LoadU64(const uint8_t in[sizeof(uint64_t)]) { - uint64_t ret; - memcpy(&ret, in, sizeof(uint64_t)); - return ret; -} - -static uint32_t LoadU32(const uint8_t in[sizeof(uint32_t)]) { - uint32_t ret; - memcpy(&ret, in, sizeof(uint32_t)); - return ret; -} - -static uint16_t LoadU16(const uint8_t in[sizeof(uint16_t)]) { - uint16_t ret; - memcpy(&ret, in, sizeof(uint16_t)); - return ret; -} - // compute 8bytes for each segment parallelly -#define CRC32C32BYTES(P, IND) \ - do { \ - crc1 = __crc32cd(crc1, LoadU64((P) + SEGMENTBYTES * 1 + (IND)*8)); \ - crc2 = __crc32cd(crc2, LoadU64((P) + SEGMENTBYTES * 2 + (IND)*8)); \ - crc3 = __crc32cd(crc3, LoadU64((P) + SEGMENTBYTES * 3 + (IND)*8)); \ - crc0 = __crc32cd(crc0, LoadU64((P) + SEGMENTBYTES * 0 + (IND)*8)); \ +#define CRC32C32BYTES(P, IND) \ + do { \ + crc1 = __crc32cd(crc1, ReadUint64LE((P) + SEGMENTBYTES * 1 + (IND)*8)); \ + crc2 = __crc32cd(crc2, ReadUint64LE((P) + SEGMENTBYTES * 2 + (IND)*8)); \ + crc3 = __crc32cd(crc3, ReadUint64LE((P) + SEGMENTBYTES * 3 + (IND)*8)); \ + crc0 = __crc32cd(crc0, ReadUint64LE((P) + SEGMENTBYTES * 0 + (IND)*8)); \ } while (0); // compute 8*8 bytes for each segment parallelly @@ -101,7 +84,7 @@ uint32_t ExtendArm64(uint32_t crc, const uint8_t *data, size_t size) { t2 = (uint64_t)vmull_p64(crc2, k2); t1 = (uint64_t)vmull_p64(crc1, k1); t0 = (uint64_t)vmull_p64(crc0, k0); - crc = __crc32cd(crc3, LoadU64(data)); + crc = __crc32cd(crc3, ReadUint64LE(data)); data += sizeof(uint64_t); crc ^= __crc32cd(0, t2); crc ^= __crc32cd(0, t1); @@ -111,18 +94,18 @@ uint32_t ExtendArm64(uint32_t crc, const uint8_t *data, size_t size) { } while (length >= 8) { - crc = __crc32cd(crc, LoadU64(data)); + crc = __crc32cd(crc, ReadUint64LE(data)); data += 8; length -= 8; } if (length & 4) { - crc = __crc32cw(crc, LoadU32(data)); + crc = __crc32cw(crc, ReadUint32LE(data)); data += 4; } if (length & 2) { - crc = __crc32ch(crc, LoadU16(data)); + crc = __crc32ch(crc, ReadUint16LE(data)); data += 2; } diff --git a/src/crc32c_read_le.h b/src/crc32c_read_le.h index 1ebcf5d..bb2231e 100644 --- a/src/crc32c_read_le.h +++ b/src/crc32c_read_le.h @@ -12,13 +12,23 @@ namespace crc32c { -// Reads a little-endian 32-bit integer from a 32-bit-aligned buffer. +// Reads a little-endian 16-bit integer from bytes, not necessarily aligned. +inline uint16_t ReadUint16LE(const uint8_t* buffer) { +#if BYTE_ORDER_BIG_ENDIAN + return ((uint16_t{buffer[0]}) | (uint16_t{buffer[1]} << 8)); +#else // !BYTE_ORDER_BIG_ENDIAN + uint16_t result; + // This should be optimized to a single instruction. + std::memcpy(&result, buffer, sizeof(result)); + return result; +#endif // BYTE_ORDER_BIG_ENDIAN +} + +// Reads a little-endian 32-bit integer from bytes, not necessarily aligned. inline uint32_t ReadUint32LE(const uint8_t* buffer) { #if BYTE_ORDER_BIG_ENDIAN - return ((static_cast(static_cast(buffer[0]))) | - (static_cast(static_cast(buffer[1])) << 8) | - (static_cast(static_cast(buffer[2])) << 16) | - (static_cast(static_cast(buffer[3])) << 24)); + return ((uint32_t{buffer[0]}) | (uint32_t{buffer[1]} << 8) | + (uint32_t{buffer[2]} << 16) | (uint32_t{buffer[3]} << 24)); #else // !BYTE_ORDER_BIG_ENDIAN uint32_t result; // This should be optimized to a single instruction. @@ -27,17 +37,13 @@ inline uint32_t ReadUint32LE(const uint8_t* buffer) { #endif // BYTE_ORDER_BIG_ENDIAN } -// Reads a little-endian 64-bit integer from a 64-bit-aligned buffer. +// Reads a little-endian 64-bit integer from bytes, not necessarily aligned. inline uint64_t ReadUint64LE(const uint8_t* buffer) { #if BYTE_ORDER_BIG_ENDIAN - return ((static_cast(static_cast(buffer[0]))) | - (static_cast(static_cast(buffer[1])) << 8) | - (static_cast(static_cast(buffer[2])) << 16) | - (static_cast(static_cast(buffer[3])) << 24) | - (static_cast(static_cast(buffer[4])) << 32) | - (static_cast(static_cast(buffer[5])) << 40) | - (static_cast(static_cast(buffer[6])) << 48) | - (static_cast(static_cast(buffer[7])) << 56)); + return ((uint64_t{buffer[0]}) | (uint64_t{buffer[1]} << 8) | + (uint64_t{buffer[2]} << 16) | (uint64_t{buffer[3]} << 24) | + (uint64_t{buffer[4]} << 32) | (uint64_t{buffer[5]} << 40) | + (uint64_t{buffer[6]} << 48) | (uint64_t{buffer[7]} << 56)); #else // !BYTE_ORDER_BIG_ENDIAN uint64_t result; // This should be optimized to a single instruction. diff --git a/src/crc32c_read_le_unittest.cc b/src/crc32c_read_le_unittest.cc index 2a30302..8fbfc41 100644 --- a/src/crc32c_read_le_unittest.cc +++ b/src/crc32c_read_le_unittest.cc @@ -13,20 +13,25 @@ namespace crc32c { +TEST(Crc32CReadLETest, ReadUint16LE) { + // little-endian 0x1234 + uint8_t bytes[] = {0x34, 0x12}; + + EXPECT_EQ(uint16_t{0x1234}, ReadUint16LE(bytes)); +} + TEST(Crc32CReadLETest, ReadUint32LE) { // little-endian 0x12345678 - alignas(4) uint8_t bytes[] = {0x78, 0x56, 0x34, 0x12}; + uint8_t bytes[] = {0x78, 0x56, 0x34, 0x12}; - ASSERT_EQ(RoundUp<4>(bytes), bytes) << "Stack array is not aligned"; - EXPECT_EQ(static_cast(0x12345678), ReadUint32LE(bytes)); + EXPECT_EQ(uint32_t{0x12345678}, ReadUint32LE(bytes)); } TEST(Crc32CReadLETest, ReadUint64LE) { // little-endian 0x123456789ABCDEF0 - alignas(8) uint8_t bytes[] = {0xF0, 0xDE, 0xBC, 0x9A, 0x78, 0x56, 0x34, 0x12}; + uint8_t bytes[] = {0xF0, 0xDE, 0xBC, 0x9A, 0x78, 0x56, 0x34, 0x12}; - ASSERT_EQ(RoundUp<8>(bytes), bytes) << "Stack array is not aligned"; - EXPECT_EQ(static_cast(0x123456789ABCDEF0), ReadUint64LE(bytes)); + EXPECT_EQ(uint64_t{0x123456789ABCDEF0}, ReadUint64LE(bytes)); } } // namespace crc32c