Addressing PR comments
authorahsonkhan <ahson_ahmedk@yahoo.com>
Tue, 21 Feb 2017 19:58:04 +0000 (11:58 -0800)
committerahsonkhan <ahson_ahmedk@yahoo.com>
Tue, 21 Feb 2017 19:58:04 +0000 (11:58 -0800)
Commit migrated from https://github.com/dotnet/coreclr/commit/05abafc89e73196ba3ce7587d10728c41eed05e3

src/coreclr/src/mscorlib/src/System/Runtime/RuntimeImports.cs
src/coreclr/src/mscorlib/src/System/Span.cs

index 745a9e8..b775dbf 100644 (file)
@@ -27,6 +27,6 @@ namespace System.Runtime
         }
 
         [DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
-        extern internal unsafe static void RhZeroMemory(byte* b, nuint byteLength);
+        extern private unsafe static void RhZeroMemory(byte* b, nuint byteLength);
     }
 }
index 95039fb..7435a93 100644 (file)
@@ -642,211 +642,207 @@ namespace System
             if (byteLength == 0)
                 return;
 
-            fixed (byte* bytePointer = &b)
-            {
-                // Note: It's important that this switch handles lengths at least up to 22.
-                // See notes below near the main loop for why.
+            // Note: It's important that this switch handles lengths at least up to 22.
+            // See notes below near the main loop for why.
 
-                // The switch will be very fast since it can be implemented using a jump
-                // table in assembly. See http://stackoverflow.com/a/449297/4077294 for more info.
+            // The switch will be very fast since it can be implemented using a jump
+            // table in assembly. See http://stackoverflow.com/a/449297/4077294 for more info.
 
-                switch (byteLength)
-                {
-                    case 1:
-                        *bytePointer = (byte)0;
-                        return;
-                    case 2:
-                        *(short*)bytePointer = (short)0;
-                        return;
-                    case 3:
-                        *(short*)bytePointer = (short)0;
-                        *(bytePointer + 2) = (byte)0;
-                        return;
-                    case 4:
-                        *(int*)bytePointer = (int)0;
-                        return;
-                    case 5:
-                        *(int*)bytePointer = (int)0;
-                        *(bytePointer + 4) = (byte)0;
-                        return;
-                    case 6:
-                        *(int*)bytePointer = (int)0;
-                        *(short*)(bytePointer + 4) = (short)0;
-                        return;
-                    case 7:
-                        *(int*)bytePointer = (int)0;
-                        *(short*)(bytePointer + 4) = (short)0;
-                        *(bytePointer + 6) = (byte)0;
-                        return;
-                    case 8:
-                        *(long*)bytePointer = (long)0;
-                        return;
-                    case 9:
-                        *(long*)bytePointer = (long)0;
-                        *(bytePointer + 8) = (byte)0;
-                        return;
-                    case 10:
-                        *(long*)bytePointer = (long)0;
-                        *(short*)(bytePointer + 8) = (short)0;
-                        return;
-                    case 11:
-                        *(long*)bytePointer = (long)0;
-                        *(short*)(bytePointer + 8) = (short)0;
-                        *(bytePointer + 10) = (byte)0;
-                        return;
-                    case 12:
-                        *(long*)bytePointer = (long)0;
-                        *(int*)(bytePointer + 8) = (int)0;
-                        return;
-                    case 13:
-                        *(long*)bytePointer = (long)0;
-                        *(int*)(bytePointer + 8) = (int)0;
-                        *(bytePointer + 12) = (byte)0;
-                        return;
-                    case 14:
-                        *(long*)bytePointer = (long)0;
-                        *(int*)(bytePointer + 8) = (int)0;
-                        *(short*)(bytePointer + 12) = (short)0;
-                        return;
-                    case 15:
-                        *(long*)bytePointer = (long)0;
-                        *(int*)(bytePointer + 8) = (int)0;
-                        *(short*)(bytePointer + 12) = (short)0;
-                        *(bytePointer + 14) = (byte)0;
-                        return;
-                    case 16:
-                        *(long*)bytePointer = (long)0;
-                        *(long*)(bytePointer + 8) = (long)0;
-                        return;
-                    case 17:
-                        *(long*)bytePointer = (long)0;
-                        *(long*)(bytePointer + 8) = (long)0;
-                        *(bytePointer + 16) = (byte)0;
-                        return;
-                    case 18:
-                        *(long*)bytePointer = (long)0;
-                        *(long*)(bytePointer + 8) = (long)0;
-                        *(short*)(bytePointer + 16) = (short)0;
-                        return;
-                    case 19:
-                        *(long*)bytePointer = (long)0;
-                        *(long*)(bytePointer + 8) = (long)0;
-                        *(short*)(bytePointer + 16) = (short)0;
-                        *(bytePointer + 18) = (byte)0;
-                        return;
-                    case 20:
-                        *(long*)bytePointer = (long)0;
-                        *(long*)(bytePointer + 8) = (long)0;
-                        *(int*)(bytePointer + 16) = (int)0;
-                        return;
-                    case 21:
-                        *(long*)bytePointer = (long)0;
-                        *(long*)(bytePointer + 8) = (long)0;
-                        *(int*)(bytePointer + 16) = (int)0;
-                        *(bytePointer + 20) = (byte)0;
-                        return;
-                    case 22:
-                        *(long*)bytePointer = (long)0;
-                        *(long*)(bytePointer + 8) = (long)0;
-                        *(int*)(bytePointer + 16) = (int)0;
-                        *(short*)(bytePointer + 20) = (short)0;
-                        return;
-                }
+            switch (byteLength)
+            {
+                case 1:
+                    b = 0;
+                    return;
+                case 2:
+                    Unsafe.As<byte, short>(ref b) = 0;
+                    return;
+                case 3:
+                    Unsafe.As<byte, short>(ref b) = 0;
+                    Unsafe.Add<byte>(ref b, 2) = 0;
+                    return;
+                case 4:
+                    Unsafe.As<byte, int>(ref b) = 0;
+                    return;
+                case 5:
+                    Unsafe.As<byte, int>(ref b) = 0;
+                    Unsafe.Add<byte>(ref b, 4) = 0;
+                    return;
+                case 6:
+                    Unsafe.As<byte, int>(ref b) = 0;
+                    Unsafe.As<byte, short>(ref Unsafe.Add<byte>(ref b, 4)) = 0;
+                    return;
+                case 7:
+                    Unsafe.As<byte, int>(ref b) = 0;
+                    Unsafe.As<byte, short>(ref Unsafe.Add<byte>(ref b, 4)) = 0;
+                    Unsafe.Add<byte>(ref b, 6) = 0;
+                    return;
+                case 8:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    return;
+                case 9:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.Add<byte>(ref b, 8) = 0;
+                    return;
+                case 10:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, short>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    return;
+                case 11:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, short>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    Unsafe.Add<byte>(ref b, 10) = 0;
+                    return;
+                case 12:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, int>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    return;
+                case 13:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, int>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    Unsafe.Add<byte>(ref b, 12) = 0;
+                    return;
+                case 14:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, int>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    Unsafe.As<byte, short>(ref Unsafe.Add<byte>(ref b, 12)) = 0;
+                    return;
+                case 15:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, int>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    Unsafe.As<byte, short>(ref Unsafe.Add<byte>(ref b, 12)) = 0;
+                    Unsafe.Add<byte>(ref b, 14) = 0;
+                    return;
+                case 16:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, long>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    return;
+                case 17:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, long>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    Unsafe.Add<byte>(ref b, 16) = 0;
+                    return;
+                case 18:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, long>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    Unsafe.As<byte, short>(ref Unsafe.Add<byte>(ref b, 16)) = 0;
+                    return;
+                case 19:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, long>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    Unsafe.As<byte, short>(ref Unsafe.Add<byte>(ref b, 16)) = 0;
+                    Unsafe.Add<byte>(ref b, 18) = 0;
+                    return;
+                case 20:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, long>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    Unsafe.As<byte, int>(ref Unsafe.Add<byte>(ref b, 16)) = 0;
+                    return;
+                case 21:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, long>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    Unsafe.As<byte, int>(ref Unsafe.Add<byte>(ref b, 16)) = 0;
+                    Unsafe.Add<byte>(ref b, 20) = 0;
+                    return;
+                case 22:
+                    Unsafe.As<byte, long>(ref b) = 0;
+                    Unsafe.As<byte, long>(ref Unsafe.Add<byte>(ref b, 8)) = 0;
+                    Unsafe.As<byte, int>(ref Unsafe.Add<byte>(ref b, 16)) = 0;
+                    Unsafe.As<byte, short>(ref Unsafe.Add<byte>(ref b, 20)) = 0;
+                    return;
+            }
 
-                // P/Invoke into the native version for large lengths
-                if (byteLength >= 512) goto PInvoke;
+            // P/Invoke into the native version for large lengths
+            if (byteLength >= 512)
+            {
+                RuntimeImports.RhZeroMemory(ref b, byteLength);
+                return;
+            }
 
-                nuint i = 0; // byte offset at which we're copying
+            nuint i = 0; // byte offset at which we're copying
 
-                if (((int)bytePointer & 3) != 0)
+            if ((Unsafe.As<byte, int>(ref b) & 3) != 0)
+            {
+                if ((Unsafe.As<byte, int>(ref b) & 1) != 0)
                 {
-                    if (((int)bytePointer & 1) != 0)
-                    {
-                        *(bytePointer + i) = (byte)0;
-                        i += 1;
-                        if (((int)bytePointer & 2) != 0)
-                            goto IntAligned;
-                    }
-                    *(short*)(bytePointer + i) = (short)0;
-                    i += 2;
+                    Unsafe.Add<byte>(ref b, (int)i) = 0;
+                    i += 1;
+                    if ((Unsafe.As<byte, int>(ref b) & 2) != 0)
+                        goto IntAligned;
                 }
+                Unsafe.As<byte, short>(ref Unsafe.Add<byte>(ref b, (int)i)) = 0;
+                i += 2;
+            }
 
-                IntAligned:
-
-                // On 64-bit IntPtr.Size == 8, so we want to advance to the next 8-aligned address. If
-                // (int)bytePointer % 8 is 0, 5, 6, or 7, we will already have advanced by 0, 3, 2, or 1
-                // bytes to the next aligned address (respectively), so do nothing. On the other hand,
-                // if it is 1, 2, 3, or 4 we will want to copy-and-advance another 4 bytes until
-                // we're aligned.
-                // The thing 1, 2, 3, and 4 have in common that the others don't is that if you
-                // subtract one from them, their 3rd lsb will not be set. Hence, the below check.
-
-                if ((((int)bytePointer - 1) & 4) == 0)
-                {
-                    *(int*)(bytePointer + i) = (int)0;
-                    i += 4;
-                }
+            IntAligned:
 
-                nuint end = byteLength - 16;
-                byteLength -= i; // lower 4 bits of byteLength represent how many bytes are left *after* the unrolled loop
+            // On 64-bit IntPtr.Size == 8, so we want to advance to the next 8-aligned address. If
+            // (int)bytePointer % 8 is 0, 5, 6, or 7, we will already have advanced by 0, 3, 2, or 1
+            // bytes to the next aligned address (respectively), so do nothing. On the other hand,
+            // if it is 1, 2, 3, or 4 we will want to copy-and-advance another 4 bytes until
+            // we're aligned.
+            // The thing 1, 2, 3, and 4 have in common that the others don't is that if you
+            // subtract one from them, their 3rd lsb will not be set. Hence, the below check.
 
-                // We know due to the above switch-case that this loop will always run 1 iteration; max
-                // bytes we clear before checking is 23 (7 to align the pointers, 16 for 1 iteration) so
-                // the switch handles lengths 0-22.
-                Debug.Assert(end >= 7 && i <= end);
+            if (((Unsafe.As<byte, int>(ref b) - 1) & 4) == 0)
+            {
+                Unsafe.As<byte, int>(ref Unsafe.Add<byte>(ref b, (int)i)) = 0;
+                i += 4;
+            }
 
-                // This is separated out into a different variable, so the i + 16 addition can be
-                // performed at the start of the pipeline and the loop condition does not have
-                // a dependency on the writes.
-                nuint counter;
+            nuint end = byteLength - 16;
+            byteLength -= i; // lower 4 bits of byteLength represent how many bytes are left *after* the unrolled loop
 
-                do
-                {
-                    counter = i + 16;
+            // We know due to the above switch-case that this loop will always run 1 iteration; max
+            // bytes we clear before checking is 23 (7 to align the pointers, 16 for 1 iteration) so
+            // the switch handles lengths 0-22.
+            Debug.Assert(end >= 7 && i <= end);
 
-                    // This loop looks very costly since there appear to be a bunch of temporary values
-                    // being created with the adds, but the jit (for x86 anyways) will convert each of
-                    // these to use memory addressing operands.
+            // This is separated out into a different variable, so the i + 16 addition can be
+            // performed at the start of the pipeline and the loop condition does not have
+            // a dependency on the writes.
+            nuint counter;
 
-                    // So the only cost is a bit of code size, which is made up for by the fact that
-                    // we save on writes to bytePointer.
+            do
+            {
+                counter = i + 16;
 
-                    *(long*)(bytePointer + i) = (long)0;
-                    *(long*)(bytePointer + i + 8) = (long)0;
+                // This loop looks very costly since there appear to be a bunch of temporary values
+                // being created with the adds, but the jit (for x86 anyways) will convert each of
+                // these to use memory addressing operands.
 
-                    i = counter;
+                // So the only cost is a bit of code size, which is made up for by the fact that
+                // we save on writes to bytePointer.
 
-                    // See notes above for why this wasn't used instead
-                    // i += 16;
-                }
-                while (counter <= end);
+                Unsafe.As<byte, long>(ref Unsafe.Add<byte>(ref b, (int)i)) = 0;
+                Unsafe.As<byte, long>(ref Unsafe.Add<byte>(ref b, (int)i + 8)) = 0;
 
-                if ((byteLength & 8) != 0)
-                {
-                    *(long*)(bytePointer + i) = (long)0;
-                    i += 8;
-                }
-                if ((byteLength & 4) != 0)
-                {
-                    *(int*)(bytePointer + i) = (int)0;
-                    i += 4;
-                }
-                if ((byteLength & 2) != 0)
-                {
-                    *(short*)(bytePointer + i) = (short)0;
-                    i += 2;
-                }
-                if ((byteLength & 1) != 0)
-                {
-                    *(bytePointer + i) = (byte)0;
-                    // We're not using i after this, so not needed
-                    // i += 1;
-                }
+                i = counter;
 
-                return;
+                // See notes above for why this wasn't used instead
+                // i += 16;
+            }
+            while (counter <= end);
 
-                PInvoke:
-                RuntimeImports.RhZeroMemory(ref b, byteLength);
+            if ((byteLength & 8) != 0)
+            {
+                Unsafe.As<byte, long>(ref Unsafe.Add<byte>(ref b, (int)i)) = 0;
+                i += 8;
+            }
+            if ((byteLength & 4) != 0)
+            {
+                Unsafe.As<byte, int>(ref Unsafe.Add<byte>(ref b, (int)i)) = 0;
+                i += 4;
+            }
+            if ((byteLength & 2) != 0)
+            {
+                Unsafe.As<byte, short>(ref Unsafe.Add<byte>(ref b, (int)i)) = 0;
+                i += 2;
+            }
+            if ((byteLength & 1) != 0)
+            {
+                Unsafe.Add<byte>(ref b, (int)i) = 0;
+                // We're not using i after this, so not needed
+                // i += 1;
             }
         }