Fix vectorization of Ascii.Equals for lengths 8..15#123115
Fix vectorization of Ascii.Equals for lengths 8..15#123115tannergooding merged 2 commits intodotnet:mainfrom
Ascii.Equals for lengths 8..15#123115Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the vectorization of Ascii.Equals where the code incorrectly used Vector128<TLeft>.Count as the minimum length threshold for the WideningLoader case. Since WideningLoader converts bytes to ushorts, TLeft=byte has a count of 16 while TRight=ushort has a count of 8. This caused the code to skip vectorization for byte-to-char comparisons with lengths between 8 and 15 elements, falling back to the slower scalar path unnecessarily.
Changes:
- Fixed the minimum length check to use
Vector128<TRight>.Countinstead ofVector128<TLeft>.Count - Replaced all
TLoader.Count128/256/512references withVector<TRight>.Countto ensure both left and right search spaces advance by the same amount - Removed the now-unused
Count128,Count256, andCount512properties from theILoaderinterface and its implementations - Corrected spelling errors: "loose" → "lose"
is the issue here that vectorization path doesn't kick in when it should or it's a correctness fix? |
|
The issue is that vectorization path doesn't kick in, but the result is correct either way. |
|
Tagging subscribers to this area: @dotnet/area-system-runtime |
In case of `WideningLoader` it currently uses `Vector128<TLeft>.Count` (16) as the minimum length, but 8 bytes/chars is the actual minimum. Also removed `Count128/Count256/Count512` as they are not useful - both vectors must always advance by the same amount anyway.
In case of
WideningLoaderit currently usesVector128<TLeft>.Count(16) as the minimum length, but 8 bytes/chars is the actual minimum.Also removed
Count128/Count256/Count512as they are not useful - both vectors must always advance by the same amount anyway.