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 decode fix #31

Merged
merged 22 commits into from
Sep 5, 2024
Merged

Avx2 decode fix #31

merged 22 commits into from
Sep 5, 2024

Conversation

lemire
Copy link
Member

@lemire lemire commented Sep 4, 2024

This is basically PR #30 with various minor fixes.

@Nick-Nuon you had two bugs:

  • Instead of Vector256<sbyte> chkVector = Avx2.AddSaturate(Avx2.Shuffle(checkValues.AsByte(), checkHash).AsSByte(), src.AsSByte());, you had Vector256<sbyte> chkVector = Avx2.AddSaturate(Avx2.Shuffle(checkValues.AsByte(), checkHash).AsByte(), src.AsByte());.
  • The ToBase64Mask function would return a short, thus truncating the mask (which needs 32 bits).

The AVX results should be 3 times faster at least, but this PR helps a great deal because we no longer fall on the scalar path.

I am not sure I understand what is our performance limitation, but something is hurting us. Still, the results are not bad.

I invite you to run a benchmark.

❤️

@lemire lemire requested a review from Nick-Nuon September 4, 2024 05:37
@lemire
Copy link
Member Author

lemire commented Sep 4, 2024

@Nick-Nuon I simplified our benchmarks (so that it is faster) and I have added a table to our README. It is not bad.

Copy link
Collaborator

@Nick-Nuon Nick-Nuon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the help and pointing these out : it didn't occur to me I might still have bugs.

I get similar results on my rig and otherwise I couldn't find any bugbears so I'm cool with merging. :)

@lemire
Copy link
Member Author

lemire commented Sep 5, 2024

it didn't occur to me I might still have bugs.

To be fair, they were minor bugs. Everything else was fine.

@lemire
Copy link
Member Author

lemire commented Sep 5, 2024

Merging.

@lemire lemire merged commit 1ffc7d7 into main Sep 5, 2024
1 check 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