From 56fa69d278712fbdd4cb2648d283290842d612da Mon Sep 17 00:00:00 2001 From: Levi Broderick Date: Thu, 11 Apr 2019 11:11:49 -0700 Subject: [PATCH] PR feedback: CountNumberOfLeadingAsciiBytesFrom24BitInteger --- .../shared/System/Text/ASCIIUtility.Helpers.cs | 55 +++++++++++++++++----- .../shared/System/Text/ASCIIUtility.cs | 4 +- .../System/Text/Unicode/Utf8Utility.Validation.cs | 2 +- 3 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/System.Private.CoreLib/shared/System/Text/ASCIIUtility.Helpers.cs b/src/System.Private.CoreLib/shared/System/Text/ASCIIUtility.Helpers.cs index b48a001..731d52a 100644 --- a/src/System.Private.CoreLib/shared/System/Text/ASCIIUtility.Helpers.cs +++ b/src/System.Private.CoreLib/shared/System/Text/ASCIIUtility.Helpers.cs @@ -5,6 +5,7 @@ using System.Diagnostics; using System.Numerics; using System.Runtime.CompilerServices; +using System.Runtime.Intrinsics.X86; namespace System.Text { @@ -31,29 +32,59 @@ namespace System.Text return (value & UInt32HighBitsOnlyMask) == 0; } - /// - /// Given a 24-bit integer which represents a three-byte buffer read in machine endianness, - /// counts the number of consecutive ASCII bytes starting from the beginning of the buffer. - /// Returns a value 0 - 3, inclusive. (The caller is responsible for ensuring that an all- - /// ASCII value does not make its way to this method.) + /// Given a DWORD which represents a four-byte buffer read in machine endianness, and which + /// the caller has asserted contains a non-ASCII byte *somewhere* in the data, counts the + /// number of consecutive ASCII bytes starting from the beginning of the buffer. Returns + /// a value 0 - 3, inclusive. (The caller is responsible for ensuring that the buffer doesn't + /// contain all-ASCII data.) /// [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal static uint CountNumberOfLeadingAsciiBytesFrom24BitInteger(uint value) + internal static uint CountNumberOfLeadingAsciiBytesFromUInt32WithSomeNonAsciiData(uint value) { Debug.Assert(!AllBytesInUInt32AreAscii(value), "Caller shouldn't provide an all-ASCII value."); + // Use BMI1 directly rather than going through BitOperations. We only see a perf gain here + // if we're able to emit a real tzcnt instruction; the software fallback used by BitOperations + // is too slow for our purposes since we can provide our own faster, specialized software fallback. + + if (Bmi1.IsSupported) + { + Debug.Assert(BitConverter.IsLittleEndian); + return Bmi1.TrailingZeroCount(value & UInt32HighBitsOnlyMask) >> 3; + } + + // Couldn't emit tzcnt, use specialized software fallback. + // The 'allBytesUpToNowAreAscii' DWORD uses bit twiddling to hold a 1 or a 0 depending + // on whether all processed bytes were ASCII. Then we accumulate all of the + // results to calculate how many consecutive ASCII bytes are present. + + value = ~value; + if (BitConverter.IsLittleEndian) { - return (uint)BitOperations.TrailingZeroCount(value & UInt32HighBitsOnlyMask) >> 3; + // Read first byte + value >>= 7; + uint allBytesUpToNowAreAscii = value & 1; + uint numAsciiBytes = allBytesUpToNowAreAscii; + + // Read second byte + value >>= 8; + allBytesUpToNowAreAscii &= value; + numAsciiBytes += allBytesUpToNowAreAscii; + + // Read third byte + value >>= 8; + allBytesUpToNowAreAscii &= value; + numAsciiBytes += allBytesUpToNowAreAscii; + + return numAsciiBytes; } else { - // The 'allBytesUpToNowAreAscii' DWORD uses bit twiddling to hold a 1 or a 0 depending - // on whether all processed bytes were ASCII. Then we accumulate all of the - // results to calculate how many consecutive ASCII bytes are present. - - value = ~value; + // BinaryPrimitives.ReverseEndianness is only implemented as an intrinsic on + // little-endian platforms, so using it in this big-endian path would be too + // expensive. Instead we'll just change how we perform the shifts. // Read first byte value = BitOperations.RotateLeft(value, 1); diff --git a/src/System.Private.CoreLib/shared/System/Text/ASCIIUtility.cs b/src/System.Private.CoreLib/shared/System/Text/ASCIIUtility.cs index 6193a0a..18a668a 100644 --- a/src/System.Private.CoreLib/shared/System/Text/ASCIIUtility.cs +++ b/src/System.Private.CoreLib/shared/System/Text/ASCIIUtility.cs @@ -216,7 +216,7 @@ namespace System.Text // we get to the high byte; or (b) all of the earlier bytes are ASCII, so the high byte must be // non-ASCII. In both cases we only care about the low 24 bits. - pBuffer += CountNumberOfLeadingAsciiBytesFrom24BitInteger(currentUInt32); + pBuffer += CountNumberOfLeadingAsciiBytesFromUInt32WithSomeNonAsciiData(currentUInt32); goto Finish; } @@ -378,7 +378,7 @@ namespace System.Text uint currentDWord; Debug.Assert(!AllBytesInUInt32AreAscii(currentDWord), "Shouldn't be here unless we see non-ASCII data."); - pBuffer += CountNumberOfLeadingAsciiBytesFrom24BitInteger(currentDWord); + pBuffer += CountNumberOfLeadingAsciiBytesFromUInt32WithSomeNonAsciiData(currentDWord); goto Finish; diff --git a/src/System.Private.CoreLib/shared/System/Text/Unicode/Utf8Utility.Validation.cs b/src/System.Private.CoreLib/shared/System/Text/Unicode/Utf8Utility.Validation.cs index 671bf1f..8ef9369 100644 --- a/src/System.Private.CoreLib/shared/System/Text/Unicode/Utf8Utility.Validation.cs +++ b/src/System.Private.CoreLib/shared/System/Text/Unicode/Utf8Utility.Validation.cs @@ -211,7 +211,7 @@ namespace System.Text.Unicode // We only handle up to three ASCII bytes here since we handled the four ASCII byte case above. { - uint numLeadingAsciiBytes = ASCIIUtility.CountNumberOfLeadingAsciiBytesFrom24BitInteger(thisDWord); + uint numLeadingAsciiBytes = ASCIIUtility.CountNumberOfLeadingAsciiBytesFromUInt32WithSomeNonAsciiData(thisDWord); pInputBuffer += numLeadingAsciiBytes; if (pFinalPosWhereCanReadDWordFromInputBuffer < pInputBuffer) -- 2.7.4