Use safe span slice-advance loop in Number digit parser#127388
Closed
EgorBo wants to merge 2 commits intodotnet:mainfrom
Closed
Use safe span slice-advance loop in Number digit parser#127388EgorBo wants to merge 2 commits intodotnet:mainfrom
EgorBo wants to merge 2 commits intodotnet:mainfrom
Conversation
Replaces the raw byte* + Unsafe.ReadUnaligned<ulong> SWAR digit-parsing path
with safe ReadOnlySpan<byte> using the JIT-friendly chunked
'while (data.Length >= CONST) { ...; data = data.Slice(CONST); }' pattern.
- ParseEightDigitsUnrolled now takes ReadOnlySpan<byte> and uses
BinaryPrimitives.ReadUInt64LittleEndian, dropping the manual BE swap.
- DigitsToUInt32 / DigitsToUInt64 take ReadOnlySpan<byte> and use the safe
chunked loop with a foreach byte tail.
- [RequiresUnsafe] removed from all three.
- Callers updated to construct ReadOnlySpan<byte> from number.DigitsPtr.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the low-level decimal digit parsing logic used by NumberToFloatingPointBits to avoid raw pointer-walking + Unsafe.ReadUnaligned in favor of ReadOnlySpan<byte> and a JIT-friendly while (span.Length >= CONST) { …; span = span.Slice(CONST); } loop shape, aiming to preserve bounds-check elision while removing [RequiresUnsafe] usage from the helpers.
Changes:
- Convert
ParseEightDigitsUnrolled,DigitsToUInt32, andDigitsToUInt64toReadOnlySpan<byte>-based implementations. - Update
AccumulateDecimalDigitsIntoBigIntegerand the ≤19-digit fast path inNumberToFloatingPointBits<TFloat>to constructReadOnlySpan<byte>overnumber.DigitsPtr. - Use
BinaryPrimitives.ReadUInt64LittleEndiandirectly to handle endianness.
Address codegen regression in AccumulateDecimalDigitsIntoBigInteger reported by jit-diff: the previous variable-count Slice(0, count) couldn't be bounds-check elided because the JIT cannot reason through Math.Min(...). Split the loop into a constant-step (9) chunked path that matches the JIT's slice-advance pattern exactly, plus a single tail call for the 1..8 byte remainder. Both Slice calls now have constants the JIT can reason about. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Member
Author
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.
Note
This PR was authored with assistance from GitHub Copilot.
Replaces the raw
byte*+Unsafe.ReadUnaligned<ulong>SWAR digit-parsing path inNumber.NumberToFloatingPointBits.cswith the safe span-based "slice-advance" loop pattern that the JIT now recognizes and bounds-check elides.This is the same pattern PR #127381 (NumericsHelpers) and #127382 (HashCode) use:
Changes:
ParseEightDigitsUnrolled(byte*)→ParseEightDigitsUnrolled(ReadOnlySpan<byte>). UsesBinaryPrimitives.ReadUInt64LittleEndiandirectly, which removes the manualif (!BitConverter.IsLittleEndian) val = ReverseEndianness(val);branch on big-endian.DigitsToUInt32(byte*, int)/DigitsToUInt64(byte*, int)→DigitsToUInt32(ReadOnlySpan<byte>)/DigitsToUInt64(ReadOnlySpan<byte>)with the JIT-friendlywhile (digits.Length >= 8) { …; digits = digits.Slice(8); }chunked shape and a safeforeachbyte tail.[RequiresUnsafe]removed from all three.AccumulateDecimalDigitsIntoBigIntegerand the ≤19-digit fast path inNumberToFloatingPointBits<TFloat>) to construct aReadOnlySpan<byte>overnumber.DigitsPtr.Codegen diff: TODO
Validation: full
System.Runtime.Testsrun — only the same 8 unrelated pre-existing failures (TimeZoneInfo NearMaxValue/NearMinValue, UnionAttributes); 7089 Single/Double/Half/Decimal parse tests pass.