CRC big endian support #97
Conversation
| x = ((x >> 8) & m) | (x & m) << 8; | ||
| m = 0xffff0000ffff; | ||
| x = ((x >> 16) & m) | (x & m) << 16; | ||
| return x >> 32 | x << 32; |
There was a problem hiding this comment.
if this algorithm came from somewhere, include a link
There was a problem hiding this comment.
I worked it out on paper just now ... 😮💨 seems right
There was a problem hiding this comment.
if no link, add comments explaining like
// swap every other byte, now every 2-byte chunk is reversed
...
// swap every other 2-byte chunk, now every 4-byte chunk is reversed
...
// swap the two 4-byte chunks, now all 8 bytes are reversed
There was a problem hiding this comment.
This bit twiddling trick is pretty popular in fast crc implementations in the wild. Don't know where it originated, but most libs i looked at included a variant of this.
Poking around more modern hash implementations, like xxHash, that trick seems to have fallen out of favor. Replaced it with compiler built ins and a typical posix byte swap implementation as fallback.
| @@ -21,4 +22,32 @@ | |||
| return val; \ | |||
| } | |||
|
|
|||
| /* helper function to reverse byte order on big-endian platforms*/ | |||
| static inline uint32_t aws_swap_bytes_if_needed_32(uint32_t x) { | |||
There was a problem hiding this comment.
trivial: maybe I'm just not familiar with the literature, but to me "swap_bytes_if_needed" doesn't indicate which way they would swap, or what would be needed to make them swap
| static inline uint32_t aws_swap_bytes_if_needed_32(uint32_t x) { | |
| static inline uint32_t aws_swap_bytes_to_le_if_needed_32(uint32_t x) { |
There was a problem hiding this comment.
renamed to aws_bswap64/32_if_be.
swap bytes or byte swap (bswap is common intrinsic name) is a fairly typical name for reversing order of all bytes.
| const uint64_t *current = (const uint64_t *)(const void *)input; | ||
| while (remaining >= 8) { | ||
| uint64_t c1 = *current++ ^ crc; | ||
| crc = crc64nvme_table_be[0][c1 & 0xff] ^ crc64nvme_table_be[1][(c1 >> 8) & 0xff] ^ |
There was a problem hiding this comment.
I'm not smart enough to check this in under a day ... but it looks like the other one, and the tests pass so probably good 👍
Issue #, if available:
Description of changes:
big endian support for crc algos + ci
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.