Conversation
|
Tagging subscribers to this area: @dotnet/area-system-memory |
There was a problem hiding this comment.
Pull request overview
Removes the TARGET_64BIT-specific bounds-check “fast path” from Span<T> / ReadOnlySpan<T> constructors and Slice(start, length), consolidating on the (uint)-based check used on 32-bit.
Changes:
- Drop
#if TARGET_64BITbounds-check implementations for array ctors inSpan<T>andReadOnlySpan<T>. - Drop
#if TARGET_64BITbounds-check implementations forSlice(int start, int length)and introduce a localcurLengthbefore checking.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Span.cs | Removes 64-bit-specific bounds checks and updates Slice to use a local length before validation. |
| src/libraries/System.Private.CoreLib/src/System/ReadOnlySpan.cs | Removes 64-bit-specific bounds checks and updates Slice to use a local length before validation. |
| this = default; | ||
| return; // returns default | ||
| } | ||
| #if TARGET_64BIT |
There was a problem hiding this comment.
We had copied this pattern and referenced it in more than just ROSpan and Span from what I recall. It possibly exists in Tensor and a few other places as well.
There was a problem hiding this comment.
yeah I was just checking diffs for Span specifically since it's where most bounds check come from, and this pattern makes jit life more complicated. Unfortunately, it's still a regression
There was a problem hiding this comment.
Just to check the diffs again (but I suspect it should be good for arm64)