-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Intrinsify Span.Slice(int,int) for Span[X..] #122065
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
base: main
Are you sure you want to change the base?
Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
|
@EgorBo I do believe that static ReadOnlySpan<byte> Foo(ReadOnlySpan<byte> data)
{
if (data.Length < sizeof(ushort))
throw new ArgumentException("Data is too short", nameof(data));
var header = MemoryMarshal.Read<ushort>(data);
if (data.Length < sizeof(ushort) + header)
{
throw new ArgumentException("Data is too short for header", nameof(data));
}
return data.Slice(sizeof(ushort), header);
}Ideally there should be no bound checks inserted by the jit. This can be extrapolated further to include multiple data sections/slices |
|
Top regression method is funny :) Reducing those throw calls to 1 could easily result in an improvement |
It is, but this PR was targeting a very specific pattern emitted by
No clue why those tails were not merged |
|
@EgorBo I wonder if results are much better now after your tail merge fix |
|
I wonder if there is a conceptual difference between |
Should be the same thing |
|
Interesting. If you look at |
Closes #121987
Closes #122042
Just an experiment, if dotnet/roslyn#81485 lands, we can delete/ignore this (or intrinsify Slice anyway since it's a popular operation in general)