From a59cff5f07f291891884a1bd76d5cca63b612fbb Mon Sep 17 00:00:00 2001 From: Miha Zupan Date: Thu, 29 Dec 2022 19:34:41 +0100 Subject: [PATCH] Stop using PackedIndexOf on ARM --- .../src/System/Globalization/Ordinal.cs | 2 +- .../IndexOfAnyValues/IndexOfAny1CharValue.cs | 4 +- .../IndexOfAnyValues/IndexOfAny2CharValues.cs | 4 +- .../IndexOfAnyValues/IndexOfAny3CharValues.cs | 4 +- .../IndexOfAnyCharValuesInRange.cs | 4 +- .../src/System/SpanHelpers.Packed.cs | 60 +++++++++---------- .../src/System/SpanHelpers.T.cs | 18 +++--- .../src/System/String.Searching.cs | 2 +- 8 files changed, 46 insertions(+), 52 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Globalization/Ordinal.cs b/src/libraries/System.Private.CoreLib/src/System/Globalization/Ordinal.cs index f5157b0cbba8..1dd36f673066 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Globalization/Ordinal.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Globalization/Ordinal.cs @@ -340,7 +340,7 @@ internal static int IndexOfOrdinalIgnoreCase(ReadOnlySpan source, ReadOnly // Do a quick search for the first element of "value". int relativeIndex = isLetter ? PackedSpanHelpers.PackedIndexOfIsSupported - ? PackedSpanHelpers.PackedIndexOfAny(ref Unsafe.Add(ref searchSpace, offset), valueCharU, valueCharL, searchSpaceLength) + ? PackedSpanHelpers.IndexOfAny(ref Unsafe.Add(ref searchSpace, offset), valueCharU, valueCharL, searchSpaceLength) : SpanHelpers.IndexOfAnyChar(ref Unsafe.Add(ref searchSpace, offset), valueCharU, valueCharL, searchSpaceLength) : SpanHelpers.IndexOfChar(ref Unsafe.Add(ref searchSpace, offset), valueChar, searchSpaceLength); if (relativeIndex < 0) diff --git a/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny1CharValue.cs b/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny1CharValue.cs index 8461c8b2b50c..762f1872cb64 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny1CharValue.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny1CharValue.cs @@ -23,7 +23,7 @@ internal sealed class IndexOfAny1CharValue : IndexOfAnyValues< [MethodImpl(MethodImplOptions.AggressiveInlining)] internal override int IndexOfAny(ReadOnlySpan span) => TShouldUsePacked.Value - ? PackedSpanHelpers.PackedIndexOf(ref MemoryMarshal.GetReference(span), _e0, span.Length) + ? PackedSpanHelpers.IndexOf(ref MemoryMarshal.GetReference(span), _e0, span.Length) : SpanHelpers.NonPackedIndexOfValueType>( ref Unsafe.As(ref MemoryMarshal.GetReference(span)), Unsafe.As(ref _e0), @@ -32,7 +32,7 @@ internal sealed class IndexOfAny1CharValue : IndexOfAnyValues< [MethodImpl(MethodImplOptions.AggressiveInlining)] internal override int IndexOfAnyExcept(ReadOnlySpan span) => TShouldUsePacked.Value - ? PackedSpanHelpers.PackedIndexOfAnyExcept(ref MemoryMarshal.GetReference(span), _e0, span.Length) + ? PackedSpanHelpers.IndexOfAnyExcept(ref MemoryMarshal.GetReference(span), _e0, span.Length) : SpanHelpers.NonPackedIndexOfValueType>( ref Unsafe.As(ref MemoryMarshal.GetReference(span)), Unsafe.As(ref _e0), diff --git a/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny2CharValues.cs b/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny2CharValues.cs index 8560999b3d4b..f649ffc75265 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny2CharValues.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny2CharValues.cs @@ -23,7 +23,7 @@ internal sealed class IndexOfAny2CharValue : IndexOfAnyValues< [MethodImpl(MethodImplOptions.AggressiveInlining)] internal override int IndexOfAny(ReadOnlySpan span) => TShouldUsePacked.Value - ? PackedSpanHelpers.PackedIndexOfAny(ref MemoryMarshal.GetReference(span), _e0, _e1, span.Length) + ? PackedSpanHelpers.IndexOfAny(ref MemoryMarshal.GetReference(span), _e0, _e1, span.Length) : SpanHelpers.NonPackedIndexOfAnyValueType>( ref Unsafe.As(ref MemoryMarshal.GetReference(span)), Unsafe.As(ref _e0), @@ -33,7 +33,7 @@ internal sealed class IndexOfAny2CharValue : IndexOfAnyValues< [MethodImpl(MethodImplOptions.AggressiveInlining)] internal override int IndexOfAnyExcept(ReadOnlySpan span) => TShouldUsePacked.Value - ? PackedSpanHelpers.PackedIndexOfAnyExcept(ref MemoryMarshal.GetReference(span), _e0, _e1, span.Length) + ? PackedSpanHelpers.IndexOfAnyExcept(ref MemoryMarshal.GetReference(span), _e0, _e1, span.Length) : SpanHelpers.NonPackedIndexOfAnyValueType>( ref Unsafe.As(ref MemoryMarshal.GetReference(span)), Unsafe.As(ref _e0), diff --git a/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny3CharValues.cs b/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny3CharValues.cs index 519fe27245de..5ae583b0f174 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny3CharValues.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAny3CharValues.cs @@ -23,7 +23,7 @@ internal sealed class IndexOfAny3CharValue : IndexOfAnyValues< [MethodImpl(MethodImplOptions.AggressiveInlining)] internal override int IndexOfAny(ReadOnlySpan span) => TShouldUsePacked.Value - ? PackedSpanHelpers.PackedIndexOfAny(ref MemoryMarshal.GetReference(span), _e0, _e1, _e2, span.Length) + ? PackedSpanHelpers.IndexOfAny(ref MemoryMarshal.GetReference(span), _e0, _e1, _e2, span.Length) : SpanHelpers.NonPackedIndexOfAnyValueType>( ref Unsafe.As(ref MemoryMarshal.GetReference(span)), Unsafe.As(ref _e0), @@ -34,7 +34,7 @@ internal sealed class IndexOfAny3CharValue : IndexOfAnyValues< [MethodImpl(MethodImplOptions.AggressiveInlining)] internal override int IndexOfAnyExcept(ReadOnlySpan span) => TShouldUsePacked.Value - ? PackedSpanHelpers.PackedIndexOfAnyExcept(ref MemoryMarshal.GetReference(span), _e0, _e1, _e2, span.Length) + ? PackedSpanHelpers.IndexOfAnyExcept(ref MemoryMarshal.GetReference(span), _e0, _e1, _e2, span.Length) : SpanHelpers.NonPackedIndexOfAnyValueType>( ref Unsafe.As(ref MemoryMarshal.GetReference(span)), Unsafe.As(ref _e0), diff --git a/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAnyCharValuesInRange.cs b/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAnyCharValuesInRange.cs index 961de773e257..018ef273b483 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAnyCharValuesInRange.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IndexOfAnyValues/IndexOfAnyCharValuesInRange.cs @@ -39,7 +39,7 @@ internal override char[] GetValues() [MethodImpl(MethodImplOptions.AggressiveInlining)] internal override int IndexOfAny(ReadOnlySpan span) => TShouldUsePacked.Value - ? PackedSpanHelpers.PackedIndexOfAnyInRange(ref MemoryMarshal.GetReference(span), _lowInclusive, _rangeInclusive, span.Length) + ? PackedSpanHelpers.IndexOfAnyInRange(ref MemoryMarshal.GetReference(span), _lowInclusive, _rangeInclusive, span.Length) : SpanHelpers.NonPackedIndexOfAnyInRangeUnsignedNumber>( ref Unsafe.As(ref MemoryMarshal.GetReference(span)), Unsafe.As(ref _lowInclusive), @@ -49,7 +49,7 @@ internal override char[] GetValues() [MethodImpl(MethodImplOptions.AggressiveInlining)] internal override int IndexOfAnyExcept(ReadOnlySpan span) => TShouldUsePacked.Value - ? PackedSpanHelpers.PackedIndexOfAnyExceptInRange(ref MemoryMarshal.GetReference(span), _lowInclusive, _rangeInclusive, span.Length) + ? PackedSpanHelpers.IndexOfAnyExceptInRange(ref MemoryMarshal.GetReference(span), _lowInclusive, _rangeInclusive, span.Length) : SpanHelpers.NonPackedIndexOfAnyInRangeUnsignedNumber>( ref Unsafe.As(ref MemoryMarshal.GetReference(span)), Unsafe.As(ref _lowInclusive), diff --git a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Packed.cs b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Packed.cs index 8fce1de5e321..91d516a7f5d5 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Packed.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.Packed.cs @@ -6,7 +6,6 @@ using System.Numerics; using System.Runtime.CompilerServices; using System.Runtime.Intrinsics; -using System.Runtime.Intrinsics.Arm; using System.Runtime.Intrinsics.X86; #pragma warning disable IDE0060 // https://github.com/dotnet/roslyn-analyzers/issues/6228 @@ -19,53 +18,51 @@ namespace System // included in this file which are specific to the packed implementation. internal static partial class PackedSpanHelpers { - public static bool PackedIndexOfIsSupported => Sse2.IsSupported || AdvSimd.IsSupported; + // We only do this optimization on X86 as the packing is noticeably more expensive on ARM in comparison. + // While the impact on worst-case (match at the start) is minimal on X86, it's prohibitively large on ARM. + public static bool PackedIndexOfIsSupported => Sse2.IsSupported; // Not all values can benefit from packing the searchSpace. See comments in PackSources below. - // On X86, the values must be in the [1, 254] range. - // On ARM, the values must be in the [0, 254] range. [MethodImpl(MethodImplOptions.AggressiveInlining)] public static unsafe bool CanUsePackedIndexOf(T value) => PackedIndexOfIsSupported && RuntimeHelpers.IsBitwiseEquatable() && sizeof(T) == sizeof(ushort) && - (Sse2.IsSupported - ? *(ushort*)&value - 1u < 254u - : *(ushort*)&value < 255u); + *(ushort*)&value - 1u < 254u; [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int PackedIndexOf(ref char searchSpace, char value, int length) => - PackedIndexOf>(ref Unsafe.As(ref searchSpace), (short)value, length); + public static int IndexOf(ref char searchSpace, char value, int length) => + IndexOf>(ref Unsafe.As(ref searchSpace), (short)value, length); [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int PackedIndexOfAnyExcept(ref char searchSpace, char value, int length) => - PackedIndexOf>(ref Unsafe.As(ref searchSpace), (short)value, length); + public static int IndexOfAnyExcept(ref char searchSpace, char value, int length) => + IndexOf>(ref Unsafe.As(ref searchSpace), (short)value, length); [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int PackedIndexOfAny(ref char searchSpace, char value0, char value1, int length) => - PackedIndexOfAny>(ref Unsafe.As(ref searchSpace), (short)value0, (short)value1, length); + public static int IndexOfAny(ref char searchSpace, char value0, char value1, int length) => + IndexOfAny>(ref Unsafe.As(ref searchSpace), (short)value0, (short)value1, length); [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int PackedIndexOfAnyExcept(ref char searchSpace, char value0, char value1, int length) => - PackedIndexOfAny>(ref Unsafe.As(ref searchSpace), (short)value0, (short)value1, length); + public static int IndexOfAnyExcept(ref char searchSpace, char value0, char value1, int length) => + IndexOfAny>(ref Unsafe.As(ref searchSpace), (short)value0, (short)value1, length); [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int PackedIndexOfAny(ref char searchSpace, char value0, char value1, char value2, int length) => - PackedIndexOfAny>(ref Unsafe.As(ref searchSpace), (short)value0, (short)value1, (short)value2, length); + public static int IndexOfAny(ref char searchSpace, char value0, char value1, char value2, int length) => + IndexOfAny>(ref Unsafe.As(ref searchSpace), (short)value0, (short)value1, (short)value2, length); [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int PackedIndexOfAnyExcept(ref char searchSpace, char value0, char value1, char value2, int length) => - PackedIndexOfAny>(ref Unsafe.As(ref searchSpace), (short)value0, (short)value1, (short)value2, length); + public static int IndexOfAnyExcept(ref char searchSpace, char value0, char value1, char value2, int length) => + IndexOfAny>(ref Unsafe.As(ref searchSpace), (short)value0, (short)value1, (short)value2, length); [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int PackedIndexOfAnyInRange(ref char searchSpace, char lowInclusive, char rangeInclusive, int length) => - PackedIndexOfAnyInRange>(ref Unsafe.As(ref searchSpace), (short)lowInclusive, (short)rangeInclusive, length); + public static int IndexOfAnyInRange(ref char searchSpace, char lowInclusive, char rangeInclusive, int length) => + IndexOfAnyInRange>(ref Unsafe.As(ref searchSpace), (short)lowInclusive, (short)rangeInclusive, length); [MethodImpl(MethodImplOptions.AggressiveInlining)] - public static int PackedIndexOfAnyExceptInRange(ref char searchSpace, char lowInclusive, char rangeInclusive, int length) => - PackedIndexOfAnyInRange>(ref Unsafe.As(ref searchSpace), (short)lowInclusive, (short)rangeInclusive, length); + public static int IndexOfAnyExceptInRange(ref char searchSpace, char lowInclusive, char rangeInclusive, int length) => + IndexOfAnyInRange>(ref Unsafe.As(ref searchSpace), (short)lowInclusive, (short)rangeInclusive, length); - public static bool PackedContains(ref short searchSpace, short value, int length) + public static bool Contains(ref short searchSpace, short value, int length) { Debug.Assert(CanUsePackedIndexOf(value)); @@ -207,7 +204,7 @@ public static bool PackedContains(ref short searchSpace, short value, int length return false; } - private static int PackedIndexOf(ref short searchSpace, short value, int length) + private static int IndexOf(ref short searchSpace, short value, int length) where TNegator : struct, SpanHelpers.INegator { Debug.Assert(CanUsePackedIndexOf(value)); @@ -348,7 +345,7 @@ private static int PackedIndexOf(ref short searchSpace, short value, i return -1; } - private static int PackedIndexOfAny(ref short searchSpace, short value0, short value1, int length) + private static int IndexOfAny(ref short searchSpace, short value0, short value1, int length) where TNegator : struct, SpanHelpers.INegator { Debug.Assert(CanUsePackedIndexOf(value0)); @@ -498,7 +495,7 @@ private static int PackedIndexOfAny(ref short searchSpace, short value return -1; } - private static int PackedIndexOfAny(ref short searchSpace, short value0, short value1, short value2, int length) + private static int IndexOfAny(ref short searchSpace, short value0, short value1, short value2, int length) where TNegator : struct, SpanHelpers.INegator { Debug.Assert(CanUsePackedIndexOf(value0)); @@ -651,7 +648,7 @@ private static int PackedIndexOfAny(ref short searchSpace, short value return -1; } - private static int PackedIndexOfAnyInRange(ref short searchSpace, short lowInclusive, short rangeInclusive, int length) + private static int IndexOfAnyInRange(ref short searchSpace, short lowInclusive, short rangeInclusive, int length) where TNegator : struct, SpanHelpers.INegator { Debug.Assert(CanUsePackedIndexOf(lowInclusive)); @@ -798,15 +795,12 @@ private static Vector256 PackSources(Vector256 source0, Vector256 PackSources(Vector128 source0, Vector128 source1) { + Debug.Assert(Sse2.IsSupported); // Pack two vectors of characters into bytes. While the type is Vector128, these are really UInt16 characters. // X86: Downcast every character using saturation. // - Values <= 32767 result in min(value, 255). // - Values > 32767 result in 0. Because of this we can't accept needles that contain 0. - // ARM64: Do narrowing saturation over unsigned values. - // - All values result in min(value, 255) - return Sse2.IsSupported - ? Sse2.PackUnsignedSaturate(source0, source1).AsByte() - : AdvSimd.ExtractNarrowingSaturateUpper(AdvSimd.ExtractNarrowingSaturateLower(source0.AsUInt16()), source1.AsUInt16()); + return Sse2.PackUnsignedSaturate(source0, source1).AsByte(); } [MethodImpl(MethodImplOptions.AggressiveInlining)] diff --git a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs index c223af0e751a..d3d643f13ee6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs +++ b/src/libraries/System.Private.CoreLib/src/System/SpanHelpers.T.cs @@ -1308,7 +1308,7 @@ public static int SequenceCompareTo(ref T first, int firstLength, ref T secon { if (PackedSpanHelpers.CanUsePackedIndexOf(value)) { - return PackedSpanHelpers.PackedContains(ref Unsafe.As(ref searchSpace), *(short*)&value, length); + return PackedSpanHelpers.Contains(ref Unsafe.As(ref searchSpace), *(short*)&value, length); } return NonPackedContainsValueType(ref searchSpace, value, length); @@ -1451,8 +1451,8 @@ internal static int IndexOfChar(ref char searchSpace, char value, int length) if (PackedSpanHelpers.CanUsePackedIndexOf(value)) { return typeof(TNegator) == typeof(DontNegate) - ? PackedSpanHelpers.PackedIndexOf(ref Unsafe.As(ref searchSpace), *(char*)&value, length) - : PackedSpanHelpers.PackedIndexOfAnyExcept(ref Unsafe.As(ref searchSpace), *(char*)&value, length); + ? PackedSpanHelpers.IndexOf(ref Unsafe.As(ref searchSpace), *(char*)&value, length) + : PackedSpanHelpers.IndexOfAnyExcept(ref Unsafe.As(ref searchSpace), *(char*)&value, length); } return NonPackedIndexOfValueType(ref searchSpace, value, length); @@ -1608,8 +1608,8 @@ internal static int IndexOfAnyChar(ref char searchSpace, char value0, char value if (PackedSpanHelpers.CanUsePackedIndexOf(value0) && PackedSpanHelpers.CanUsePackedIndexOf(value1)) { return typeof(TNegator) == typeof(DontNegate) - ? PackedSpanHelpers.PackedIndexOfAny(ref Unsafe.As(ref searchSpace), *(char*)&value0, *(char*)&value1, length) - : PackedSpanHelpers.PackedIndexOfAnyExcept(ref Unsafe.As(ref searchSpace), *(char*)&value0, *(char*)&value1, length); + ? PackedSpanHelpers.IndexOfAny(ref Unsafe.As(ref searchSpace), *(char*)&value0, *(char*)&value1, length) + : PackedSpanHelpers.IndexOfAnyExcept(ref Unsafe.As(ref searchSpace), *(char*)&value0, *(char*)&value1, length); } return NonPackedIndexOfAnyValueType(ref searchSpace, value0, value1, length); @@ -1785,8 +1785,8 @@ internal static int IndexOfAnyChar(ref char searchSpace, char value0, char value if (PackedSpanHelpers.CanUsePackedIndexOf(value0) && PackedSpanHelpers.CanUsePackedIndexOf(value1) && PackedSpanHelpers.CanUsePackedIndexOf(value2)) { return typeof(TNegator) == typeof(DontNegate) - ? PackedSpanHelpers.PackedIndexOfAny(ref Unsafe.As(ref searchSpace), *(char*)&value0, *(char*)&value1, *(char*)&value2, length) - : PackedSpanHelpers.PackedIndexOfAnyExcept(ref Unsafe.As(ref searchSpace), *(char*)&value0, *(char*)&value1, *(char*)&value2, length); + ? PackedSpanHelpers.IndexOfAny(ref Unsafe.As(ref searchSpace), *(char*)&value0, *(char*)&value1, *(char*)&value2, length) + : PackedSpanHelpers.IndexOfAnyExcept(ref Unsafe.As(ref searchSpace), *(char*)&value0, *(char*)&value1, *(char*)&value2, length); } return NonPackedIndexOfAnyValueType(ref searchSpace, value0, value1, value2, length); @@ -3092,8 +3092,8 @@ internal static int IndexOfAnyExceptInRangeUnsignedNumber(ref T searchSpace, char charRange = (char)(*(char*)&highInclusive - charLowInclusive); return typeof(TNegator) == typeof(DontNegate) - ? PackedSpanHelpers.PackedIndexOfAnyInRange(ref charSearchSpace, charLowInclusive, charRange, length) - : PackedSpanHelpers.PackedIndexOfAnyExceptInRange(ref charSearchSpace, charLowInclusive, charRange, length); + ? PackedSpanHelpers.IndexOfAnyInRange(ref charSearchSpace, charLowInclusive, charRange, length) + : PackedSpanHelpers.IndexOfAnyExceptInRange(ref charSearchSpace, charLowInclusive, charRange, length); } return NonPackedIndexOfAnyInRangeUnsignedNumber(ref searchSpace, lowInclusive, highInclusive, length); diff --git a/src/libraries/System.Private.CoreLib/src/System/String.Searching.cs b/src/libraries/System.Private.CoreLib/src/System/String.Searching.cs index feda6cc64889..331fd3392e98 100644 --- a/src/libraries/System.Private.CoreLib/src/System/String.Searching.cs +++ b/src/libraries/System.Private.CoreLib/src/System/String.Searching.cs @@ -79,7 +79,7 @@ private int IndexOfCharOrdinalIgnoreCase(char value) char valueUc = (char)(value | 0x20); char valueLc = (char)(value & ~0x20); return PackedSpanHelpers.PackedIndexOfIsSupported - ? PackedSpanHelpers.PackedIndexOfAny(ref _firstChar, valueLc, valueUc, Length) + ? PackedSpanHelpers.IndexOfAny(ref _firstChar, valueLc, valueUc, Length) : SpanHelpers.IndexOfAnyChar(ref _firstChar, valueLc, valueUc, Length); }