-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Empty slice fix #107316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Empty slice fix #107316
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -655,34 +655,70 @@ public ReadOnlyTensorSpan<T> Slice(params scoped ReadOnlySpan<NRange> ranges) | |
if (ranges.Length != Lengths.Length) | ||
throw new ArgumentOutOfRangeException(nameof(ranges), "Number of dimensions to slice does not equal the number of dimensions in the span"); | ||
|
||
ReadOnlyTensorSpan<T> toReturn; | ||
scoped Span<nint> lengths; | ||
scoped Span<nint> offsets; | ||
nint[]? lengthsArray; | ||
nint[]? offsetsArray; | ||
if (Rank > TensorShape.MaxInlineRank) | ||
{ | ||
lengths = stackalloc nint[Rank]; | ||
offsets = stackalloc nint[Rank]; | ||
lengthsArray = ArrayPool<nint>.Shared.Rent(Rank); | ||
lengths = lengthsArray; | ||
lengths = lengths.Slice(0, Rank); | ||
lengthsArray = null; | ||
michaelgsharp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
offsetsArray = ArrayPool<nint>.Shared.Rent(Rank); | ||
offsets = offsetsArray; | ||
offsets = offsets.Slice(0, Rank); | ||
offsetsArray = null; | ||
michaelgsharp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
else | ||
{ | ||
lengths = new nint[Rank]; | ||
offsets = new nint[Rank]; | ||
lengths = stackalloc nint[Rank]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not something to fix in this PR, but dynamically lengthed stackalloc is "bad" Rather we should be using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sounds good. Why is dynamically lengthed stackalloc bad? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because it results in a lot of additional codegen to handle the fact the length is "unknown". Since we know the maximum is small, its better to do that and slice to avoid the pessimization. Otherwise, it typically ends up cheaper to just use the array pool; rather than stackalloc at all. |
||
offsets = stackalloc nint[Rank]; | ||
|
||
lengthsArray = null; | ||
offsetsArray = null; | ||
} | ||
|
||
for (int i = 0; i < ranges.Length; i++) | ||
{ | ||
(offsets[i], lengths[i]) = ranges[i].GetOffsetAndLength(Lengths[i]); | ||
} | ||
|
||
nint index = 0; | ||
for (int i = 0; i < offsets.Length; i++) | ||
// When we have an empty Tensor and someone wants to slice all of it, we should return an empty Tensor. | ||
if (TensorSpanHelpers.CalculateTotalLength(Lengths) == 0) | ||
{ | ||
index += Strides[i] * (offsets[i]); | ||
for (int i = 0; i < offsets.Length; i++) | ||
{ | ||
if (offsets[i] != 0 || lengths[i] != 0) | ||
michaelgsharp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ThrowHelper.ThrowIndexOutOfRangeException(); | ||
} | ||
toReturn = new ReadOnlyTensorSpan<T>(ref _reference, lengths, _shape.Strides, _shape._memoryLength); | ||
michaelgsharp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
else | ||
michaelgsharp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
nint index = 0; | ||
for (int i = 0; i < offsets.Length; i++) | ||
{ | ||
if (offsets[i] < 0 || offsets[i] >= Lengths[i]) | ||
michaelgsharp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
ThrowHelper.ThrowIndexOutOfRangeException(); | ||
|
||
index += Strides[i] * (offsets[i]); | ||
} | ||
|
||
if (index >= _shape._memoryLength || index < 0) | ||
ThrowHelper.ThrowIndexOutOfRangeException(); | ||
|
||
toReturn = new ReadOnlyTensorSpan<T>(ref Unsafe.Add(ref _reference, index), lengths, _shape.Strides, _shape._memoryLength - index); | ||
} | ||
|
||
if (index >= _shape._memoryLength || index < 0) | ||
ThrowHelper.ThrowIndexOutOfRangeException(); | ||
if (offsetsArray != null) | ||
ArrayPool<nint>.Shared.Return(offsetsArray); | ||
if (lengthsArray != null) | ||
ArrayPool<nint>.Shared.Return(lengthsArray); | ||
|
||
return new ReadOnlyTensorSpan<T>(ref Unsafe.Add(ref _reference, index), lengths, _shape.Strides, _shape._memoryLength - index); | ||
return toReturn; | ||
} | ||
|
||
/// <summary> | ||
|
Uh oh!
There was an error while loading. Please reload this page.