Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert some more array FCalls to managed #102739

Merged
merged 53 commits into from
Jun 27, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
566b15e
Implement IsSimpleCopy and CanAssignArrayType in managed code
huoyaoyuan May 27, 2024
8a592b9
Fold IsSimpleCopy back to CanAssignArrayType
huoyaoyuan May 27, 2024
0dce3d4
Delete FCall and QCall definitions for copy
huoyaoyuan May 27, 2024
d29e5d8
Convert InternalSetValue to managed
huoyaoyuan May 28, 2024
5dd0e3c
Setup FCalls
huoyaoyuan May 28, 2024
527021c
Remove FCall for GetCorElementTypeOfElementType
huoyaoyuan May 28, 2024
e7d843a
Complete TryUnBox
huoyaoyuan May 28, 2024
2bb5fe7
Fix FCall definition
huoyaoyuan May 28, 2024
f7da172
Implement InitializeArray in managed
huoyaoyuan May 28, 2024
0924555
Implement GetSpanDataFrom in managed
huoyaoyuan May 28, 2024
dc85381
Remove FCall definition
huoyaoyuan May 28, 2024
f99c553
Fix RVA field address
huoyaoyuan May 28, 2024
c5e6cc4
Fix RVA assert
huoyaoyuan May 28, 2024
470b86f
Do not use hydrated RtFieldInfo
huoyaoyuan May 28, 2024
c7821e6
Use QCall for LoadSize
huoyaoyuan May 29, 2024
5225203
Fix CanAssignArrayType condition
huoyaoyuan May 29, 2024
07293e3
Fix compilation
huoyaoyuan May 29, 2024
5da88c3
Simplify AssignType enum
huoyaoyuan May 29, 2024
5222bca
Fix I and U in CanPrimitiveWiden
huoyaoyuan May 29, 2024
335b2fb
CanCastTo should be QCall
huoyaoyuan May 30, 2024
93ec9d8
Remove ThrowHelper usages that not in hot path
huoyaoyuan May 31, 2024
c3f5097
Merge branch 'main' into array-fcall
huoyaoyuan Jun 6, 2024
481e4d4
Revert the known broken InternalSetValue
huoyaoyuan Jun 6, 2024
a266a30
Revert "Revert the known broken InternalSetValue"
huoyaoyuan Jun 6, 2024
b8e4584
Fix heap overwrite
huoyaoyuan Jun 7, 2024
5465a97
Add disabled test for RVA field reflection
huoyaoyuan Jun 9, 2024
4ce4f51
Push back changes around IsSimpleCopy and CanAssignArrayType
huoyaoyuan Jun 10, 2024
30b7d8f
Move PrimitiveWiden to InvokeUtils
huoyaoyuan Jun 12, 2024
8a9c5bc
Merge coreclr specific InvokeUtils
huoyaoyuan Jun 12, 2024
f95fa34
Revert "Merge coreclr specific InvokeUtils"
huoyaoyuan Jun 13, 2024
ebfd224
Move InvokeUtils to shared
huoyaoyuan Jun 13, 2024
d9205c6
Apply suggestions from code review
huoyaoyuan Jun 18, 2024
7eef724
Merge branch 'main' into array-fcall
huoyaoyuan Jun 18, 2024
8cfb58f
Fix trailing whitespace
huoyaoyuan Jun 18, 2024
fbb9d71
Reduce QCalls
huoyaoyuan Jun 18, 2024
324df76
Reduce CorElementType overhead
huoyaoyuan Jun 18, 2024
089ae12
Separate RVA reflection change out
huoyaoyuan Jun 19, 2024
a5e7441
Revert GetCorElementTypeOfElementType
huoyaoyuan Jun 19, 2024
5552d63
Change the MethodTable FCall to be optimal for primitive and share wi…
huoyaoyuan Jun 19, 2024
133cd8a
Return ref from GetSpanDataFrom
huoyaoyuan Jun 19, 2024
e17bf0e
Handle enum conversion to underlying type
huoyaoyuan Jun 19, 2024
dd1e2dd
Update exception message
huoyaoyuan Jun 19, 2024
32b8789
Add KeepAlive
huoyaoyuan Jun 20, 2024
582eaf1
Remove unnecessary qcall handle
huoyaoyuan Jun 20, 2024
1836ae5
Fix ref src
huoyaoyuan Jun 20, 2024
cb6aa00
Update FCall name and assert
huoyaoyuan Jun 24, 2024
49cf866
Remove unused changes
huoyaoyuan Jun 24, 2024
7a0c398
Use void* for address
huoyaoyuan Jun 24, 2024
db677ad
Apply suggestions from code review
huoyaoyuan Jun 25, 2024
353715e
Remove unused enum FCall
huoyaoyuan Jun 24, 2024
8fb1c96
Use sizeof
huoyaoyuan Jun 25, 2024
26d4c80
Add some comment
huoyaoyuan Jun 25, 2024
5f7263f
Add comment to lifetime
huoyaoyuan Jun 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
491 changes: 334 additions & 157 deletions src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ internal static unsafe class CastHelpers
// Unlike the IsInstanceOfInterface and IsInstanceOfClass functions,
// this test must deal with all kinds of type tests
[DebuggerHidden]
private static object? IsInstanceOfAny(void* toTypeHnd, object? obj)
internal static object? IsInstanceOfAny(void* toTypeHnd, object? obj)
{
if (obj != null)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,28 +1,115 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Buffers.Binary;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.Serialization;
using System.Runtime.Versioning;
using System.Threading;

namespace System.Runtime.CompilerServices
{
public static partial class RuntimeHelpers
{
[Intrinsic]
[MethodImpl(MethodImplOptions.InternalCall)]
public static extern void InitializeArray(Array array, RuntimeFieldHandle fldHandle);
public static unsafe void InitializeArray(Array array, RuntimeFieldHandle fldHandle)
{
RtFieldInfo fldInfo = (RtFieldInfo)FieldInfo.GetFieldFromHandle(fldHandle);
huoyaoyuan marked this conversation as resolved.
Show resolved Hide resolved

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern unsafe void* GetSpanDataFrom(
if (array is null)
ThrowHelper.ThrowArgumentNullException(ExceptionArgument.array);

if ((fldInfo.Attributes & FieldAttributes.HasFieldRVA) == 0)
ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_BadFieldForInitializeArray);

// Note that we do not check that the field is actually in the PE file that is initializing
// the array. Basically the data being published is can be accessed by anyone with the proper
huoyaoyuan marked this conversation as resolved.
Show resolved Hide resolved
// permissions (C# marks these as assembly visibility, and thus are protected from outside
// snooping)

if (!array.GetCorElementTypeOfElementType().IsPrimitiveType()) // Enum is included
ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_MustBePrimitiveArray);

MethodTable* pMT = GetMethodTable(array);
nuint totalSize = pMT->ComponentSize * array.NativeLength;

uint size = ((MethodTable*)((RuntimeType)fldInfo.FieldType).TypeHandle.Value)->GetNumInstanceFieldBytes();

// make certain you don't go off the end of the rva static
if (totalSize > size)
ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_BadFieldForInitializeArray);

void* src = (void*)RuntimeFieldHandle.GetStaticFieldAddress(fldInfo);
jkotas marked this conversation as resolved.
Show resolved Hide resolved
ref byte dst = ref MemoryMarshal.GetArrayDataReference(array);

Debug.Assert(!pMT->GetArrayElementTypeHandle().AsMethodTable()->ContainsGCPointers);

if (BitConverter.IsLittleEndian)
{
SpanHelpers.Memmove(ref dst, ref *(byte*)src, totalSize);
}
else
{
switch (pMT->ComponentSize)
{
case 1:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use sizeof(byte) and corresponding primitives for this switch statement?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using such pattern in CoreLib. Sounds reasonable.

SpanHelpers.Memmove(ref dst, ref *(byte*)src, totalSize);
break;
case 2:
BinaryPrimitives.ReverseEndianness(
new ReadOnlySpan<ushort>(src, array.Length),
new Span<ushort>(ref Unsafe.As<byte, ushort>(ref dst), array.Length));
break;
case 4:
BinaryPrimitives.ReverseEndianness(
new ReadOnlySpan<uint>(src, array.Length),
new Span<uint>(ref Unsafe.As<byte, uint>(ref dst), array.Length));
break;
case 8:
BinaryPrimitives.ReverseEndianness(
new ReadOnlySpan<ulong>(src, array.Length),
new Span<ulong>(ref Unsafe.As<byte, ulong>(ref dst), array.Length));
break;
default:
Debug.Fail("Incorrect primitive type size!");
break;
}
}
}

private static unsafe void* GetSpanDataFrom(
huoyaoyuan marked this conversation as resolved.
Show resolved Hide resolved
RuntimeFieldHandle fldHandle,
RuntimeTypeHandle targetTypeHandle,
out int count);
out int count)
{
RtFieldInfo fldInfo = (RtFieldInfo)FieldInfo.GetFieldFromHandle(fldHandle);

if ((fldInfo.Attributes & FieldAttributes.HasFieldRVA) == 0)
ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_BadFieldForInitializeArray);

TypeHandle th = new TypeHandle((void*)targetTypeHandle.Value);
Debug.Assert(!th.IsTypeDesc); // TypeDesc can't be used as generic parameter

if (!th.GetVerifierCorElementType().IsPrimitiveType()) // Enum is included
huoyaoyuan marked this conversation as resolved.
Show resolved Hide resolved
ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_MustBePrimitiveArray);

uint totalSize = ((MethodTable*)((RuntimeType)fldInfo.FieldType).TypeHandle.Value)->GetNumInstanceFieldBytes();
uint targetTypeSize = th.AsMethodTable()->GetNumInstanceFieldBytes();

IntPtr data = RuntimeFieldHandle.GetStaticFieldAddress(fldInfo);
if (data % targetTypeSize != 0)
ThrowHelper.ThrowArgumentException(ExceptionResource.Argument_BadFieldForInitializeArray);

if (!BitConverter.IsLittleEndian)
{
throw new PlatformNotSupportedException();
}

count = (int)(totalSize / targetTypeSize);
return (void*)data;
huoyaoyuan marked this conversation as resolved.
Show resolved Hide resolved
}

// GetObjectValue is intended to allow value classes to be manipulated as 'Object'
// but have aliasing behavior of a value class. The intent is that you would use
Expand Down Expand Up @@ -686,6 +773,8 @@ public int MultiDimensionalArrayRank
// Warning! UNLIKE the similarly named Reflection api, this method also returns "true" for Enums.
public bool IsPrimitive => (Flags & enum_flag_Category_Mask) is enum_flag_Category_PrimitiveValueType or enum_flag_Category_TruePrimitive;

public bool IsTruePrimitive => (Flags & enum_flag_Category_Mask) is enum_flag_Category_TruePrimitive;

public bool HasInstantiation => (Flags & enum_flag_HasComponentSize) == 0 && (Flags & enum_flag_GenericsMask) != enum_flag_GenericsMask_NonGeneric;

public bool IsGenericTypeDefinition => (Flags & (enum_flag_HasComponentSize | enum_flag_GenericsMask)) == enum_flag_GenericsMask_TypicalInst;
Expand Down Expand Up @@ -715,6 +804,13 @@ public TypeHandle GetArrayElementTypeHandle()

[MethodImpl(MethodImplOptions.InternalCall)]
public extern uint GetNumInstanceFieldBytes();

[MethodImpl(MethodImplOptions.InternalCall)]
public extern CorElementType GetVerifierCorElementType();

[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern bool CanCastTo_NoCacheLookup(void* fromTypeHnd, void* toTypeHnd);

}

// Subset of src\vm\methodtable.h
Expand All @@ -740,9 +836,9 @@ public bool CanCompareBitsOrUseFastGetHashCode
}

/// <summary>
/// A type handle, which can wrap either a pointer to a <c>TypeDesc</c> or to a <see cref="MethodTable"/>.
/// A type handle, which can wrap either a pointer to a <see cref="TypeDesc"/> or to a <see cref="MethodTable"/>.
/// </summary>
internal unsafe struct TypeHandle
internal readonly unsafe struct TypeHandle
{
// Subset of src\vm\typehandle.h

Expand Down Expand Up @@ -784,11 +880,63 @@ public bool IsTypeDesc
return (MethodTable*)m_asTAddr;
}

/// <summary>
/// Gets the <see cref="TypeDesc"/> pointer wrapped by the current instance.
/// </summary>
/// <remarks>This is only safe to call if <see cref="IsTypeDesc"/> returned <see langword="true"/>.</remarks>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public TypeDesc* AsTypeDesc()
{
Debug.Assert(IsTypeDesc);

return (TypeDesc*)((nint)m_asTAddr - 2);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static TypeHandle TypeHandleOf<T>()
{
return new TypeHandle((void*)RuntimeTypeHandle.ToIntPtr(typeof(T).TypeHandle));
}

public static bool AreSameType(TypeHandle left, TypeHandle right) => left.m_asTAddr == right.m_asTAddr;
jkotas marked this conversation as resolved.
Show resolved Hide resolved

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool CanCastTo(TypeHandle destTH)
{
CastResult result = CastCache.TryGet(CastHelpers.s_table!, (nuint)m_asTAddr, (nuint)destTH.m_asTAddr);

if (result != CastResult.MaybeCast)
return result == CastResult.CanCast;

return MethodTable.CanCastTo_NoCacheLookup(m_asTAddr, destTH.m_asTAddr);
}

public bool IsValueType
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
get
{
return IsTypeDesc
? AsTypeDesc()->GetInternalCorElementType() == CorElementType.ELEMENT_TYPE_VALUETYPE
: AsMethodTable()->IsValueType;
}
}

public bool IsInterface => !IsTypeDesc && AsMethodTable()->IsInterface;

public CorElementType GetVerifierCorElementType() => IsTypeDesc
? AsTypeDesc()->GetInternalCorElementType()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since MethodTable::GetVerifierCorElementType is FCall, there is not much value in duplicating TypeDesc::GetVerifierCorElementType() in C#. I would make FCall for the whole TypeHandle::GetVerifierCorElementType instead.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetVerifierCorElementType() seems unused on managed side. Maybe just delete this FCall and keep implementation on native side (for jitinterface, arraynative etc.)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's used for deciding primitive widen. Some other usages were refactored away, but GetCorElementType is using it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah nvm, I was searching on main branch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the remaining CorElementType usages are about primitive widen and fast path of int[] <-> uint[] conversion. It's only used for primitive-like types. In these cases, GetVerifierCorElementType has unnecessary overhead, and Enum has defined a custom FCall.

It think the GetVerifierCorElementType FCall should be removed and share a new FCall for primitive-like types with Enum. Is it correct that a TypeHandle can only be primitive when it's MethodTable and the primitive flag is set? Are (U)IntPtr always represented as MethodTable with ELEMENT_TYPE_I/U?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it correct that a TypeHandle can only be primitive when it's MethodTable and the primitive flag is set? Are (U)IntPtr always represented as MethodTable with ELEMENT_TYPE_I/U?

Yes, that's correct.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well all of the usages of GetCorElementTypeOfElementType are in fact about primitive types. It can be refactored later.

: AsMethodTable()->GetVerifierCorElementType();
}

internal unsafe struct TypeDesc
{
private readonly int m_typeAndFlags;

// This is the ELEMENT_TYPE* that would be used in the type sig for this type
// For enums this is the underlying type
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public CorElementType GetInternalCorElementType() => (CorElementType)(m_typeAndFlags & 0xFF);
}

// Helper structs used for tail calls via helper.
Expand Down
Loading
Loading