-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add Avx512 support to IndexOfAnyAsciiSearcher #103710
Conversation
@MihuBot benchmark Regex |
System.Text.RegularExpressions.Tests.Perf_Regex_Industry_RustLang_Sherlock
A few other runs
|
public BitVector256 Lookup = lookup; | ||
|
||
[MethodImpl(MethodImplOptions.AggressiveInlining)] | ||
public readonly Vector128<byte> Bitmap128() => Bitmap512._lower._lower; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: why a method rather than a property?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's a bit more code to define the get
since we need the MethodImpl
on it. No other reason.
@@ -17,18 +17,36 @@ internal static class IndexOfAnyAsciiSearcher | |||
{ | |||
public struct AsciiState(Vector128<byte> bitmap, BitVector256 lookup) | |||
{ | |||
public Vector256<byte> Bitmap = Vector256.Create(bitmap); | |||
public Vector512<byte> Bitmap512 = Vector512.Create(bitmap); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These can't / shouldn't be readonly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TryIndexOfAny
helpers (called by non-searchvalues IndexOfAny
) are writing into this field directly when computing the bitmap to avoid needing an extra local.
As far as the vectorized code is concerned, these could be readonly.
This reverts commit ce1ae77.
…#103710)" (dotnet#104688)" This reverts commit a48a39f.
Closes #93222
Pretty much a copy-paste of the existing Vector128/Vector256 paths (#93222 (comment)).
I had to spam a couple more
AggressiveInlining
s to get all the small helpers to inline even in microbenchmarks where they're never called. This is the source of slight improvements for early matches in a couple benchmarks below.Numbers-wise it's a ~0.5 - 1 ns regression for early matches, and a speedup to ~1.5x in throughput for longer inputs.
Early matches
Throughput
I'll rerun the Regex benchmarks.