Port managed Array.Copy/Clear from CoreCLR version (dotnet#225)

Also fixes a few infrastructure issues that I have run into along the way.

Author: jkotas

docs/workflow/building/coreclr/nativeaot.md

@@ -8,8 +8,7 @@ The Native AOT toolchain can be currently built for Linux, macOS and Windows x64
 - Run `build[.cmd|.sh] nativeaot+libs+installer -rc [Debug|Release] -lc Release`. This will restore nuget packages required for building and build the parts of the repo required for the Native AOT toolchain.
 - The build will place the toolchain packages at `artifacts\packages\[Debug|Release]\Shipping`. To publish your project using these packages:
    - Add the package directory to your `nuget.config` file. For example, replace `dotnet-experimental` line in `samples\HelloWorld\nuget.config` with `<add key="local" value="C:\runtimelab\artifacts\packages\Debug\Shipping" />`
-   - Run `dotnet restore --packages pkg -r [win-x64|linux-x64|osx-64]` to restore the package into your project. `--package pkg` option restores the package into a local directory that is easy to cleanup once you are done. It avoids polluting the global nuget cache with your locally built dev package.
-   - Publish your project as usual: `dotnet publish -r [win-x64|linux-x64|osx-64] -c Release`.
+   - Run `dotnet publish --packages pkg -r [win-x64|linux-x64|osx-64] -c [Debug|Release]` to publish your project. `--package pkg` option restores the package into a local directory that is easy to cleanup once you are done. It avoids polluting the global nuget cache with your locally built dev package.
 
 ## Visual Studio Solutions
 

src/coreclr/src/dlls/mscoree/coreclr/CMakeLists.txt

@@ -109,7 +109,7 @@ set(CORECLR_LIBRARIES
     ildbsymlib
     utilcode
     v3binder
-    System.Globalization.Native-static
+    System.Globalization.Native-Static
     interop
 )
 

src/coreclr/src/nativeaot/Runtime/MiscHelpers.cpp

@@ -366,96 +366,6 @@ COOP_PINVOKE_HELPER(UInt8 *, RhGetCodeTarget, (UInt8 * pCodeOrg))
     return pCodeOrg;
 }
 
-//
-// Return true if the array slice is valid
-//
-FORCEINLINE bool CheckArraySlice(Array * pArray, Int32 index, Int32 length)
-{
-    Int32 arrayLength = pArray->GetArrayLength();
-
-    return (0 <= index) && (index <= arrayLength) &&
-           (0 <= length) && (length <= arrayLength) &&
-           (length <= arrayLength - index);
-}
-
-//
-// This function handles all cases of Array.Copy that do not require conversions or casting. It returns false if the copy cannot be performed, leaving
-// the handling of the complex cases or throwing appropriate exception to the higher level framework.
-//
-COOP_PINVOKE_HELPER(Boolean, RhpArrayCopy, (Array * pSourceArray, Int32 sourceIndex, Array * pDestinationArray, Int32 destinationIndex, Int32 length))
-{
-    if (pSourceArray == NULL || pDestinationArray == NULL)
-        return false;
-
-    EEType* pArrayType = pSourceArray->get_EEType();
-    EEType* pDestinationArrayType = pDestinationArray->get_EEType();
-    if (pArrayType != pDestinationArrayType)
-    {
-        if (!pArrayType->IsEquivalentTo(pDestinationArrayType))
-           return false;
-    }
-
-    size_t componentSize = pArrayType->get_ComponentSize();
-    if (componentSize == 0) // Not an array
-        return false;
-
-    if (!CheckArraySlice(pSourceArray, sourceIndex, length))
-        return false;
-
-    if (!CheckArraySlice(pDestinationArray, destinationIndex, length))
-        return false;
-
-    if (length == 0)
-        return true;
-
-    UInt8 * pSourceData = (UInt8 *)pSourceArray->GetArrayData() + sourceIndex * componentSize;
-    UInt8 * pDestinationData = (UInt8 *)pDestinationArray->GetArrayData() + destinationIndex * componentSize;
-    size_t size = length * componentSize;
-
-    if (pArrayType->HasReferenceFields())
-    {
-        if (pDestinationData <= pSourceData || pSourceData + size <= pDestinationData)
-            InlineForwardGCSafeCopy(pDestinationData, pSourceData, size);
-        else
-            InlineBackwardGCSafeCopy(pDestinationData, pSourceData, size);
-
-        InlinedBulkWriteBarrier(pDestinationData, size);
-    }
-    else
-    {
-        memmove(pDestinationData, pSourceData, size);
-    }
-
-    return true;
-}
-
-//
-// This function handles all cases of Array.Clear that do not require conversions. It returns false if the operation cannot be performed, leaving
-// the handling of the complex cases or throwing appropriate exception to the higher level framework. It is only allowed to return false for illegal
-// calls as the BCL side has fallback for "complex cases" only.
-//
-COOP_PINVOKE_HELPER(Boolean, RhpArrayClear, (Array * pArray, Int32 index, Int32 length))
-{
-    if (pArray == NULL)
-        return false;
-
-    EEType* pArrayType = pArray->get_EEType();
-
-    size_t componentSize = pArrayType->get_ComponentSize();
-    if (componentSize == 0) // Not an array
-        return false;
-
-    if (!CheckArraySlice(pArray, index, length))
-        return false;
-
-    if (length == 0)
-        return true;
-
-    InlineGCSafeFillMemory((UInt8 *)pArray->GetArrayData() + index * componentSize, length * componentSize, 0);
-
-    return true;
-}
-
 // Get the universal transition thunk. If the universal transition stub is called through
 // the normal PE static linkage model, a jump stub would be used which may interfere with
 // the custom calling convention of the universal transition thunk. So instead, a special

src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Array.CoreRT.cs

@@ -227,30 +227,74 @@ private ref int GetRawMultiDimArrayBounds()
             return ref Unsafe.AddByteOffset(ref _numComponents, POINTER_SIZE);
         }
 
-        // Copies length elements from sourceArray, starting at sourceIndex, to
-        // destinationArray, starting at destinationIndex.
-        //
-        public static void Copy(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length)
-        {
-            if (!RuntimeImports.TryArrayCopy(sourceArray, sourceIndex, destinationArray, destinationIndex, length))
-                CopyImpl(sourceArray, sourceIndex, destinationArray, destinationIndex, length, false);
-        }
-
         // Provides a strong exception guarantee - either it succeeds, or
         // it throws an exception with no side effects.  The arrays must be
         // compatible array types based on the array element type - this
         // method does not support casting, boxing, or primitive widening.
         // It will up-cast, assuming the array types are correct.
         public static void ConstrainedCopy(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length)
         {
-            if (!RuntimeImports.TryArrayCopy(sourceArray, sourceIndex, destinationArray, destinationIndex, length))
-                CopyImpl(sourceArray, sourceIndex, destinationArray, destinationIndex, length, true);
+            CopyImpl(sourceArray, sourceIndex, destinationArray, destinationIndex, length, reliable: true);
         }
 
         public static void Copy(Array sourceArray, Array destinationArray, int length)
         {
-            if (!RuntimeImports.TryArrayCopy(sourceArray, 0, destinationArray, 0, length))
-                CopyImpl(sourceArray, 0, destinationArray, 0, length, false);
+            if (sourceArray is null)
+                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.sourceArray);
+            if (destinationArray is null)
+                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.destinationArray);
+
+            EETypePtr eeType = sourceArray.EETypePtr;
+            if (eeType.FastEquals(destinationArray.EETypePtr) &&
+                eeType.IsSzArray &&
+                (uint)length <= (nuint)sourceArray.LongLength &&
+                (uint)length <= (nuint)destinationArray.LongLength)
+            {
+                nuint byteCount = (uint)length * (nuint)eeType.ComponentSize;
+                ref byte src = ref Unsafe.As<RawArrayData>(sourceArray).Data;
+                ref byte dst = ref Unsafe.As<RawArrayData>(destinationArray).Data;
+
+                if (eeType.HasPointers)
+                    Buffer.BulkMoveWithWriteBarrier(ref dst, ref src, byteCount);
+                else
+                    Buffer.Memmove(ref dst, ref src, byteCount);
+
+                // GC.KeepAlive(sourceArray) not required. pMT kept alive via sourceArray
+                return;
+            }
+
+            // Less common
+            CopyImpl(sourceArray, sourceArray.GetLowerBound(0), destinationArray, destinationArray.GetLowerBound(0), length, reliable: false);
+        }
+
+        public static unsafe void Copy(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length)
+        {
+            if (sourceArray != null && destinationArray != null)
+            {
+                EETypePtr eeType = sourceArray.EETypePtr;
+                if (eeType.FastEquals(destinationArray.EETypePtr) &&
+                    eeType.IsSzArray &&
+                    length >= 0 && sourceIndex >= 0 && destinationIndex >= 0 &&
+                    (uint)(sourceIndex + length) <= (nuint)sourceArray.LongLength &&
+                    (uint)(destinationIndex + length) <= (nuint)destinationArray.LongLength)
+                {
+                    nuint elementSize = (nuint)eeType.ComponentSize;
+                    nuint byteCount = (uint)length * elementSize;
+                    ref byte src = ref Unsafe.AddByteOffset(ref Unsafe.As<RawArrayData>(sourceArray).Data, (uint)sourceIndex * elementSize);
+                    ref byte dst = ref Unsafe.AddByteOffset(ref Unsafe.As<RawArrayData>(destinationArray).Data, (uint)destinationIndex * elementSize);
+
+                    if (eeType.HasPointers)
+                        Buffer.BulkMoveWithWriteBarrier(ref dst, ref src, byteCount);
+                    else
+                        Buffer.Memmove(ref dst, ref src, byteCount);
+
+                    // GC.KeepAlive(sourceArray) not required. pMT kept alive via sourceArray
+                    return;
+                }
+            }
+
+            // Less common
+            CopyImpl(sourceArray!, sourceIndex, destinationArray!, destinationIndex, length, reliable: false);
         }
 
         //
@@ -260,9 +304,9 @@ public static void Copy(Array sourceArray, Array destinationArray, int length)
         private static unsafe void CopyImpl(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length, bool reliable)
         {
             if (sourceArray is null)
-                throw new ArgumentNullException(nameof(sourceArray));
+                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.sourceArray);
             if (destinationArray is null)
-                throw new ArgumentNullException(nameof(destinationArray));
+                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.destinationArray);
 
             int sourceRank = sourceArray.Rank;
             int destinationRank = destinationArray.Rank;
@@ -836,24 +880,38 @@ private static unsafe void CopyImplPrimitiveTypeWithWidening(Array sourceArray,
             }
         }
 
-        public static void Clear(Array array, int index, int length)
+        public static unsafe void Clear(Array array, int index, int length)
         {
-            if (!RuntimeImports.TryArrayClear(array, index, length))
-                ReportClearErrors(array, index, length);
-        }
+            if (array == null)
+                ThrowHelper.ThrowArgumentNullException(ExceptionArgument.array);
 
-        private static unsafe void ReportClearErrors(Array array, int index, int length)
-        {
-            if (array is null)
-                throw new ArgumentNullException(nameof(array));
+            ref byte p = ref Unsafe.As<RawArrayData>(array).Data;
+            int lowerBound = 0;
 
-            if (index < 0 || index > array.Length || length < 0 || length > array.Length)
-                throw new IndexOutOfRangeException();
-            if (length > (array.Length - index))
-                throw new IndexOutOfRangeException();
+            EETypePtr eeType = array.EETypePtr;
+            if (!eeType.IsSzArray)
+            {
+                int rank = eeType.ArrayRank;
+                lowerBound = Unsafe.Add(ref Unsafe.As<byte, int>(ref p), rank);
+                p = ref Unsafe.Add(ref p, 2 * sizeof(int) * rank); // skip the bounds
+            }
+
+            int offset = index - lowerBound;
+
+            if (index < lowerBound || offset < 0 || length < 0 || (uint)(offset + length) > (nuint)array.LongLength)
+                ThrowHelper.ThrowIndexOutOfRangeException();
+
+            nuint elementSize = eeType.ComponentSize;
+
+            ref byte ptr = ref Unsafe.AddByteOffset(ref p, (uint)offset * elementSize);
+            nuint byteLength = (uint)length * elementSize;
+
+            if (eeType.HasPointers)
+                SpanHelpers.ClearWithReferences(ref Unsafe.As<byte, IntPtr>(ref ptr), byteLength / (uint)sizeof(IntPtr));
+            else
+                SpanHelpers.ClearWithoutReferences(ref ptr, byteLength);
 
-            // The above checks should have covered all the reasons why Clear would fail.
-            Debug.Assert(false);
+            // GC.KeepAlive(array) not required. pMT kept alive via `ptr`
         }
 
         public int GetLength(int dimension)

src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Buffer.CoreRT.cs

@@ -42,9 +42,9 @@ internal static unsafe void Memmove<T>(ref T destination, ref T source, nuint el
             {
                 // Blittable memmove
 
-                RuntimeImports.memmove(
-                    (byte*)Unsafe.AsPointer(ref destination),
-                    (byte*)Unsafe.AsPointer(ref source),
+                Memmove(
+                    ref Unsafe.As<T, byte>(ref destination),
+                    ref Unsafe.As<T, byte>(ref source),
                     elementCount * (nuint)Unsafe.SizeOf<T>());
             }
             else

src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/EETypePtr.cs

@@ -93,19 +93,6 @@ public bool FastEquals(EETypePtr other)
             return RuntimeImports.AreTypesEquivalent(this, other);
         }
 
-        //
-        // An even faster version of FastEquals that only checks if two EEType pointers are identical.
-        // Note: this method might return false for cases where FastEquals would return true.
-        // Only use if you know what you're doing.
-        //
-        internal bool FastEqualsUnreliable(EETypePtr other)
-        {
-            Debug.Assert(!this.IsNull);
-            Debug.Assert(!other.IsNull);
-
-            return this.RawValue == other.RawValue;
-        }
-
         // Caution: You cannot safely compare RawValue's as RH does NOT unify EETypes. Use the == or Equals() methods exposed by EETypePtr itself.
         internal IntPtr RawValue
         {

src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs

@@ -1071,14 +1071,6 @@ internal static float fabsf(float x)
         [DllImport(RuntimeImports.RuntimeLibrary, ExactSpelling = true)]
         internal static extern unsafe void* memset(byte* mem, int value, nuint size);
 
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        [RuntimeImport(RuntimeLibrary, "RhpArrayCopy")]
-        internal static extern bool TryArrayCopy(Array sourceArray, int sourceIndex, Array destinationArray, int destinationIndex, int length);
-
-        [MethodImpl(MethodImplOptions.InternalCall)]
-        [RuntimeImport(RuntimeLibrary, "RhpArrayClear")]
-        internal static extern bool TryArrayClear(Array array, int index, int length);
-
 #if TARGET_X86 || TARGET_AMD64
         [DllImport(RuntimeLibrary, ExactSpelling = true, CallingConvention = CallingConvention.Cdecl)]
         internal static extern unsafe void RhCpuIdEx(int* cpuInfo, int functionId, int subFunctionId);