Vectorize Forward/Reflected CRC-32 and Forward CRC-64#124832
Open
bartonjs wants to merge 8 commits intodotnet:mainfrom
Open
Vectorize Forward/Reflected CRC-32 and Forward CRC-64#124832bartonjs wants to merge 8 commits intodotnet:mainfrom
bartonjs wants to merge 8 commits intodotnet:mainfrom
Conversation
Contributor
|
Tagging subscribers to this area: @dotnet/area-system-io-hashing, @bartonjs, @vcsjones |
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request implements vectorized CRC computation for CRC-32 (both forward and reflected variants) and CRC-64 (forward only). The changes refactor the existing vectorization code from the Crc32 and Crc64 classes into the respective ParameterSet classes, enabling vectorization support for custom CRC polynomials.
Changes:
- Introduces abstract base classes (
ForwardCrc32,ReflectedCrc32,ForwardCrc64) that handle vectorization using a partial method pattern - Adds
CrcPolynomialHelperclass with aUInt640struct for computing folding and Barrett constants for arbitrary polynomials - Migrates vectorization logic from
Crc32/Crc64static classes to parameter set implementations
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| CrcPolynomialHelper.cs | New helper class for computing CRC folding/Barrett constants using 640-bit arithmetic |
| Crc32ParameterSet.cs | Adds ReflectedCrc32 and ForwardCrc32 abstract base classes with partial method hooks for vectorization |
| Crc32ParameterSet.Vectorized.cs | Implements vectorized CRC-32 computation for both reflected and forward variants |
| Crc32ParameterSet.WellKnown.cs | Updates IEEE 802.3 and CRC-32C to use new base classes with ARM intrinsics support |
| Crc32ParameterSet.Table.cs | Updates table-based implementations to inherit from new base classes |
| Crc32.cs | Removes partial modifier, delegates to parameter set's Update method |
| Crc32.Vectorized.cs | Deleted - logic moved to ParameterSet |
| Crc32.Table.cs | Deleted - table moved to ParameterSet.WellKnown |
| Crc32.Arm.cs | Deleted - ARM intrinsics moved to ParameterSet.WellKnown |
| Crc64ParameterSet.cs | Adds ForwardCrc64 abstract base class with vectorization support |
| Crc64ParameterSet.Vectorized.cs | Implements vectorized forward CRC-64 computation |
| Crc64ParameterSet.WellKnown.cs | Updates ECMA-182 to use ForwardCrc64 base class |
| Crc64ParameterSet.Table.cs | Updates table-based forward implementation to inherit from ForwardCrc64 |
| Crc64.cs | Removes partial modifier, delegates to parameter set's Update method |
| Crc64.Vectorized.cs | Deleted - logic moved to ParameterSet |
| Crc64.Table.cs | Deleted - table moved to ParameterSet.WellKnown |
| System.IO.Hashing.csproj | Updates file references to reflect deletions and additions |
| Crc32Tests_ParameterSet_Custom.cs | Adds test driver for forward CRC-32 with non-standard polynomial |
Member
Author
|
Perf-wise, it's basically unchanged from the current vectorized implementation for CRC-32/IEEE and CRC-64/ECMA.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This provides a vector implementation of CRC-32, and forward-only for CRC-64.
Reflected CRC-64 theoretically requires UInt128 to hold the 65-bit constants. That's not "it can't be done", but it's complicating the algorithm, so it's being pushed to a followup.
Intel's
isa-lrepository does have a reflected CRC-64 that only uses the xmm registers, and seems to be using only ulong for the constants (e.g.rk20: DQ 0x1b1ab00000000001), so it does seem to be possible, but the tooling isn't coming to that conclusion automatically.Contributes to #124576.