PR feedback: CountNumberOfLeadingAsciiBytesFrom24BitInteger
authorLevi Broderick <levib@microsoft.com>
Thu, 11 Apr 2019 18:11:49 +0000 (11:11 -0700)
committerLevi Broderick <levib@microsoft.com>
Thu, 11 Apr 2019 18:11:49 +0000 (11:11 -0700)
src/System.Private.CoreLib/shared/System/Text/ASCIIUtility.Helpers.cs
src/System.Private.CoreLib/shared/System/Text/ASCIIUtility.cs
src/System.Private.CoreLib/shared/System/Text/Unicode/Utf8Utility.Validation.cs

index b48a001..731d52a 100644 (file)
@@ -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;
         }
 
-
         /// <summary>
-        /// 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.)
         /// </summary>
         [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);
index 6193a0a..18a668a 100644 (file)
@@ -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;
 
index 671bf1f..8ef9369 100644 (file)
@@ -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)