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 MemoryMarshal.GetArrayDataReference to a JIT intrinsic #79760

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
521aa0e
Convert MemoryMarshal.GetArrayDataReference to a JIT intrinsic
MichalPetryka Jul 23, 2022
a98f01a
Update importer.cpp
MichalPetryka Jul 23, 2022
832180d
Update importer.cpp
MichalPetryka Jul 23, 2022
aaa574c
Update importer.cpp
MichalPetryka Jul 23, 2022
b831cd3
Update MemoryMarshalGetArrayDataReference.cs
MichalPetryka Jul 23, 2022
ea73036
Update MemoryMarshalGetArrayDataReference.cs
MichalPetryka Jul 23, 2022
4e2fb81
Fix nullchecks, improve tests, remove dead code
MichalPetryka Jul 24, 2022
b42e81a
Update project files
MichalPetryka Jul 24, 2022
77603b8
Update MemoryMarshalGetArrayDataReference.cs
MichalPetryka Jul 24, 2022
d9f07e6
Fix cloning of bounds-check-less INDEX_ADDRs
SingleAccretion Jul 24, 2022
2f3ccde
Fix formatting
MichalPetryka Jul 24, 2022
d476b0d
Remove COMMA
MichalPetryka Jul 25, 2022
83bff3f
Move the nullcheck insertion to morph
MichalPetryka Jul 25, 2022
f35d9dd
Fix compilation
MichalPetryka Jul 25, 2022
3944c63
Revert morph changes
MichalPetryka Jul 26, 2022
2f65de6
Try a hack to see if the diffs are better
MichalPetryka Jul 26, 2022
eb7614f
Revert "Try a hack to see if the diffs are better"
MichalPetryka Jul 26, 2022
083b88f
Add more tests
MichalPetryka Jul 26, 2022
d36e428
Future-proof against delegate inlining
MichalPetryka Jul 26, 2022
2249ab8
Redo changes, add test
MichalPetryka Dec 16, 2022
be3ad65
Revert merge issue
MichalPetryka Dec 16, 2022
7adb363
Reorganize tests
MichalPetryka Dec 16, 2022
5d17712
Update MemoryMarshalGetArrayDataReference.cs
MichalPetryka Dec 16, 2022
aade930
Update MemoryMarshalGetArrayDataReference.cs
MichalPetryka Dec 16, 2022
4764152
Test
MichalPetryka Dec 16, 2022
bfbf746
Update MemoryMarshalGetArrayDataReference.cs
MichalPetryka Dec 17, 2022
cf597a8
Update importercalls.cpp
MichalPetryka Dec 17, 2022
110479d
Update compiler.hpp
MichalPetryka Dec 17, 2022
3ac6969
Update compiler.hpp
MichalPetryka Dec 17, 2022
4157675
Update importercalls.cpp
MichalPetryka Dec 17, 2022
359e41a
Add an assert
MichalPetryka Jan 5, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ public static unsafe partial class MemoryMarshal
/// </remarks>
[Intrinsic]
[NonVersionable]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static ref T GetArrayDataReference<T>(T[] array) =>
ref Unsafe.As<byte, T>(ref Unsafe.As<RawArrayData>(array).Data);
ref GetArrayDataReference(array);

/// <summary>
/// Returns a reference to the 0th element of <paramref name="array"/>. If the array is empty, returns a reference to where the 0th element
Expand Down
13 changes: 10 additions & 3 deletions src/coreclr/jit/compiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -918,7 +918,7 @@ inline GenTreeIntCon* Compiler::gtNewIconHandleNode(size_t value, GenTreeFlags f
#if defined(LATE_DISASM)
node = new (this, LargeOpOpcode()) GenTreeIntCon(TYP_I_IMPL, value, fields DEBUGARG(/*largeNode*/ true));
#else
node = new (this, GT_CNS_INT) GenTreeIntCon(TYP_I_IMPL, value, fields);
node = new (this, GT_CNS_INT) GenTreeIntCon(TYP_I_IMPL, value, fields);
#endif
node->gtFlags |= flags;
return node;
Expand Down Expand Up @@ -1099,8 +1099,15 @@ inline GenTreeIndexAddr* Compiler::gtNewIndexAddr(GenTree* arrayOp,
unsigned elemSize =
(elemType == TYP_STRUCT) ? info.compCompHnd->getClassSize(elemClassHandle) : genTypeSize(elemType);

GenTreeIndexAddr* indexAddr = new (this, GT_INDEX_ADDR)
GenTreeIndexAddr(arrayOp, indexOp, elemType, elemClassHandle, elemSize, lengthOffset, firstElemOffset);
#ifdef DEBUG
bool boundsCheck = JitConfig.JitSkipArrayBoundCheck() != 1;
#else
bool boundsCheck = true;
#endif

GenTreeIndexAddr* indexAddr =
new (this, GT_INDEX_ADDR) GenTreeIndexAddr(arrayOp, indexOp, elemType, elemClassHandle, elemSize, lengthOffset,
firstElemOffset, boundsCheck);

return indexAddr;
}
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8580,7 +8580,7 @@ GenTree* Compiler::gtCloneExpr(
copy = new (this, GT_INDEX_ADDR)
GenTreeIndexAddr(asIndAddr->Arr(), asIndAddr->Index(), asIndAddr->gtElemType,
asIndAddr->gtStructElemClass, asIndAddr->gtElemSize, asIndAddr->gtLenOffset,
asIndAddr->gtElemOffset);
asIndAddr->gtElemOffset, asIndAddr->IsBoundsChecked());
copy->AsIndexAddr()->gtIndRngFailBB = asIndAddr->gtIndRngFailBB;
}
break;
Expand Down
12 changes: 4 additions & 8 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -6530,7 +6530,8 @@ struct GenTreeIndexAddr : public GenTreeOp
CORINFO_CLASS_HANDLE structElemClass,
unsigned elemSize,
unsigned lenOffset,
unsigned elemOffset)
unsigned elemOffset,
bool boundsCheck)
: GenTreeOp(GT_INDEX_ADDR, TYP_BYREF, arr, ind)
, gtStructElemClass(structElemClass)
, gtIndRngFailBB(nullptr)
Expand All @@ -6540,13 +6541,8 @@ struct GenTreeIndexAddr : public GenTreeOp
, gtElemOffset(elemOffset)
{
assert(!varTypeIsStruct(elemType) || (structElemClass != NO_CLASS_HANDLE));
#ifdef DEBUG
if (JitConfig.JitSkipArrayBoundCheck() == 1)
{
// Skip bounds check
}
else
#endif

if (boundsCheck)
{
// Do bounds check
gtFlags |= GTF_INX_RNGCHK;
Expand Down
38 changes: 38 additions & 0 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2601,6 +2601,34 @@ GenTree* Compiler::impIntrinsic(GenTree* newobjThis,
break;
}

case NI_System_Runtime_InteropService_MemoryMarshal_GetArrayDataReference:
{
assert(sig->numArgs == 1);
assert(sig->sigInst.methInstCount == 1);

GenTree* array = impPopStack().val;
CORINFO_CLASS_HANDLE elemHnd = sig->sigInst.methInst[0];
MichalPetryka marked this conversation as resolved.
Show resolved Hide resolved
CorInfoType jitType = info.compCompHnd->asCorInfoType(elemHnd);
var_types elemType = JITtype2varType(jitType);

if (fgAddrCouldBeNull(array))
MichalPetryka marked this conversation as resolved.
Show resolved Hide resolved
{
GenTree* arrayClone;
array = impCloneExpr(array, &arrayClone, NO_CLASS_HANDLE, (unsigned)CHECK_SPILL_ALL,
nullptr DEBUGARG("MemoryMarshal.GetArrayDataReference array"));

impAppendTree(gtNewNullCheck(array, compCurBB), (unsigned)CHECK_SPILL_ALL, impCurStmtDI);
array = arrayClone;
}

GenTree* index = gtNewIconNode(0, TYP_I_IMPL);
GenTreeIndexAddr* indexAddr = gtNewArrayIndexAddr(array, index, elemType, elemHnd);
indexAddr->gtFlags &= ~GTF_INX_RNGCHK;
indexAddr->gtFlags |= GTF_INX_ADDR_NONNULL;
retNode = indexAddr;
break;
}

case NI_Internal_Runtime_MethodTable_Of:
case NI_System_Activator_AllocatorOf:
case NI_System_Activator_DefaultConstructorOf:
Expand Down Expand Up @@ -7514,6 +7542,16 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
}
}
}
else if (strcmp(namespaceName, "System.Runtime.InteropServices") == 0)
{
if (strcmp(className, "MemoryMarshal") == 0)
{
if (strcmp(methodName, "GetArrayDataReference") == 0)
{
result = NI_System_Runtime_InteropService_MemoryMarshal_GetArrayDataReference;
}
}
}
else if (strncmp(namespaceName, "System.Runtime.Intrinsics", 25) == 0)
{
// We go down this path even when FEATURE_HW_INTRINSICS isn't enabled
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/namedintrinsiclist.h
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ enum NamedIntrinsic : unsigned short
NI_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray,
NI_System_Runtime_CompilerServices_RuntimeHelpers_IsKnownConstant,

NI_System_Runtime_InteropService_MemoryMarshal_GetArrayDataReference,

NI_System_String_Equals,
NI_System_String_get_Chars,
NI_System_String_get_Length,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,8 @@ public static unsafe partial class MemoryMarshal
/// </remarks>
[Intrinsic]
[NonVersionable]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static ref T GetArrayDataReference<T>(T[] array) =>
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
ref Unsafe.As<byte, T>(ref Unsafe.As<RawArrayData>(array).Data);
ref GetArrayDataReference(array);

/// <summary>
/// Returns a reference to the 0th element of <paramref name="array"/>. If the array is empty, returns a reference to where the 0th element
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,6 @@ private static MethodIL TryGetIntrinsicMethodIL(MethodDesc method)
return UnsafeIntrinsics.EmitIL(method);
}
break;
case "MemoryMarshal":
{
if (owningType.Namespace == "System.Runtime.InteropServices")
return MemoryMarshalIntrinsics.EmitIL(method);
}
break;
case "Volatile":
{
if (owningType.Namespace == "System.Threading")
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -655,9 +655,6 @@
<Compile Include="..\..\Common\TypeSystem\IL\Stubs\UnsafeIntrinsics.cs">
<Link>IL\Stubs\UnsafeIntrinsics.cs</Link>
</Compile>
<Compile Include="..\..\Common\TypeSystem\IL\Stubs\MemoryMarshalIntrinsics.cs">
<Link>IL\Stubs\MemoryMarshalIntrinsics.cs</Link>
</Compile>
<Compile Include="..\..\Common\TypeSystem\IL\Stubs\VolatileIntrinsics.cs">
<Link>IL\Stubs\VolatileIntrinsics.cs</Link>
</Compile>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,6 @@ private MethodIL TryGetIntrinsicMethodIL(MethodDesc method)
return UnsafeIntrinsics.EmitIL(method);
}

if (mdType.Name == "MemoryMarshal" && mdType.Namespace == "System.Runtime.InteropServices")
{
return MemoryMarshalIntrinsics.EmitIL(method);
}

if (mdType.Name == "Volatile" && mdType.Namespace == "System.Threading")
{
return VolatileIntrinsics.EmitIL(method);
Expand Down Expand Up @@ -171,7 +166,7 @@ public override MethodIL GetMethodIL(MethodDesc method)
}

// Check to see if there is an override for the EcmaMethodIL. If there is not
// then simply return the EcmaMethodIL. In theory this could call
// then simply return the EcmaMethodIL. In theory this could call
// CreateCrossModuleInlineableTokensForILBody, but we explicitly do not want
// to do that. The reason is that this method is called during the multithreaded
// portion of compilation, and CreateCrossModuleInlineableTokensForILBody
Expand Down Expand Up @@ -224,7 +219,7 @@ class ManifestModuleWrappedMethodIL : MethodIL, IEcmaMethodIL, IMethodTokensAreU
MutableModule _mutableModule;

public ManifestModuleWrappedMethodIL() {}

public bool Initialize(MutableModule mutableModule, EcmaMethodIL wrappedMethod)
{
bool failedToReplaceToken = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
<Compile Include="..\..\Common\TypeSystem\IL\ILReader.cs" Link="IL\ILReader.cs" />
<Compile Include="..\..\Common\TypeSystem\IL\Stubs\ComparerIntrinsics.cs" Link="IL\Stubs\ComparerIntrinsics.cs" />
<Compile Include="..\..\Common\TypeSystem\IL\Stubs\InterlockedIntrinsics.cs" Link="IL\Stubs\InterlockedIntrinsics.cs" />
<Compile Include="..\..\Common\TypeSystem\IL\Stubs\MemoryMarshalIntrinsics.cs" Link="IL\Stubs\MemoryMarshalIntrinsics.cs" />
<Compile Include="..\..\Common\TypeSystem\IL\Stubs\RuntimeHelpersIntrinsics.cs" Link="IL\Stubs\RuntimeHelpersIntrinsics.cs" />
<Compile Include="..\..\Common\TypeSystem\IL\Stubs\UnsafeIntrinsics.cs" Link="IL\Stubs\UnsafeIntrinsics.cs" />
<Compile Include="..\..\Common\TypeSystem\IL\Stubs\VolatileIntrinsics.cs" Link="IL\Stubs\VolatileIntrinsics.cs" />
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,6 @@ DEFINE_METHOD(UNSAFE, UNBOX, Unbox, NoSig)
DEFINE_METHOD(UNSAFE, WRITE, Write, NoSig)

DEFINE_CLASS(MEMORY_MARSHAL, Interop, MemoryMarshal)
DEFINE_METHOD(MEMORY_MARSHAL, GET_ARRAY_DATA_REFERENCE_SZARRAY, GetArrayDataReference, GM_ArrT_RetRefT)
DEFINE_METHOD(MEMORY_MARSHAL, GET_ARRAY_DATA_REFERENCE_MDARRAY, GetArrayDataReference, SM_Array_RetRefByte)

DEFINE_CLASS(INTERLOCKED, Threading, Interlocked)
Expand Down
37 changes: 0 additions & 37 deletions src/coreclr/vm/jitinterface.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7001,39 +7001,6 @@ bool getILIntrinsicImplementationForUnsafe(MethodDesc * ftn,
return false;
}

bool getILIntrinsicImplementationForMemoryMarshal(MethodDesc * ftn,
CORINFO_METHOD_INFO * methInfo)
{
STANDARD_VM_CONTRACT;

_ASSERTE(CoreLibBinder::IsClass(ftn->GetMethodTable(), CLASS__MEMORY_MARSHAL));

mdMethodDef tk = ftn->GetMemberDef();

if (tk == CoreLibBinder::GetMethod(METHOD__MEMORY_MARSHAL__GET_ARRAY_DATA_REFERENCE_SZARRAY)->GetMemberDef())
{
mdToken tokRawSzArrayData = CoreLibBinder::GetField(FIELD__RAW_ARRAY_DATA__DATA)->GetMemberDef();

static BYTE ilcode[] = { CEE_LDARG_0,
CEE_LDFLDA,0,0,0,0,
CEE_RET };

ilcode[2] = (BYTE)(tokRawSzArrayData);
ilcode[3] = (BYTE)(tokRawSzArrayData >> 8);
ilcode[4] = (BYTE)(tokRawSzArrayData >> 16);
ilcode[5] = (BYTE)(tokRawSzArrayData >> 24);

methInfo->ILCode = const_cast<BYTE*>(ilcode);
methInfo->ILCodeSize = sizeof(ilcode);
methInfo->maxStack = 1;
methInfo->EHcount = 0;
methInfo->options = (CorInfoOptions)0;
return true;
}

return false;
}

bool getILIntrinsicImplementationForVolatile(MethodDesc * ftn,
CORINFO_METHOD_INFO * methInfo)
{
Expand Down Expand Up @@ -7485,10 +7452,6 @@ getMethodInfoHelper(
{
fILIntrinsic = getILIntrinsicImplementationForUnsafe(ftn, methInfo);
}
else if (CoreLibBinder::IsClass(pMT, CLASS__MEMORY_MARSHAL))
{
fILIntrinsic = getILIntrinsicImplementationForMemoryMarshal(ftn, methInfo);
}
else if (CoreLibBinder::IsClass(pMT, CLASS__INTERLOCKED))
{
fILIntrinsic = getILIntrinsicImplementationForInterlocked(ftn, methInfo);
Expand Down
Loading