-
Notifications
You must be signed in to change notification settings - Fork 54
Open
Labels
feature-requestNew feature or enhancement. May require GitHub community feedback.New feature or enhancement. May require GitHub community feedback.p3This is a minor priority issueThis is a minor priority issue
Description
Describe the bug
aws_checksums_crc32c_avx512() is doing an extra bit flipping on the previousCRC causing incorrect checksums
Expected Behavior
aws_checksums_crc32c_avx512() computed checksum matches sse4.2 or sw computed versions.
Current Behavior
aws_checksums_crc32c_hw() has bit flips previousCRC value and has this comment:
/* this is the entry point. We should only do the bit flip once. It should not be done for the subfunctions and
* branches.*/
uint32_t crc = ~previousCrc32;
aws_checksums_crc32c_avx512() is a subfunction and it should NOT bit flip again.
Reproduction Steps
Compute crc32c checksum on the same buffer with and without AVX512 code and compare the results.
Possible Solution
diff --git a/source/intel/intrin/crc32c_sse42_avx512.c b/source/intel/intrin/crc32c_sse42_avx512.c
index 837a1ba..a4c7a00 100644
--- a/source/intel/intrin/crc32c_sse42_avx512.c
+++ b/source/intel/intrin/crc32c_sse42_avx512.c
@@ -27,7 +27,7 @@ uint32_t aws_checksums_crc32c_avx512(const uint8_t *input, int length, uint32_t
AWS_ASSERT(
length >= 256 && "invariant violated. length must be greater than 255 bytes to use avx512 to compute crc.");
- uint32_t crc = ~previous_crc;
+ uint32_t crc = previous_crc;
/*
* Definitions of the bit-reflected domain constants k1,k2,k3,k4,k5,k6
* are similar to those given at the end of the paper
Additional Information/Context
No response
aws-checksums version used
AVX512 branch
Compiler and version used
gcc version 11.4.0
Operating System and version
Ubuntu 22.04
Metadata
Metadata
Assignees
Labels
feature-requestNew feature or enhancement. May require GitHub community feedback.New feature or enhancement. May require GitHub community feedback.p3This is a minor priority issueThis is a minor priority issue