Skip to content

Use safe Span.Slice loop pattern in HashCode.AddBytes#127382

Merged
EgorBo merged 2 commits intodotnet:mainfrom
EgorBo:hashcode-safe-loop
Apr 24, 2026
Merged

Use safe Span.Slice loop pattern in HashCode.AddBytes#127382
EgorBo merged 2 commits intodotnet:mainfrom
EgorBo:hashcode-safe-loop

Conversation

@EgorBo
Copy link
Copy Markdown
Member

@EgorBo EgorBo commented Apr 24, 2026

Note

This PR was authored with assistance from GitHub Copilot (AI-generated content).

Rewrites HashCode.AddBytes to use the safe loop pattern:

while (value.Length >= sizeof(int) * 4)
{
    _v1 = Round(_v1, BitConverter.ToUInt32(value));
    _v2 = Round(_v2, BitConverter.ToUInt32(value.Slice(sizeof(int) * 1)));
    _v3 = Round(_v3, BitConverter.ToUInt32(value.Slice(sizeof(int) * 2)));
    _v4 = Round(_v4, BitConverter.ToUInt32(value.Slice(sizeof(int) * 3)));

    _length += 4;
    value = value.Slice(sizeof(int) * 4);
}

This replaces Unsafe.ReadUnaligned / Unsafe.Add(ref, off) / Unsafe.IsAddressLessThan based pointer walking with BitConverter.ToInt32/ToUInt32 and ReadOnlySpan<byte>.Slice over the input span (the queue-flush switch and the 4-byte / 1-byte tail loops are converted in the same way).

The JIT recognizes this exact loop shape (while (data.Length >= CONST) { ...; data = data.Slice(CONST); }) and elides the bounds checks, so the resulting codegen is equivalent to the previous unsafe form.

Codegen diff

https://www.diffchecker.com/JhXDl9Eg/

Rewrites `HashCode.AddBytes` to use the safe `while (value.Length >= CONST) { ...; value = value.Slice(CONST); }` pattern, replacing `Unsafe.ReadUnaligned` / `Unsafe.Add(ref, off)` / `Unsafe.IsAddressLessThan` based pointer walking with `MemoryMarshal.Read<T>` and `ReadOnlySpan<byte>.Slice` over the input span.

The JIT recognizes this exact loop shape and elides the bounds checks, so the resulting codegen is equivalent to the previous unsafe form.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 24, 2026 12:16
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-memory
See info in area-owners.md if you want to be subscribed.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors System.HashCode.AddBytes(ReadOnlySpan<byte>) in System.Private.CoreLib to use a Span slicing loop shape intended to be bounds-check-elidable by the JIT, replacing the prior Unsafe.*-based pointer walking.

Changes:

  • Replaced Unsafe.ReadUnaligned / Unsafe.Add / Unsafe.IsAddressLessThan pointer-walk logic with MemoryMarshal.Read<T> plus ReadOnlySpan<byte>.Slice loops.
  • Updated queue-flush and 4-byte tail processing to advance via slicing rather than ref arithmetic.
  • Converted the 1-byte tail handling to iterate remaining bytes via foreach.

Comment thread src/libraries/System.Private.CoreLib/src/System/HashCode.cs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment thread src/libraries/System.Private.CoreLib/src/System/HashCode.cs
@EgorBo
Copy link
Copy Markdown
Member Author

EgorBo commented Apr 24, 2026

@MihuBot

@EgorBo EgorBo enabled auto-merge (squash) April 24, 2026 17:31
@EgorBo EgorBo merged commit 87e1564 into dotnet:main Apr 24, 2026
145 of 149 checks passed
@EgorBo EgorBo deleted the hashcode-safe-loop branch April 24, 2026 17:32
EgorBo added a commit to EgorBo/runtime-1 that referenced this pull request Apr 25, 2026
The previous loop pattern computed the trailing overlapping vector via
`span.Slice(span.Length - Vector{N}<T>.Count)` AFTER advancing the
span by `Vector{N}<T>.Count`. For inputs where
`Vector{N}<T>.Count <= original.Length < Vector{N}<T>.Count * 2` (e.g.
exactly one vector worth of elements), the post-slice span length is
< Vector{N}<T>.Count and the trailing slice index goes negative,
throwing ArgumentOutOfRangeException.

Capture the trailing overlapping vector slice BEFORE the first slice
mutation, mirroring how the original `ref T lastVectorStart` capture
worked. Also relax the loop condition from `>= * 2` to `>=` to
match the convention used in dotnet#127381 / dotnet#127382 / dotnet#127386 — the one
redundant final compare on exact-multiple lengths is harmless because
min/max is idempotent.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants