Skip to content
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

Remove some bounds checks around Vector.Create(Span) #113035

Closed
wants to merge 3 commits into from

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Mar 1, 2025

let's see if this works..

@EgorBo

This comment was marked as resolved.

Copy link
Contributor

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

@EgorBo
Copy link
Member Author

EgorBo commented Mar 1, 2025

@MihuBot

@EgorBo
Copy link
Member Author

EgorBo commented Mar 2, 2025

This might work, but we need examples where Create(Span) is used to validate this. So far, there is only one use in the whole BCL - in IPAddress (that I recently added there) - everywhere else we use unsafe APIs to load/store vectors

@tannergooding
Copy link
Member

tannergooding commented Mar 3, 2025

This might work, but we need examples where Create(Span) is used to validate this. So far, there is only one use in the whole BCL - in IPAddress (that I recently added there) - everywhere else we use unsafe APIs to load/store vectors

@EgorBo, isn't this a bit of a "chicken and egg" problem? The BCL largely doesn't use these APIs because its unoptimized and so we won't get any hits when attempting to optimize it.

We could pick a couple simple examples in SpanHelpers and update them as part of the PR to validate the improvement (although that might negatively impact Mono) or provide a regression test and validate that gets good codegen improvements with this change. I think simply getting it to the point where we can optimize some cases is a good starting point and will let us make follow up iterative improvements, as well as push users towards using it in their code.

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.

2 participants