Skip to content

Commit

Permalink
Fix recent IndexOf regressions (#80779)
Browse files Browse the repository at this point in the history
* Improve IndexOf codegen for non-char types

* Call directly into NonPackedIndexOf where it makes sense
  • Loading branch information
MihaZupan authored Jan 18, 2023
1 parent cdf90c6 commit c956f69
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ public static int IndexOf(ref char searchSpace, int searchSpaceLength, ref char
while (remainingSearchSpaceLength > 0)
{
// Do a quick search for the first element of "value".
int relativeIndex = IndexOfChar(ref Unsafe.Add(ref searchSpace, offset), valueHead, remainingSearchSpaceLength);
// Using the non-packed variant as the input is short and would not benefit from the packed implementation.
int relativeIndex = NonPackedIndexOfChar(ref Unsafe.Add(ref searchSpace, offset), valueHead, remainingSearchSpaceLength);
if (relativeIndex < 0)
break;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,14 @@ internal static partial class PackedSpanHelpers

// Not all values can benefit from packing the searchSpace. See comments in PackSources below.
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static unsafe bool CanUsePackedIndexOf<T>(T value) =>
PackedIndexOfIsSupported &&
RuntimeHelpers.IsBitwiseEquatable<T>() &&
sizeof(T) == sizeof(ushort) &&
*(ushort*)&value - 1u < 254u;
public static unsafe bool CanUsePackedIndexOf<T>(T value)
{
Debug.Assert(PackedIndexOfIsSupported);
Debug.Assert(RuntimeHelpers.IsBitwiseEquatable<T>());
Debug.Assert(sizeof(T) == sizeof(ushort));

return *(ushort*)&value - 1u < 254u;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int IndexOf(ref char searchSpace, char value, int length) =>
Expand Down
14 changes: 9 additions & 5 deletions src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1306,7 +1306,7 @@ public static int SequenceCompareTo<T>(ref T first, int firstLength, ref T secon
[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static unsafe bool ContainsValueType<T>(ref T searchSpace, T value, int length) where T : struct, INumber<T>
{
if (PackedSpanHelpers.PackedIndexOfIsSupported && PackedSpanHelpers.CanUsePackedIndexOf(value))
if (PackedSpanHelpers.PackedIndexOfIsSupported && typeof(T) == typeof(short) && PackedSpanHelpers.CanUsePackedIndexOf(value))
{
return PackedSpanHelpers.Contains(ref Unsafe.As<T, short>(ref searchSpace), *(short*)&value, length);
}
Expand Down Expand Up @@ -1435,6 +1435,10 @@ internal static bool NonPackedContainsValueType<T>(ref T searchSpace, T value, i
internal static int IndexOfChar(ref char searchSpace, char value, int length)
=> IndexOfValueType(ref Unsafe.As<char, short>(ref searchSpace), (short)value, length);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static int NonPackedIndexOfChar(ref char searchSpace, char value, int length) =>
NonPackedIndexOfValueType<short, DontNegate<short>>(ref Unsafe.As<char, short>(ref searchSpace), (short)value, length);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static int IndexOfValueType<T>(ref T searchSpace, T value, int length) where T : struct, INumber<T>
=> IndexOfValueType<T, DontNegate<T>>(ref searchSpace, value, length);
Expand All @@ -1448,7 +1452,7 @@ private static unsafe int IndexOfValueType<TValue, TNegator>(ref TValue searchSp
where TValue : struct, INumber<TValue>
where TNegator : struct, INegator<TValue>
{
if (PackedSpanHelpers.PackedIndexOfIsSupported && PackedSpanHelpers.CanUsePackedIndexOf(value))
if (PackedSpanHelpers.PackedIndexOfIsSupported && typeof(TValue) == typeof(short) && PackedSpanHelpers.CanUsePackedIndexOf(value))
{
return typeof(TNegator) == typeof(DontNegate<short>)
? PackedSpanHelpers.IndexOf(ref Unsafe.As<TValue, char>(ref searchSpace), *(char*)&value, length)
Expand Down Expand Up @@ -1605,7 +1609,7 @@ private static unsafe int IndexOfAnyValueType<TValue, TNegator>(ref TValue searc
where TValue : struct, INumber<TValue>
where TNegator : struct, INegator<TValue>
{
if (PackedSpanHelpers.PackedIndexOfIsSupported && PackedSpanHelpers.CanUsePackedIndexOf(value0) && PackedSpanHelpers.CanUsePackedIndexOf(value1))
if (PackedSpanHelpers.PackedIndexOfIsSupported && typeof(TValue) == typeof(short) && PackedSpanHelpers.CanUsePackedIndexOf(value0) && PackedSpanHelpers.CanUsePackedIndexOf(value1))
{
return typeof(TNegator) == typeof(DontNegate<short>)
? PackedSpanHelpers.IndexOfAny(ref Unsafe.As<TValue, char>(ref searchSpace), *(char*)&value0, *(char*)&value1, length)
Expand Down Expand Up @@ -1782,7 +1786,7 @@ private static unsafe int IndexOfAnyValueType<TValue, TNegator>(ref TValue searc
where TValue : struct, INumber<TValue>
where TNegator : struct, INegator<TValue>
{
if (PackedSpanHelpers.PackedIndexOfIsSupported && PackedSpanHelpers.CanUsePackedIndexOf(value0) && PackedSpanHelpers.CanUsePackedIndexOf(value1) && PackedSpanHelpers.CanUsePackedIndexOf(value2))
if (PackedSpanHelpers.PackedIndexOfIsSupported && typeof(TValue) == typeof(short) && PackedSpanHelpers.CanUsePackedIndexOf(value0) && PackedSpanHelpers.CanUsePackedIndexOf(value1) && PackedSpanHelpers.CanUsePackedIndexOf(value2))
{
return typeof(TNegator) == typeof(DontNegate<short>)
? PackedSpanHelpers.IndexOfAny(ref Unsafe.As<TValue, char>(ref searchSpace), *(char*)&value0, *(char*)&value1, *(char*)&value2, length)
Expand Down Expand Up @@ -3085,7 +3089,7 @@ private static unsafe int IndexOfAnyInRangeUnsignedNumber<T, TNegator>(ref T sea
where T : struct, IUnsignedNumber<T>, IComparisonOperators<T, T, bool>
where TNegator : struct, INegator<T>
{
if (PackedSpanHelpers.PackedIndexOfIsSupported && PackedSpanHelpers.CanUsePackedIndexOf(lowInclusive) && PackedSpanHelpers.CanUsePackedIndexOf(highInclusive) && highInclusive >= lowInclusive)
if (PackedSpanHelpers.PackedIndexOfIsSupported && typeof(T) == typeof(ushort) && PackedSpanHelpers.CanUsePackedIndexOf(lowInclusive) && PackedSpanHelpers.CanUsePackedIndexOf(highInclusive) && highInclusive >= lowInclusive)
{
ref char charSearchSpace = ref Unsafe.As<T, char>(ref searchSpace);
char charLowInclusive = *(char*)&lowInclusive;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1074,15 +1074,32 @@ public string Replace(string oldValue, string? newValue)
// Find all occurrences of the oldValue character.
char c = oldValue[0];
int i = 0;
while (true)

if (PackedSpanHelpers.PackedIndexOfIsSupported && PackedSpanHelpers.CanUsePackedIndexOf(c))
{
int pos = SpanHelpers.IndexOfChar(ref Unsafe.Add(ref _firstChar, i), c, Length - i);
if (pos < 0)
while (true)
{
break;
int pos = PackedSpanHelpers.IndexOf(ref Unsafe.Add(ref _firstChar, i), c, Length - i);
if (pos < 0)
{
break;
}
replacementIndices.Append(i + pos);
i += pos + 1;
}
}
else
{
while (true)
{
int pos = SpanHelpers.NonPackedIndexOfChar(ref Unsafe.Add(ref _firstChar, i), c, Length - i);
if (pos < 0)
{
break;
}
replacementIndices.Append(i + pos);
i += pos + 1;
}
replacementIndices.Append(i + pos);
i += pos + 1;
}
}
else
Expand Down

0 comments on commit c956f69

Please sign in to comment.