From 35abaf5afefc1fd61dc8c446cbe9a0b50a38b179 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 13 Oct 2020 20:12:28 -0700 Subject: [PATCH 1/2] Port managed Array.Copy/Clear from CoreCLR version Also fixes a few infrastructure issues that I have run into along the way. --- docs/workflow/building/coreclr/nativeaot.md | 3 +- .../src/dlls/mscoree/coreclr/CMakeLists.txt | 2 +- .../src/nativeaot/Runtime/MiscHelpers.cpp | 90 ------------- .../src/System/Array.CoreRT.cs | 118 +++++++++++++----- .../src/System/Buffer.CoreRT.cs | 6 +- .../src/System/Runtime/RuntimeImports.cs | 8 -- 6 files changed, 93 insertions(+), 134 deletions(-) diff --git a/docs/workflow/building/coreclr/nativeaot.md b/docs/workflow/building/coreclr/nativeaot.md index af5baf7b078f..220cea4656b9 100644 --- a/docs/workflow/building/coreclr/nativeaot.md +++ b/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 `` - - 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 diff --git a/src/coreclr/src/dlls/mscoree/coreclr/CMakeLists.txt b/src/coreclr/src/dlls/mscoree/coreclr/CMakeLists.txt index 67cbf59fe947..92dac26ec279 100644 --- a/src/coreclr/src/dlls/mscoree/coreclr/CMakeLists.txt +++ b/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 ) diff --git a/src/coreclr/src/nativeaot/Runtime/MiscHelpers.cpp b/src/coreclr/src/nativeaot/Runtime/MiscHelpers.cpp index c95e516e26c7..50d071a96512 100644 --- a/src/coreclr/src/nativeaot/Runtime/MiscHelpers.cpp +++ b/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 diff --git a/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Array.CoreRT.cs b/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Array.CoreRT.cs index 7c9fa59c27c2..5b01d0de3c41 100644 --- a/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Array.CoreRT.cs +++ b/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Array.CoreRT.cs @@ -227,15 +227,6 @@ 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 @@ -243,14 +234,67 @@ public static void Copy(Array sourceArray, int sourceIndex, Array destinationArr // 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) + public static unsafe 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); + + EEType* pEEType = sourceArray.EEType; + if (pEEType == destinationArray.EEType && + pEEType->IsSzArray && + (uint)length <= (nuint)sourceArray.LongLength && + (uint)length <= (nuint)destinationArray.LongLength) + { + nuint byteCount = (uint)length * (nuint)pEEType->ComponentSize; + ref byte src = ref Unsafe.As(sourceArray).Data; + ref byte dst = ref Unsafe.As(destinationArray).Data; + + if (pEEType->HasGCPointers) + 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) + { + EEType* pEEType = sourceArray.EEType; + if (pEEType == destinationArray.EEType && + pEEType->IsSzArray && + length >= 0 && sourceIndex >= 0 && destinationIndex >= 0 && + (uint)(sourceIndex + length) <= (nuint)sourceArray.LongLength && + (uint)(destinationIndex + length) <= (nuint)destinationArray.LongLength) + { + nuint elementSize = (nuint)pEEType->ComponentSize; + nuint byteCount = (uint)length * elementSize; + ref byte src = ref Unsafe.AddByteOffset(ref Unsafe.As(sourceArray).Data, (uint)sourceIndex * elementSize); + ref byte dst = ref Unsafe.AddByteOffset(ref Unsafe.As(destinationArray).Data, (uint)destinationIndex * elementSize); + + if (pEEType->HasGCPointers) + 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(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(); + EEType* pEEType = array.EEType; + if (!pEEType->IsSzArray) + { + int rank = pEEType->ArrayRank; + lowerBound = Unsafe.Add(ref Unsafe.As(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 = pEEType->ComponentSize; + + ref byte ptr = ref Unsafe.AddByteOffset(ref p, (uint)offset * elementSize); + nuint byteLength = (uint)length * elementSize; + + if (pEEType->HasGCPointers) + SpanHelpers.ClearWithReferences(ref Unsafe.As(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) diff --git a/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Buffer.CoreRT.cs b/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Buffer.CoreRT.cs index 93ee9d1aba7c..485ee740e152 100644 --- a/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Buffer.CoreRT.cs +++ b/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Buffer.CoreRT.cs @@ -42,9 +42,9 @@ internal static unsafe void Memmove(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(ref destination), + ref Unsafe.As(ref source), elementCount * (nuint)Unsafe.SizeOf()); } else diff --git a/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs b/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs index bf4d1f3b89e4..806cee560c9b 100644 --- a/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Runtime/RuntimeImports.cs +++ b/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); From 5af9508ae13cd9e4f0de143707e0605b4825babf Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Thu, 15 Oct 2020 00:10:38 -0700 Subject: [PATCH 2/2] Use EETypePtr --- .../src/System/Array.CoreRT.cs | 32 +++++++++---------- .../src/System/EETypePtr.cs | 13 -------- 2 files changed, 16 insertions(+), 29 deletions(-) diff --git a/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Array.CoreRT.cs b/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Array.CoreRT.cs index 5b01d0de3c41..65efddab064f 100644 --- a/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Array.CoreRT.cs +++ b/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/Array.CoreRT.cs @@ -237,24 +237,24 @@ public static void ConstrainedCopy(Array sourceArray, int sourceIndex, Array des CopyImpl(sourceArray, sourceIndex, destinationArray, destinationIndex, length, reliable: true); } - public static unsafe void Copy(Array sourceArray, Array destinationArray, int length) + public static void Copy(Array sourceArray, Array destinationArray, int length) { if (sourceArray is null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.sourceArray); if (destinationArray is null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.destinationArray); - EEType* pEEType = sourceArray.EEType; - if (pEEType == destinationArray.EEType && - pEEType->IsSzArray && + 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)pEEType->ComponentSize; + nuint byteCount = (uint)length * (nuint)eeType.ComponentSize; ref byte src = ref Unsafe.As(sourceArray).Data; ref byte dst = ref Unsafe.As(destinationArray).Data; - if (pEEType->HasGCPointers) + if (eeType.HasPointers) Buffer.BulkMoveWithWriteBarrier(ref dst, ref src, byteCount); else Buffer.Memmove(ref dst, ref src, byteCount); @@ -271,19 +271,19 @@ public static unsafe void Copy(Array sourceArray, int sourceIndex, Array destina { if (sourceArray != null && destinationArray != null) { - EEType* pEEType = sourceArray.EEType; - if (pEEType == destinationArray.EEType && - pEEType->IsSzArray && + 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)pEEType->ComponentSize; + nuint elementSize = (nuint)eeType.ComponentSize; nuint byteCount = (uint)length * elementSize; ref byte src = ref Unsafe.AddByteOffset(ref Unsafe.As(sourceArray).Data, (uint)sourceIndex * elementSize); ref byte dst = ref Unsafe.AddByteOffset(ref Unsafe.As(destinationArray).Data, (uint)destinationIndex * elementSize); - if (pEEType->HasGCPointers) + if (eeType.HasPointers) Buffer.BulkMoveWithWriteBarrier(ref dst, ref src, byteCount); else Buffer.Memmove(ref dst, ref src, byteCount); @@ -888,10 +888,10 @@ public static unsafe void Clear(Array array, int index, int length) ref byte p = ref Unsafe.As(array).Data; int lowerBound = 0; - EEType* pEEType = array.EEType; - if (!pEEType->IsSzArray) + EETypePtr eeType = array.EETypePtr; + if (!eeType.IsSzArray) { - int rank = pEEType->ArrayRank; + int rank = eeType.ArrayRank; lowerBound = Unsafe.Add(ref Unsafe.As(ref p), rank); p = ref Unsafe.Add(ref p, 2 * sizeof(int) * rank); // skip the bounds } @@ -901,12 +901,12 @@ public static unsafe void Clear(Array array, int index, int length) if (index < lowerBound || offset < 0 || length < 0 || (uint)(offset + length) > (nuint)array.LongLength) ThrowHelper.ThrowIndexOutOfRangeException(); - nuint elementSize = pEEType->ComponentSize; + nuint elementSize = eeType.ComponentSize; ref byte ptr = ref Unsafe.AddByteOffset(ref p, (uint)offset * elementSize); nuint byteLength = (uint)length * elementSize; - if (pEEType->HasGCPointers) + if (eeType.HasPointers) SpanHelpers.ClearWithReferences(ref Unsafe.As(ref ptr), byteLength / (uint)sizeof(IntPtr)); else SpanHelpers.ClearWithoutReferences(ref ptr, byteLength); diff --git a/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/EETypePtr.cs b/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/EETypePtr.cs index 0890b8f93b12..d7b652cf0470 100644 --- a/src/coreclr/src/nativeaot/System.Private.CoreLib/src/System/EETypePtr.cs +++ b/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 {