Skip to content

Remove some branches in OptimizedInboxTextEncoder.GetIndexOfFirstCharToEncode#130378

Merged
MihaZupan merged 1 commit into
dotnet:mainfrom
MihaZupan:strew-removeSlice
Jul 9, 2026
Merged

Remove some branches in OptimizedInboxTextEncoder.GetIndexOfFirstCharToEncode#130378
MihaZupan merged 1 commit into
dotnet:mainfrom
MihaZupan:strew-removeSlice

Conversation

@MihaZupan

Copy link
Copy Markdown
Member

Follow-up to review feedback on #129625 (comment).

Since idx is always in range (either 0 when the SIMD search is skipped, or
a value already validated against data.Length), the (uint)idx < (uint)data.Length
guard and the separate remaining span are unnecessary. We now slice data in
place inside the proven-in-range branch and run the loops directly over it.

No behavior change. Codegen for the method improves:

Before After
Code size 516 bytes 485 bytes
Instructions 156 146
Callee-saved regs spilled +1 (r14)

The redundant range check is gone and the extra length register is no longer
needed. Hot-loop bounds checks remain eliminated, and the data.Slice(idx)
compiles to plain pointer arithmetic with no ThrowArgumentOutOfRange.

Validated: System.Text.Encodings.Web.Tests (610 tests) pass.

Note

This PR was authored with assistance from GitHub Copilot.

@MihaZupan MihaZupan added this to the 11.0.0 milestone Jul 8, 2026
@MihaZupan MihaZupan requested a review from EgorBo July 8, 2026 21:30
@MihaZupan MihaZupan self-assigned this Jul 8, 2026
Copilot AI review requested due to automatic review settings July 8, 2026 21:30
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@MihaZupan MihaZupan enabled auto-merge (squash) July 8, 2026 21:32

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR simplifies OptimizedInboxTextEncoder.GetIndexOfFirstCharToEncode(ReadOnlySpan<char>) by removing a redundant range-check / secondary “remaining” span and instead slicing data in-place once idx is proven in-range, aiming to improve generated code quality without changing behavior.

Changes:

  • Replace the (uint)idx < (uint)data.Length guard + remaining span with in-place data = data.Slice(idx) and direct iteration over data.
  • Keep the 8x-unrolled scan and fall back to a foreach for the tail.
  • Adjust the return flow to return -1 on full consumption and return idx on first disallowed char.

@MihaZupan MihaZupan merged commit 53eb0d5 into dotnet:main Jul 9, 2026
90 checks passed
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