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

Vectorize the new {Last}IndexOfAnyExcept methods #67942

Closed
stephentoub opened this issue Apr 13, 2022 · 5 comments
Closed

Vectorize the new {Last}IndexOfAnyExcept methods #67942

stephentoub opened this issue Apr 13, 2022 · 5 comments
Assignees
Labels
area-System.Memory help wanted [up-for-grabs] Good issue for external contributors tenet-performance Performance related issue
Milestone

Comments

@stephentoub
Copy link
Member

Simple implementations were added in #67941. These should be vectorized, ideally sharing as much implementation as possible with {Last}IndexOf{Any}.

@stephentoub stephentoub added the tenet-performance Performance related issue label Apr 13, 2022
@stephentoub stephentoub added this to the 7.0.0 milestone Apr 13, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Apr 13, 2022
@EgorBo EgorBo added the help wanted [up-for-grabs] Good issue for external contributors label Apr 13, 2022
@EgorBo
Copy link
Member

EgorBo commented Apr 13, 2022

Can help anyone who decides to try it

@stephentoub
Copy link
Member Author

stephentoub commented Apr 13, 2022

@GrabYourPitchforks suggested we could possibly share the bulk of the IndexOf{Any} implementations, using a generic type parameter ala

private unsafe void ChangeCaseCommon<TConversion>(ref char source, ref char destination, int charCount) where TConversion : struct
{
Debug.Assert(typeof(TConversion) == typeof(ToUpperConversion) || typeof(TConversion) == typeof(ToLowerConversion));
bool toUpper = typeof(TConversion) == typeof(ToUpperConversion); // JIT will treat this as a constant in release builds
to control whether we're including or excluding the specified values.

But I expect we should wait to attempt that until it's been converted over to Vector128/256 (#64451), so maybe we should put something simple in place, maybe just for 1-3 chars, maybe just a single vectorized path (with Vector<T> or Vector128<T>) until the IndexOf{Any} code has stabilized.

@stephentoub stephentoub added area-System.Memory and removed untriaged New issue has not been triaged by the area owner labels Apr 13, 2022
@ghost
Copy link

ghost commented Apr 13, 2022

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

Issue Details

Simple implementations were added in #67941. These should be vectorized, ideally sharing as much implementation as possible with {Last}IndexOf{Any}.

Author: stephentoub
Assignees: -
Labels:

area-System.Memory, tenet-performance, up-for-grabs

Milestone: 7.0.0

@adamsitnik
Copy link
Member

Fixed by #73768

@adamsitnik adamsitnik modified the milestones: 8.0.0, 7.0.0 Aug 18, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Memory help wanted [up-for-grabs] Good issue for external contributors tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

4 participants