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

On 64 bit platforms, "stelem.ref" and "ldelema" ignore the high bits of a native int index #71571

Merged
merged 6 commits into from
Jul 6, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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 @@ -596,6 +596,56 @@ private static void StelemRef(Array array, int index, object? obj)
StelemRef_Helper(ref element, elementType, obj);
}

#if TARGET_64BIT
[DebuggerHidden]
[StackTraceHidden]
[DebuggerStepThrough]
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
private static ref object? LdelemaRefNint(Array array, nint index, void* type)
{
// this will throw appropriate exceptions if array is null or access is out of range.
ref object? element = ref Unsafe.As<ArrayElement[]>(array)[index].Value;
void* elementType = RuntimeHelpers.GetMethodTable(array)->ElementType;

if (elementType == type)
return ref element;

return ref ThrowArrayMismatchException();
}

[DebuggerHidden]
[StackTraceHidden]
[DebuggerStepThrough]
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
private static void StelemRefNint(Array array, nint index, object? obj)
{
// this will throw appropriate exceptions if array is null or access is out of range.
ref object? element = ref Unsafe.As<ArrayElement[]>(array)[index].Value;
void* elementType = RuntimeHelpers.GetMethodTable(array)->ElementType;

if (obj == null)
goto assigningNull;

if (elementType != RuntimeHelpers.GetMethodTable(obj))
goto notExactMatch;

doWrite:
WriteBarrier(ref element, obj);
return;

assigningNull:
AaronRobinsonMSFT marked this conversation as resolved.
Show resolved Hide resolved
element = null;
return;

notExactMatch:
if (array.GetType() == typeof(object[]))
goto doWrite;

StelemRef_Helper(ref element, elementType, obj);
}
#endif // TARGET_64BIT


[DebuggerHidden]
[StackTraceHidden]
[DebuggerStepThrough]
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/inc/corinfo.h
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,9 @@ enum CorInfoHelpFunc
CORINFO_HELP_ARRADDR_ST, // assign to element of object array with type-checking
CORINFO_HELP_LDELEMA_REF, // does a precise type comparision and returns address

CORINFO_HELP_ARRADDR_ST_I_IMPL, // assign to element of object array with type-checking (Native int index parameter)
CORINFO_HELP_LDELEMA_REF_I_IMPL, // does a precise type comparision and returns address (Native int index parameter)

/* Exceptions */

CORINFO_HELP_THROW, // Throw an exception object
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/inc/jiteeversionguid.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID;
#define GUID_DEFINED
#endif // !GUID_DEFINED

constexpr GUID JITEEVersionIdentifier = { /* f2faa5fc-a1ec-4244-aebb-5597bfd7153a */
0xf2faa5fc,
0xa1ec,
0x4244,
{0xae, 0xbb, 0x55, 0x97, 0xbf, 0xd7, 0x15, 0x3a}
constexpr GUID JITEEVersionIdentifier = { /* 0d853657-7a01-421f-b1b0-d22a8e691441 */
0x0d853657,
0x7a01,
0x421f,
{0xb1, 0xb0, 0xd2, 0x2a, 0x8e, 0x69, 0x14, 0x41}
};

//////////////////////////////////////////////////////////////////////////////////////////////////////////
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/inc/jithelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,9 @@
DYNAMICJITHELPER(CORINFO_HELP_ARRADDR_ST, NULL, CORINFO_HELP_SIG_4_STACK)
DYNAMICJITHELPER(CORINFO_HELP_LDELEMA_REF, NULL, CORINFO_HELP_SIG_4_STACK)

DYNAMICJITHELPER(CORINFO_HELP_ARRADDR_ST_I_IMPL, NULL, CORINFO_HELP_SIG_4_STACK)
DYNAMICJITHELPER(CORINFO_HELP_LDELEMA_REF_I_IMPL, NULL, CORINFO_HELP_SIG_4_STACK)

// Exceptions
JITHELPER(CORINFO_HELP_THROW, IL_Throw, CORINFO_HELP_SIG_REG_ONLY)
JITHELPER(CORINFO_HELP_RETHROW, IL_Rethrow, CORINFO_HELP_SIG_REG_ONLY)
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/inc/readytorun.h
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,10 @@ enum ReadyToRunHelper
READYTORUN_HELPER_StackProbe = 0x111,

READYTORUN_HELPER_GetCurrentManagedThreadId = 0x112,

// Array helpers for use with native ints
READYTORUN_HELPER_Stelem_Ref_I = 0x113,
READYTORUN_HELPER_Ldelema_Ref_I = 0x114,
};

#include "readytoruninstructionset.h"
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/inc/readytorunhelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ HELPER(READYTORUN_HELPER_ByRefWriteBarrier, CORINFO_HELP_ASSIGN_BYREF,
HELPER(READYTORUN_HELPER_Stelem_Ref, CORINFO_HELP_ARRADDR_ST, )
HELPER(READYTORUN_HELPER_Ldelema_Ref, CORINFO_HELP_LDELEMA_REF, )

HELPER(READYTORUN_HELPER_Stelem_Ref_I, CORINFO_HELP_ARRADDR_ST_I_IMPL, )
HELPER(READYTORUN_HELPER_Ldelema_Ref_I, CORINFO_HELP_LDELEMA_REF_I_IMPL, )

HELPER(READYTORUN_HELPER_MemSet, CORINFO_HELP_MEMSET, )
HELPER(READYTORUN_HELPER_MemCpy, CORINFO_HELP_MEMCPY, )

Expand Down
26 changes: 24 additions & 2 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13717,7 +13717,18 @@ void Compiler::impImportBlockCode(BasicBlock* block)
GenTree* type = op1;
GenTree* index = impPopStack().val;
GenTree* arr = impPopStack().val;
op1 = gtNewHelperCallNode(CORINFO_HELP_LDELEMA_REF, TYP_BYREF, arr, index, type);
#ifdef TARGET_64BIT
// The CLI Spec allows an array to be indexed by either an int32 or a native int. In the case that the index
// is a native int on a 64-bit platform, we will need to use a helper that can process the wider value.
if (index->TypeGet() == TYP_I_IMPL)
{
op1 = gtNewHelperCallNode(CORINFO_HELP_LDELEMA_REF_I_IMPL, TYP_BYREF, arr, index, type);
}
else
#endif // TARGET_64BIT
{
op1 = gtNewHelperCallNode(CORINFO_HELP_LDELEMA_REF, TYP_BYREF, arr, index, type);
}
}

impPushOnStack(op1, tiRetVal);
Expand Down Expand Up @@ -13859,7 +13870,18 @@ void Compiler::impImportBlockCode(BasicBlock* block)
impPopStack(3);

// Else call a helper function to do the assignment
op1 = gtNewHelperCallNode(CORINFO_HELP_ARRADDR_ST, TYP_VOID, array, index, value);
#ifdef TARGET_64BIT
// The CLI Spec allows an array to be indexed by either an int32 or a native int. In the case that the index
// is a native int on a 64-bit platform, we will need to use a helper that can process the wider value.
if (index->TypeGet() == TYP_I_IMPL)
{
op1 = gtNewHelperCallNode(CORINFO_HELP_ARRADDR_ST_I_IMPL, TYP_VOID, array, index, value);
}
else
#endif // TARGET_64BIT
{
op1 = gtNewHelperCallNode(CORINFO_HELP_ARRADDR_ST, TYP_VOID, array, index, value);
}
goto SPILL_APPEND;
}

Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/jit/morph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8426,7 +8426,11 @@ GenTree* Compiler::fgMorphCall(GenTreeCall* call)
// Morph stelem.ref helper call to store a null value, into a store into an array without the helper.
// This needs to be done after the arguments are morphed to ensure constant propagation has already taken place.
if (opts.OptimizationEnabled() && (call->gtCallType == CT_HELPER) &&
(call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_ARRADDR_ST)))
((call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_ARRADDR_ST))
#ifdef TARGET_64BIT
|| (call->gtCallMethHnd == eeFindHelper(CORINFO_HELP_ARRADDR_ST_I_IMPL))
#endif
))
{
assert(call->gtArgs.CountArgs() == 3);
GenTree* value = call->gtArgs.GetArgByIndex(2)->GetNode();
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/optimizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5509,6 +5509,7 @@ PhaseStatus Compiler::optFindLoopsPhase()
case CORINFO_HELP_ASSIGN_BYREF: // Not strictly needed as we don't make a GT_CALL with this
case CORINFO_HELP_SETFIELDOBJ:
case CORINFO_HELP_ARRADDR_ST:
case CORINFO_HELP_ARRADDR_ST_I_IMPL:

return CALLINT_REF_INDIRS;

Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1372,6 +1372,7 @@ void HelperCallProperties::init()
case CORINFO_HELP_UNBOX:
case CORINFO_HELP_GETREFANY:
case CORINFO_HELP_LDELEMA_REF:
case CORINFO_HELP_LDELEMA_REF_I_IMPL:

isPure = true;
break;
Expand Down Expand Up @@ -1443,6 +1444,7 @@ void HelperCallProperties::init()
case CORINFO_HELP_SETFIELDFLOAT:
case CORINFO_HELP_SETFIELDDOUBLE:
case CORINFO_HELP_ARRADDR_ST:
case CORINFO_HELP_ARRADDR_ST_I_IMPL:

mutatesHeap = true;
break;
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/valuenum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10110,6 +10110,7 @@ VNFunc Compiler::fgValueNumberJitHelperMethodVNFunc(CorInfoHelpFunc helpFunc)
break;

case CORINFO_HELP_LDELEMA_REF:
case CORINFO_HELP_LDELEMA_REF_I_IMPL:
vnf = VNF_LdElemA;
break;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -778,6 +778,59 @@ public static unsafe void StelemRef(Array array, int index, object obj)
InternalCalls.RhpAssignRef(ref element, obj);
return;

assigningNull:
element = null;
return;

notExactMatch:
#if INPLACE_RUNTIME
// This optimization only makes sense for inplace runtime where there's only one System.Object.
if (array.GetMethodTable() == MethodTable.Of<object[]>())
goto doWrite;
#endif

StelemRef_Helper(ref element, elementType, obj);
}

//
// Array stelem/ldelema helpers with RyuJIT conventions
//
[RuntimeExport("RhpStelemRefNint")]
public static unsafe void StelemRefNint(Array array, nint index, object obj)
{
// This is supported only on arrays
Debug.Assert(array.GetMethodTable()->IsArray, "first argument must be an array");

#if INPLACE_RUNTIME
// this will throw appropriate exceptions if array is null or access is out of range.
ref object element = ref Unsafe.As<ArrayElement[]>(array)[index].Value;
#else
if (array is null)
{
// TODO: If both array and obj are null, we're likely going to throw Redhawk's NullReferenceException.
// This should blame the caller.
throw obj.MethodTable->GetClasslibException(ExceptionIDs.NullReference);
}
if ((nuint)index >= (nuint)array.Length)
{
throw array.MethodTable->GetClasslibException(ExceptionIDs.IndexOutOfRange);
}
ref object rawData = ref Unsafe.As<byte, object>(ref Unsafe.As<RawArrayData>(array).Data);
ref object element = ref Unsafe.Add(ref rawData, index);
#endif

MethodTable* elementType = array.GetMethodTable()->RelatedParameterType;

if (obj == null)
goto assigningNull;

if (elementType != obj.GetMethodTable())
goto notExactMatch;

doWrite:
InternalCalls.RhpAssignRef(ref element, obj);
return;

assigningNull:
element = null;
return;
Expand Down Expand Up @@ -833,6 +886,26 @@ public static unsafe ref object LdelemaRef(Array array, int index, IntPtr elemen
return ref Unsafe.Add(ref rawData, index);
}

[RuntimeExport("RhpLdelemaRefNint")]
public static unsafe ref object LdelemaRefNint(Array array, nint index, IntPtr elementType)
{
Debug.Assert(array.GetMethodTable()->IsArray, "first argument must be an array");

MethodTable* elemType = (MethodTable*)elementType;
MethodTable* arrayElemType = array.GetMethodTable()->RelatedParameterType;

if (!AreTypesEquivalent(elemType, arrayElemType))
{
// Throw the array type mismatch exception defined by the classlib, using the input array's MethodTable*
// to find the correct classlib.

throw array.GetMethodTable()->GetClasslibException(ExceptionIDs.ArrayTypeMismatch);
}

ref object rawData = ref Unsafe.As<byte, object>(ref Unsafe.As<RawArrayData>(array).Data);
return ref Unsafe.Add(ref rawData, (int)index);
Copy link
Member

Choose a reason for hiding this comment

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

There is Unsafe.Add overload that takes IntPtr. It should not be necessary to cast the index to int here. It just going to add an extra unnecessary instruction.

Copy link
Member

Choose a reason for hiding this comment

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

If the problem is that the "minimal runtime" does not compile without the cast, just add the necessary method to the minimal Unsafe used by the minimal runtime.

}

internal static unsafe bool IsDerived(MethodTable* pDerivedType, MethodTable* pBaseType)
{
Debug.Assert(!pDerivedType->IsArray, "did not expect array type");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,10 @@ public enum ReadyToRunHelper

GetCurrentManagedThreadId = 0x112,

// Array helpers for use with native ints
Stelem_Ref_I = 0x113,
Ldelema_Ref_I = 0x114,

// **********************************************************************************************
//
// These are not actually part of the R2R file format. We have them here because it's convenient.
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/tools/Common/JitInterface/CorInfoHelpFunc.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ which is the right helper to use to allocate an object of a given type. */
CORINFO_HELP_ARRADDR_ST, // assign to element of object array with type-checking
CORINFO_HELP_LDELEMA_REF, // does a precise type comparision and returns address

CORINFO_HELP_ARRADDR_ST_I_IMPL, // assign to element of object array with type-checking (with native int index)
CORINFO_HELP_LDELEMA_REF_I_IMPL,// does a precise type comparision and returns address (with native int index)

/* Exceptions */

CORINFO_HELP_THROW, // Throw an exception object
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,13 @@ public static void GetEntryPoint(TypeSystemContext context, ReadyToRunHelper id,
mangledName = "RhpLdelemaRef";
break;

case ReadyToRunHelper.Stelem_Ref_I:
mangledName = "RhpStelemRefNint";
break;
case ReadyToRunHelper.Ldelema_Ref_I:
mangledName = "RhpLdelemaRefNint";
break;

case ReadyToRunHelper.MemCpy:
mangledName = "memcpy"; // TODO: Null reference handling
break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -739,6 +739,13 @@ private ISymbolNode GetHelperFtnUncached(CorInfoHelpFunc ftnNum)
id = ReadyToRunHelper.Ldelema_Ref;
break;

case CorInfoHelpFunc.CORINFO_HELP_ARRADDR_ST_I_IMPL:
id = ReadyToRunHelper.Stelem_Ref_I;
break;
case CorInfoHelpFunc.CORINFO_HELP_LDELEMA_REF_I_IMPL:
id = ReadyToRunHelper.Ldelema_Ref_I;
break;


case CorInfoHelpFunc.CORINFO_HELP_GETGENERICS_GCSTATIC_BASE:
id = ReadyToRunHelper.GenericGcStaticBase;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,13 @@ private ISymbolNode GetHelperFtnUncached(CorInfoHelpFunc ftnNum)
id = ReadyToRunHelper.Ldelema_Ref;
break;

case CorInfoHelpFunc.CORINFO_HELP_ARRADDR_ST_I_IMPL:
id = ReadyToRunHelper.Stelem_Ref_I;
break;
case CorInfoHelpFunc.CORINFO_HELP_LDELEMA_REF_I_IMPL:
id = ReadyToRunHelper.Ldelema_Ref_I;
break;

case CorInfoHelpFunc.CORINFO_HELP_MEMSET:
id = ReadyToRunHelper.MemSet;
break;
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -1228,6 +1228,10 @@ DEFINE_METHOD(CASTHELPERS, CHKCASTCLASSSPECIAL, ChkCastClassSpecial, SM_Ptr
DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, SM_PtrVoid_Obj_RetRefByte)
DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_Array_Int_Obj_RetVoid)
DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_Array_Int_PtrVoid_RetRefObj)
#ifdef TARGET_64BIT
DEFINE_METHOD(CASTHELPERS, STELEMREF_NINT, StelemRefNint, SM_Array_IntPtr_Obj_RetVoid)
DEFINE_METHOD(CASTHELPERS, LDELEMAREF_NINT, LdelemaRefNint, SM_Array_IntPtr_PtrVoid_RetRefObj)
#endif

DEFINE_CLASS_U(System, GCMemoryInfoData, GCMemoryInfoData)
DEFINE_FIELD_U(_highMemoryLoadThresholdBytes, GCMemoryInfoData, highMemLoadThresholdBytes)
Expand Down
Loading