Skip to content

Fix string.Split regression #130438

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.

Sign up for GitHub

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

Draft
hamarb123 wants to merge 29 commits into
base: main
Choose a base branch
from
+47 −43
Draft

Fix string.Split regression #130438

Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
4301275
Update String.Manipulation.cs
hamarb123 Jul 9, 2026
41dcd92
Optimize handling of last vector's worth on arm64
hamarb123 Jul 9, 2026
180c80d
Revert previous change
hamarb123 Jul 9, 2026
4e8d707
Add same opt to V256/V512 & fix it on xarch
hamarb123 Jul 9, 2026
9c9b01e
Fix (shouldn't affect benchmark run meaningfully)
hamarb123 Jul 9, 2026
4647df7
Update String.Manipulation.cs
hamarb123 Jul 9, 2026
d692d36
Test to see if using V64 on arm conditionally would improve perf further
hamarb123 Jul 9, 2026
39f111a
Fixes to V64 path
hamarb123 Jul 9, 2026
b855b40
Revert V64 logic
hamarb123 Jul 10, 2026
930d7f6
Manually elide bounds check that made its way in when we converted to…
hamarb123 Jul 10, 2026
7fd8a6e
Update String.Manipulation.cs
hamarb123 Jul 10, 2026
358b43d
Add comments & test out alternative fixup sequence for arm64
hamarb123 Jul 10, 2026
56a317d
Update String.Manipulation.cs
hamarb123 Jul 10, 2026
655680a
thanks github editor
hamarb123 Jul 10, 2026
c3f656f
Revert alternative enumeration test
hamarb123 Jul 10, 2026
1c28753
Remove holder from previous experiment
hamarb123 Jul 10, 2026
fe31759
attempt to appease egorbo by removing unsafe by moving scalar code in…
hamarb123 Jul 10, 2026
b0a1d81
thanks github desktop again
hamarb123 Jul 10, 2026
c5f242f
fix build
hamarb123 Jul 10, 2026
0e51b49
experiment to see if regression is the vectorized fixup, or the safe …
hamarb123 Jul 10, 2026
10a7342
fix build
hamarb123 Jul 10, 2026
7f1f799
Remove simd remaining part helper code fully & clean up
hamarb123 Jul 10, 2026
7559f13
Reduce diff by changing where check is performed (needs perf validation)
hamarb123 Jul 10, 2026
c4d10bb
Revert "Reduce diff by changing where check is performed (needs perf …
hamarb123 Jul 11, 2026
d55f70f
Revert "Remove simd remaining part helper code fully & clean up"
hamarb123 Jul 11, 2026
e02efcd
test simplified branching structure
hamarb123 Jul 11, 2026
aa4314c
Update String.Manipulation.cs
hamarb123 Jul 11, 2026
20056be
test alternate simplified branching structure
hamarb123 Jul 11, 2026
254692f
Update String.Manipulation.cs
hamarb123 Jul 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 47 additions & 43 deletions src/libraries/System.Private.CoreLib/src/System/String.Manipulation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2068,20 +2068,7 @@ internal static void MakeSeparatorListAny(ReadOnlySpan<char> source, ReadOnlySpa
sep0 = separators[0];
sep1 = separators.Length > 1 ? separators[1] : sep0;
sep2 = separators.Length > 2 ? separators[2] : sep1;
if (Vector128.IsHardwareAccelerated && source.Length >= Vector128<ushort>.Count * 2)
{
MakeSeparatorListVectorized(source, ref sepListBuilder, sep0, sep1, sep2);
return;
}

for (int i = 0; i < source.Length; i++)
{
char c = source[i];
if (c == sep0 || c == sep1 || c == sep2)
{
sepListBuilder.Append(i);
}
}
MakeSeparatorListFewChars(source, ref sepListBuilder, sep0, sep1, sep2);
}

// Handle > 3 separators with a probabilistic map, ala IndexOfAny.
Expand All @@ -2101,14 +2088,22 @@ internal static void MakeSeparatorListAny(ReadOnlySpan<char> source, ReadOnlySpa
}
}

private static void MakeSeparatorListVectorized(ReadOnlySpan<char> sourceSpan, ref ValueListBuilder<int> sepListBuilder, char c, char c2, char c3)
private static void MakeSeparatorListFewChars(ReadOnlySpan<char> sourceSpan, ref ValueListBuilder<int> sepListBuilder, char c, char c2, char c3)
{
// Redundant test so we won't prejit remainder of this method
// on platforms where it is not supported
if (!Vector128.IsHardwareAccelerated)
if (!Vector128.IsHardwareAccelerated || (uint)sourceSpan.Length < (uint)Vector128<ushort>.Count*2)
{
throw new PlatformNotSupportedException();
for (int i = 0; i < sourceSpan.Length; i++)
{
char v = sourceSpan[i];
if (v == c || v == c2 || v == c3)
{
sepListBuilder.Append(i);
}
}

return;
}

Debug.Assert(sourceSpan.Length >= Vector128<ushort>.Count*2);
int baseIndex = 0;
ReadOnlySpan<ushort> sourceSpanUInt16 = MemoryMarshal.Cast<char, ushort>(sourceSpan);
Expand Down Expand Up @@ -2198,16 +2193,20 @@ private static void MakeSeparatorListVectorized(ReadOnlySpan<char> sourceSpan, r
{
Vector512<ushort> vector = Vector512.Create(sourceSpanUInt16.Slice(sourceSpanUInt16.Length - Vector512<ushort>.Count));
Vector512<byte> cmp = Vector512.Equals(vector, v1).AsByte() | Vector512.Equals(vector, v2).AsByte() | Vector512.Equals(vector, v3).AsByte();
int finalIndex = sourceSpanUInt16.Length - Vector512<ushort>.Count;
ulong mask = cmp.ExtractMostSignificantBits() & 0x5555555555555555 & ~((1UL << (Vector512<byte>.Count - remaining.Length * sizeof(char))) - 1);
while (mask != 0)
if (cmp != Vector512<byte>.Zero)
{
uint bitPos = (uint)BitOperations.TrailingZeroCount(mask) / sizeof(char);
sepListBuilder.Append(finalIndex + (int)bitPos);
mask = BitOperations.ResetLowestSetBit(mask);
int finalIndex = sourceSpanUInt16.Length - Vector512<ushort>.Count;
ulong mask = cmp.ExtractMostSignificantBits() & 0x5555555555555555 & ~((1UL << (Vector512<byte>.Count - remaining.Length * sizeof(char))) - 1);
while (mask != 0)
{
uint bitPos = (uint)BitOperations.TrailingZeroCount(mask) / sizeof(char);
sepListBuilder.Append(finalIndex + (int)bitPos);
mask = BitOperations.ResetLowestSetBit(mask);
}
}

return;
}
return;
}
else if (Vector256.IsHardwareAccelerated && (uint)remaining.Length >= (uint)Vector256<ushort>.Count*2)
{
Expand Down Expand Up @@ -2293,19 +2292,24 @@ private static void MakeSeparatorListVectorized(ReadOnlySpan<char> sourceSpan, r
{
Vector256<ushort> vector = Vector256.Create(sourceSpanUInt16.Slice(sourceSpanUInt16.Length - Vector256<ushort>.Count));
Vector256<byte> cmp = Vector256.Equals(vector, v1).AsByte() | Vector256.Equals(vector, v2).AsByte() | Vector256.Equals(vector, v3).AsByte();
int finalIndex = sourceSpanUInt16.Length - Vector256<ushort>.Count;
uint mask = cmp.ExtractMostSignificantBits() & 0x55555555 & ~((1u << (Vector256<byte>.Count - remaining.Length * sizeof(char))) - 1);
while (mask != 0)
if (cmp != Vector256<byte>.Zero)
{
uint bitPos = (uint)BitOperations.TrailingZeroCount(mask) / sizeof(char);
sepListBuilder.Append(finalIndex + (int)bitPos);
mask = BitOperations.ResetLowestSetBit(mask);
int finalIndex = sourceSpanUInt16.Length - Vector256<ushort>.Count;
uint mask = cmp.ExtractMostSignificantBits() & 0x55555555 & ~((1u << (Vector256<byte>.Count - remaining.Length * sizeof(char))) - 1);
while (mask != 0)
{
uint bitPos = (uint)BitOperations.TrailingZeroCount(mask) / sizeof(char);
sepListBuilder.Append(finalIndex + (int)bitPos);
mask = BitOperations.ResetLowestSetBit(mask);
}
}
}
return;
}
else if (Vector128.IsHardwareAccelerated)
else
{
Debug.Assert(Vector128.IsHardwareAccelerated);
Debug.Assert(remaining.Length >= Vector128<ushort>.Count*2);

Vector128<ushort> v1 = Vector128.Create((ushort)c);
Vector128<ushort> v2 = Vector128.Create((ushort)c2);
Vector128<ushort> v3 = Vector128.Create((ushort)c3);
Expand Down Expand Up @@ -2388,19 +2392,19 @@ private static void MakeSeparatorListVectorized(ReadOnlySpan<char> sourceSpan, r
{
Vector128<ushort> vector = Vector128.Create(sourceSpanUInt16.Slice(sourceSpanUInt16.Length - Vector128<ushort>.Count));
Vector128<byte> cmp = Vector128.Equals(vector, v1).AsByte() | Vector128.Equals(vector, v2).AsByte() | Vector128.Equals(vector, v3).AsByte();
int finalIndex = sourceSpanUInt16.Length - Vector128<ushort>.Count;
uint mask = cmp.ExtractMostSignificantBits() & 0x5555 & ~((1u << (Vector128<byte>.Count - remaining.Length * sizeof(char))) - 1);
while (mask != 0)
if (cmp != Vector128<byte>.Zero)
{
uint bitPos = (uint)BitOperations.TrailingZeroCount(mask) / sizeof(char);
sepListBuilder.Append(finalIndex + (int)bitPos);
mask = BitOperations.ResetLowestSetBit(mask);
int finalIndex = sourceSpanUInt16.Length - Vector128<ushort>.Count;
uint mask = cmp.ExtractMostSignificantBits() & 0x5555 & ~((1u << (Vector128<byte>.Count - remaining.Length * sizeof(char))) - 1);
while (mask != 0)
{
uint bitPos = (uint)BitOperations.TrailingZeroCount(mask) / sizeof(char);
sepListBuilder.Append(finalIndex + (int)bitPos);
mask = BitOperations.ResetLowestSetBit(mask);
}
}
}
return;
}

Debug.Fail("We should not be able to reach this point of MakeSeparatorListVectorized.");
}

/// <summary>
Expand Down
Loading