From 280db356e5fed7d967ab31ea3abbdd911379300b Mon Sep 17 00:00:00 2001 From: Prashanth Govindarajan Date: Wed, 29 Jul 2020 17:09:13 -0700 Subject: [PATCH 1/4] GetIndexOfFirstNonAsciiChar --- .../src/System/Text/ASCIIUtility.cs | 156 +++++++++++++++--- .../Text/Unicode/Utf16Utility.Validation.cs | 2 +- 2 files changed, 138 insertions(+), 20 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs b/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs index 76075a5e66dc48..c23c9781dcb853 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs @@ -489,8 +489,8 @@ public static unsafe nuint GetIndexOfFirstNonAsciiChar(char* pBuffer, nuint buff // pmovmskb which we know are optimized, and (b) we can avoid downclocking the processor while // this method is running. - return (Sse2.IsSupported) - ? GetIndexOfFirstNonAsciiChar_Sse2(pBuffer, bufferLength) + return (BitConverter.IsLittleEndian && (Sse2.IsSupported || AdvSimd.Arm64.IsSupported)) + ? GetIndexOfFirstNonAsciiChar_Sse2OrArm64(pBuffer, bufferLength) : GetIndexOfFirstNonAsciiChar_Default(pBuffer, bufferLength); } @@ -630,9 +630,9 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Default(char* pBuffer, n goto Finish; } - private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuint bufferLength /* in chars */) + private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2OrArm64(char* pBuffer, nuint bufferLength /* in chars */) { - // This method contains logic optimized for both SSE2 and SSE41. Much of the logic in this method + // This method contains logic optimized for SSE2, SSE41 and ARM64. Much of the logic in this method // will be elided by JIT once we determine which specific ISAs we support. // Quick check for empty inputs. @@ -647,9 +647,10 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin uint SizeOfVector128InBytes = (uint)Unsafe.SizeOf>(); uint SizeOfVector128InChars = SizeOfVector128InBytes / sizeof(char); - Debug.Assert(Sse2.IsSupported, "Should've been checked by caller."); - Debug.Assert(BitConverter.IsLittleEndian, "SSE2 assumes little-endian."); + Debug.Assert(Sse2.IsSupported || AdvSimd.Arm64.IsSupported, "Sse2 or AdvSimd64 required."); + Debug.Assert(BitConverter.IsLittleEndian, "This SSE2/Arm64 implementation assumes little-endian."); + Vector128 bitmask = Vector128.Create((ushort)0x1001).AsByte(); Vector128 firstVector, secondVector; uint currentMask; char* pOriginalBuffer = pBuffer; @@ -673,13 +674,35 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin // Read the first vector unaligned. - firstVector = Sse2.LoadVector128((ushort*)pBuffer); // unaligned load + if (Sse2.IsSupported) + { + firstVector = Sse2.LoadVector128((ushort*)pBuffer); // unaligned load + } + else if (AdvSimd.Arm64.IsSupported) + { + firstVector = AdvSimd.LoadVector128((ushort*)pBuffer); // unaligned load + } + else + { + throw new PlatformNotSupportedException(); + } // The operation below forces the 0x8000 bit of each WORD to be set iff the WORD element - // has value >= 0x0800 (non-ASCII). Then we'll treat the vector as a BYTE vector in order + // has value >= 0x0080 (non-ASCII). Then we'll treat the vector as a BYTE vector in order // to extract the mask. Reminder: the 0x0080 bit of each WORD should be ignored. - currentMask = (uint)Sse2.MoveMask(Sse2.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte()); + if (Sse2.IsSupported) + { + currentMask = (uint)Sse2.MoveMask(Sse2.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte()); + } + else if (AdvSimd.Arm64.IsSupported) + { + currentMask = Unicode.Utf16Utility.GetNonAsciiBytes(AdvSimd.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte(), bitmask); + } + else + { + throw new PlatformNotSupportedException(); + } if ((currentMask & NonAsciiDataSeenMask) != 0) { @@ -725,9 +748,23 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin do { - firstVector = Sse2.LoadAlignedVector128((ushort*)pBuffer); - secondVector = Sse2.LoadAlignedVector128((ushort*)pBuffer + SizeOfVector128InChars); - Vector128 combinedVector = Sse2.Or(firstVector, secondVector); + Vector128 combinedVector; + if (Sse2.IsSupported) + { + firstVector = Sse2.LoadAlignedVector128((ushort*)pBuffer); + secondVector = Sse2.LoadAlignedVector128((ushort*)pBuffer + SizeOfVector128InChars); + combinedVector = Sse2.Or(firstVector, secondVector); + } + else if (AdvSimd.Arm64.IsSupported) + { + firstVector = AdvSimd.LoadVector128((ushort*)pBuffer); + secondVector = AdvSimd.LoadVector128((ushort*)pBuffer + SizeOfVector128InChars); + combinedVector = AdvSimd.Or(firstVector, secondVector); + } + else + { + throw new PlatformNotSupportedException(); + } if (Sse41.IsSupported) { @@ -738,7 +775,7 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin goto FoundNonAsciiDataInFirstOrSecondVector; } } - else + else if (Sse2.IsSupported) { // See comment earlier in the method for an explanation of how the below logic works. currentMask = (uint)Sse2.MoveMask(Sse2.AddSaturate(combinedVector, asciiMaskForAddSaturate).AsByte()); @@ -747,6 +784,18 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin goto FoundNonAsciiDataInFirstOrSecondVector; } } + else if (AdvSimd.Arm64.IsSupported) + { + currentMask = Unicode.Utf16Utility.GetNonAsciiBytes(AdvSimd.AddSaturate(combinedVector, asciiMaskForAddSaturate).AsByte(), bitmask); + if ((currentMask & NonAsciiDataSeenMask) != 0) + { + goto FoundNonAsciiDataInFirstOrSecondVector; + } + } + else + { + throw new PlatformNotSupportedException(); + } pBuffer += 2 * SizeOfVector128InChars; } while (pBuffer <= pFinalVectorReadPos); @@ -770,7 +819,18 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin // At least one full vector's worth of data remains, so we can safely read it. // Remember, at this point pBuffer is still aligned. - firstVector = Sse2.LoadAlignedVector128((ushort*)pBuffer); + if (Sse2.IsSupported) + { + firstVector = Sse2.LoadAlignedVector128((ushort*)pBuffer); + } + else if (AdvSimd.Arm64.IsSupported) + { + firstVector = AdvSimd.LoadVector128((ushort*)pBuffer); + } + else + { + throw new PlatformNotSupportedException(); + } if (Sse41.IsSupported) { @@ -781,7 +841,7 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin goto FoundNonAsciiDataInFirstVector; } } - else + else if (Sse2.IsSupported) { // See comment earlier in the method for an explanation of how the below logic works. currentMask = (uint)Sse2.MoveMask(Sse2.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte()); @@ -790,6 +850,18 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin goto FoundNonAsciiDataInCurrentMask; } } + else if (AdvSimd.Arm64.IsSupported) + { + currentMask = Unicode.Utf16Utility.GetNonAsciiBytes(AdvSimd.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte(), bitmask); + if ((currentMask & NonAsciiDataSeenMask) != 0) + { + goto FoundNonAsciiDataInCurrentMask; + } + } + else + { + throw new PlatformNotSupportedException(); + } IncrementCurrentOffsetBeforeFinalUnalignedVectorRead: @@ -803,7 +875,18 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin // We need to adjust the pointer because we're re-reading data. pBuffer = (char*)((byte*)pBuffer + (bufferLength & (SizeOfVector128InBytes - 1)) - SizeOfVector128InBytes); - firstVector = Sse2.LoadVector128((ushort*)pBuffer); // unaligned load + if (Sse2.IsSupported) + { + firstVector = Sse2.LoadVector128((ushort*)pBuffer); // unaligned load + } + else if (AdvSimd.Arm64.IsSupported) + { + firstVector = AdvSimd.LoadVector128((ushort*)pBuffer); // unaligned load + } + else + { + throw new PlatformNotSupportedException(); + } if (Sse41.IsSupported) { @@ -814,7 +897,7 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin goto FoundNonAsciiDataInFirstVector; } } - else + else if (Sse2.IsSupported) { // See comment earlier in the method for an explanation of how the below logic works. currentMask = (uint)Sse2.MoveMask(Sse2.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte()); @@ -823,6 +906,18 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin goto FoundNonAsciiDataInCurrentMask; } } + else if (AdvSimd.Arm64.IsSupported) + { + currentMask = Unicode.Utf16Utility.GetNonAsciiBytes(AdvSimd.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte(), bitmask); + if ((currentMask & NonAsciiDataSeenMask) != 0) + { + goto FoundNonAsciiDataInCurrentMask; + } + } + else + { + throw new PlatformNotSupportedException(); + } pBuffer += SizeOfVector128InChars; } @@ -846,7 +941,7 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin goto FoundNonAsciiDataInFirstVector; } } - else + else if (Sse2.IsSupported) { currentMask = (uint)Sse2.MoveMask(Sse2.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte()); if ((currentMask & NonAsciiDataSeenMask) != 0) @@ -854,6 +949,18 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin goto FoundNonAsciiDataInCurrentMask; } } + else if (AdvSimd.Arm64.IsSupported) + { + currentMask = Unicode.Utf16Utility.GetNonAsciiBytes(AdvSimd.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte(), bitmask); + if ((currentMask & NonAsciiDataSeenMask) != 0) + { + goto FoundNonAsciiDataInCurrentMask; + } + } + else + { + throw new PlatformNotSupportedException(); + } // Wasn't the first vector; must be the second. @@ -863,7 +970,18 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2(char* pBuffer, nuin FoundNonAsciiDataInFirstVector: // See comment earlier in the method for an explanation of how the below logic works. - currentMask = (uint)Sse2.MoveMask(Sse2.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte()); + if (Sse2.IsSupported) + { + currentMask = (uint)Sse2.MoveMask(Sse2.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte()); + } + else if (AdvSimd.Arm64.IsSupported) + { + currentMask = Unicode.Utf16Utility.GetNonAsciiBytes(AdvSimd.AddSaturate(firstVector, asciiMaskForAddSaturate).AsByte(), bitmask); + } + else + { + throw new PlatformNotSupportedException(); + } FoundNonAsciiDataInCurrentMask: diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf16Utility.Validation.cs b/src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf16Utility.Validation.cs index f2df0ccdf53c42..90c91ed2acd8ad 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf16Utility.Validation.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/Unicode/Utf16Utility.Validation.cs @@ -486,7 +486,7 @@ static Utf16Utility() } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static uint GetNonAsciiBytes(Vector128 value, Vector128 bitMask128) + internal static uint GetNonAsciiBytes(Vector128 value, Vector128 bitMask128) { Debug.Assert(AdvSimd.Arm64.IsSupported); From b4edde621fbcdf1376e5716da306ef066b6ce7bc Mon Sep 17 00:00:00 2001 From: Prashanth Govindarajan Date: Tue, 4 Aug 2020 10:42:37 -0700 Subject: [PATCH 2/4] Temp commit to change branches --- .../tests/Writer/WriteUtf8String.cs | 20 +++++++++---------- .../src/System/Text/ASCIIUtility.cs | 4 ++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Formats.Asn1/tests/Writer/WriteUtf8String.cs b/src/libraries/System.Formats.Asn1/tests/Writer/WriteUtf8String.cs index 11230e72ec3c6b..03e0d67dc6a880 100644 --- a/src/libraries/System.Formats.Asn1/tests/Writer/WriteUtf8String.cs +++ b/src/libraries/System.Formats.Asn1/tests/Writer/WriteUtf8String.cs @@ -10,16 +10,16 @@ public class WriteUtf8String : WriteCharacterString { public static IEnumerable ShortValidCases { get; } = new object[][] { - new object[] - { - string.Empty, - "00", - }, - new object[] - { - "hi", - "026869", - }, + //new object[] + //{ + // string.Empty, + // "00", + //}, + //new object[] + //{ + // "hi", + // "026869", + //}, new object[] { "Dr. & Mrs. Smith\u2010Jones \uFE60 children", diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs b/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs index c23c9781dcb853..8de9518e08889c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs @@ -490,7 +490,7 @@ public static unsafe nuint GetIndexOfFirstNonAsciiChar(char* pBuffer, nuint buff // this method is running. return (BitConverter.IsLittleEndian && (Sse2.IsSupported || AdvSimd.Arm64.IsSupported)) - ? GetIndexOfFirstNonAsciiChar_Sse2OrArm64(pBuffer, bufferLength) + ? GetIndexOfFirstNonAsciiChar_Intrinsified(pBuffer, bufferLength) : GetIndexOfFirstNonAsciiChar_Default(pBuffer, bufferLength); } @@ -630,7 +630,7 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Default(char* pBuffer, n goto Finish; } - private static unsafe nuint GetIndexOfFirstNonAsciiChar_Sse2OrArm64(char* pBuffer, nuint bufferLength /* in chars */) + private static unsafe nuint GetIndexOfFirstNonAsciiChar_Intrinsified(char* pBuffer, nuint bufferLength /* in chars */) { // This method contains logic optimized for SSE2, SSE41 and ARM64. Much of the logic in this method // will be elided by JIT once we determine which specific ISAs we support. From 9a7de4ea10e87d859faaf7ca4c8e12a4d367c5d1 Mon Sep 17 00:00:00 2001 From: Prashanth Govindarajan Date: Wed, 5 Aug 2020 17:52:00 -0700 Subject: [PATCH 3/4] Bug fix --- .../System.Private.CoreLib/src/System/Text/ASCIIUtility.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs b/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs index 8de9518e08889c..87f3ba9b432f00 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Text/ASCIIUtility.cs @@ -650,7 +650,10 @@ private static unsafe nuint GetIndexOfFirstNonAsciiChar_Intrinsified(char* pBuff Debug.Assert(Sse2.IsSupported || AdvSimd.Arm64.IsSupported, "Sse2 or AdvSimd64 required."); Debug.Assert(BitConverter.IsLittleEndian, "This SSE2/Arm64 implementation assumes little-endian."); - Vector128 bitmask = Vector128.Create((ushort)0x1001).AsByte(); + Vector128 bitmask = BitConverter.IsLittleEndian ? + Vector128.Create(0x80402010_08040201).AsByte() : + Vector128.Create(0x01020408_10204080).AsByte(); + Vector128 firstVector, secondVector; uint currentMask; char* pOriginalBuffer = pBuffer; From 15fd6a3c36d2b44d6fea88d8ce919d0d9f60a79c Mon Sep 17 00:00:00 2001 From: Prashanth Govindarajan Date: Thu, 6 Aug 2020 11:40:39 -0700 Subject: [PATCH 4/4] revert commenting out test case --- .../tests/Writer/WriteUtf8String.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Formats.Asn1/tests/Writer/WriteUtf8String.cs b/src/libraries/System.Formats.Asn1/tests/Writer/WriteUtf8String.cs index 03e0d67dc6a880..11230e72ec3c6b 100644 --- a/src/libraries/System.Formats.Asn1/tests/Writer/WriteUtf8String.cs +++ b/src/libraries/System.Formats.Asn1/tests/Writer/WriteUtf8String.cs @@ -10,16 +10,16 @@ public class WriteUtf8String : WriteCharacterString { public static IEnumerable ShortValidCases { get; } = new object[][] { - //new object[] - //{ - // string.Empty, - // "00", - //}, - //new object[] - //{ - // "hi", - // "026869", - //}, + new object[] + { + string.Empty, + "00", + }, + new object[] + { + "hi", + "026869", + }, new object[] { "Dr. & Mrs. Smith\u2010Jones \uFE60 children",