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

Avx2 new algo and tests #26

Merged
merged 77 commits into from
May 27, 2024
Merged

Avx2 new algo and tests #26

merged 77 commits into from
May 27, 2024

Conversation

Nick-Nuon
Copy link
Collaborator

@Nick-Nuon Nick-Nuon commented May 13, 2024

This is a new PR that contains the following:

-It adds a test for explicitly testing if a utf8 unit straddles 2 different SIMD vector.
-it adds a test for explicitly testing if there is a "too short" error at the very end of a sequence
-it adds tests for validating the utf6adjustment and scalarcountadjustment function
-It adds runtime dispatches for different architectures
-adds a test for an unterminated simd vector followed by an all ascii simd vector

-I took PR #25 and integrated it with the the optimizations that you introduced back en march. This concerns the AVX2 specifically.
The main difference concerns the "unterminated" path. In case of an unterminated sequence,
it moves back the "processedlengh" pointer up to the last known header byte, makes adjustment to the byte count instead of systematically calling the scalar rewintandValidate function.
-I also tried my best to separate the new code from the main algorithm to make debugging easier.

@Nick-Nuon
Copy link
Collaborator Author

Nick-Nuon commented May 23, 2024

I think this should pass all tests for AVX2! id suggest that it migth be a good time to review

Moving on to #28 in the morning,

@Nick-Nuon Nick-Nuon mentioned this pull request May 23, 2024
@Nick-Nuon
Copy link
Collaborator Author

Nick-Nuon commented May 23, 2024

Ok I screwed up something when trying to merge #28 and then with github,but the commit ad7dad3 works and is clean minus one now superfluous comment

@lemire
Copy link
Member

lemire commented May 23, 2024

I am checking out this PR and might create a PR on top of it if needed. Give me a few minutes.

@lemire
Copy link
Member

lemire commented May 23, 2024

Ok. I can reproduce the issue.

@lemire
Copy link
Member

lemire commented May 23, 2024

I think that the problem is that it tries to build the function with Avx2 in it, and only filters it out at runtime.

@lemire
Copy link
Member

lemire commented May 24, 2024

If you merge #33, it should fix this PR to a point.

@lemire
Copy link
Member

lemire commented May 25, 2024

@Nick-Nuon Feel free to merge this when you are happy!!!

@Nick-Nuon Nick-Nuon merged commit 6f34ead into main May 27, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants