From f3354480bfed32948f59b492cddf93a1584e123f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 23 Nov 2022 22:08:13 +0100 Subject: [PATCH 01/19] Fold const RVA access --- src/coreclr/inc/corinfo.h | 1 + src/coreclr/inc/icorjitinfoimpl_generated.h | 1 + src/coreclr/inc/jiteeversionguid.h | 10 +- .../jit/ICorJitInfo_wrapper_generated.hpp | 3 +- src/coreclr/jit/valuenum.cpp | 102 +++++++++++++++++- .../JitInterface/CorInfoImpl_generated.cs | 6 +- .../ThunkGenerator/ThunkInput.txt | 2 +- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 2 +- .../JitInterface/CorInfoImpl.RyuJit.cs | 6 +- .../aot/jitinterface/jitinterface_generated.h | 5 +- .../tools/superpmi/superpmi-shared/lwmlist.h | 2 +- .../superpmi-shared/methodcontext.cpp | 16 +-- .../superpmi/superpmi-shared/methodcontext.h | 6 +- .../superpmi-shim-collector/icorjitinfo.cpp | 6 +- .../icorjitinfo_generated.cpp | 3 +- .../icorjitinfo_generated.cpp | 3 +- .../tools/superpmi/superpmi/icorjitinfo.cpp | 4 +- src/coreclr/vm/jitinterface.cpp | 19 +++- 18 files changed, 158 insertions(+), 39 deletions(-) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 915ad1c4da44b..0feb8619a8108 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -3212,6 +3212,7 @@ class ICorDynamicInfo : public ICorStaticInfo CORINFO_FIELD_HANDLE field, uint8_t *buffer, int bufferSize, + int valueOffset = 0, bool ignoreMovableObjects = true ) = 0; diff --git a/src/coreclr/inc/icorjitinfoimpl_generated.h b/src/coreclr/inc/icorjitinfoimpl_generated.h index 62619e82d4de3..86ac07f8117dd 100644 --- a/src/coreclr/inc/icorjitinfoimpl_generated.h +++ b/src/coreclr/inc/icorjitinfoimpl_generated.h @@ -625,6 +625,7 @@ bool getReadonlyStaticFieldValue( CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, + int valueOffset, bool ignoreMovableObjects) override; CORINFO_CLASS_HANDLE getStaticFieldCurrentClass( diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 9211920488f6c..b999fff982f96 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -43,11 +43,11 @@ typedef const GUID *LPCGUID; #define GUID_DEFINED #endif // !GUID_DEFINED -constexpr GUID JITEEVersionIdentifier = { /* da097b39-7f43-458a-990a-0b65406d5ff3 */ - 0xda097b39, - 0x7f43, - 0x458a, - {0x99, 0xa, 0xb, 0x65, 0x40, 0x6d, 0x5f, 0xf3} +constexpr GUID JITEEVersionIdentifier = { /* 0330a175-dd05-4760-840f-a1a4c47284d3 */ + 0x330a175, + 0xdd05, + 0x4760, + {0x84, 0xf, 0xa1, 0xa4, 0xc4, 0x72, 0x84, 0xd3} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp b/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp index 690210d7fb32f..25b78bba0cae3 100644 --- a/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp +++ b/src/coreclr/jit/ICorJitInfo_wrapper_generated.hpp @@ -1488,10 +1488,11 @@ bool WrapICorJitInfo::getReadonlyStaticFieldValue( CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, + int valueOffset, bool ignoreMovableObjects) { API_ENTER(getReadonlyStaticFieldValue); - bool temp = wrapHnd->getReadonlyStaticFieldValue(field, buffer, bufferSize, ignoreMovableObjects); + bool temp = wrapHnd->getReadonlyStaticFieldValue(field, buffer, bufferSize, valueOffset, ignoreMovableObjects); API_LEAVE(getReadonlyStaticFieldValue); return temp; } diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 246694adc8d3a..9a7e6eb2b5bd2 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -2113,7 +2113,7 @@ ValueNum ValueNumStore::VNForFunc(var_types typ, VNFunc func, ValueNum arg0VN) { uint8_t buffer[TARGET_POINTER_SIZE] = {0}; if (m_pComp->info.compCompHnd->getReadonlyStaticFieldValue(field, buffer, - TARGET_POINTER_SIZE, false)) + TARGET_POINTER_SIZE, 0, false)) { // In case of 64bit jit emitting 32bit codegen this handle will be 64bit // value holding 32bit handle with upper half zeroed (hence, "= NULL"). @@ -8506,9 +8506,105 @@ void Compiler::fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl) // bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) { - ValueNum addrVN = tree->gtGetOp1()->gtVNPair.GetLiberal(); + if (!tree->gtVNPair.BothEqual()) + { + return false; + } + + ValueNum addrVN = tree->gtGetOp1()->gtVNPair.GetLiberal(); + + // First, let's see if we have IND(INT_CNS) field (RVA specifically) + FieldSeq* fieldSeq = nullptr; + if (vnStore->IsVNHandle(addrVN) && (vnStore->GetHandleFlags(addrVN) == GTF_ICON_FIELD_SEQ)) + { + fieldSeq = vnStore->FieldSeqVNToFieldSeq(addrVN); + } + else if (tree->gtGetOp1()->IsCnsIntOrI()) + { + fieldSeq = tree->gtGetOp1()->AsIntCon()->gtFieldSeq; + } + + if (fieldSeq != nullptr) + { + CORINFO_FIELD_HANDLE fieldHandle = fieldSeq->GetFieldHandle(); + assert(fieldSeq->GetKind() == FieldSeq::FieldKind::SimpleStaticKnownAddress); + + ssize_t byteOffset = tree->gtGetOp1()->AsIntCon()->IconValue() - fieldSeq->GetOffset(); + int size = (int)genTypeSize(tree->TypeGet()); + if ((size > 0) && (size <= sizeof(int64_t) && (byteOffset >= 0) && (byteOffset < INT_MAX))) + { + uint8_t buffer[sizeof(int64_t)] = {0}; + if (info.compCompHnd->getReadonlyStaticFieldValue(fieldHandle, (uint8_t*)&buffer, size, (int)byteOffset)) + { + switch (tree->TypeGet()) + { +#define READ_VALUE(typ) \ + typ val = 0; \ + memcpy(&val, buffer, sizeof(typ)); + + case TYP_BOOL: + case TYP_UBYTE: + { + READ_VALUE(uint8_t); + tree->gtVNPair.SetBoth(vnStore->VNForIntCon(val)); + return true; + } + case TYP_BYTE: + { + READ_VALUE(int8_t); + tree->gtVNPair.SetBoth(vnStore->VNForIntCon(val)); + return true; + } + case TYP_SHORT: + { + READ_VALUE(int16_t); + tree->gtVNPair.SetBoth(vnStore->VNForIntCon(val)); + return true; + } + case TYP_USHORT: + { + READ_VALUE(uint16_t); + tree->gtVNPair.SetBoth(vnStore->VNForIntCon(val)); + return true; + } + case TYP_INT: + { + READ_VALUE(int32_t); + tree->gtVNPair.SetBoth(vnStore->VNForIntCon(val)); + return true; + } + case TYP_UINT: + { + READ_VALUE(uint32_t); + tree->gtVNPair.SetBoth(vnStore->VNForIntCon(val)); + return true; + } + case TYP_LONG: + { + READ_VALUE(int64_t); + tree->gtVNPair.SetBoth(vnStore->VNForLongCon(val)); + return true; + } + case TYP_ULONG: + { + READ_VALUE(uint64_t); + tree->gtVNPair.SetBoth(vnStore->VNForLongCon(val)); + return true; + } + // We can add support for fp, structs and SIMD if needed + } + } + } + } + + // Throughput check, the logic below is only for USHORT (char) + if (!tree->TypeIs(TYP_USHORT)) + { + return false; + } + VNFuncApp funcApp; - if (!tree->TypeIs(TYP_USHORT) || !tree->gtVNPair.BothEqual() || !vnStore->GetVNFunc(addrVN, &funcApp)) + if (!vnStore->GetVNFunc(addrVN, &funcApp)) { return false; } diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs index cc8d6c17e1f29..bfca28ea85737 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl_generated.cs @@ -2244,12 +2244,12 @@ private static uint _getClassDomainID(IntPtr thisHandle, IntPtr* ppException, CO } [UnmanagedCallersOnly] - private static byte _getReadonlyStaticFieldValue(IntPtr thisHandle, IntPtr* ppException, CORINFO_FIELD_STRUCT_* field, byte* buffer, int bufferSize, byte ignoreMovableObjects) + private static byte _getReadonlyStaticFieldValue(IntPtr thisHandle, IntPtr* ppException, CORINFO_FIELD_STRUCT_* field, byte* buffer, int bufferSize, int valueOffset, byte ignoreMovableObjects) { var _this = GetThis(thisHandle); try { - return _this.getReadonlyStaticFieldValue(field, buffer, bufferSize, ignoreMovableObjects != 0) ? (byte)1 : (byte)0; + return _this.getReadonlyStaticFieldValue(field, buffer, bufferSize, valueOffset, ignoreMovableObjects != 0) ? (byte)1 : (byte)0; } catch (Exception ex) { @@ -2838,7 +2838,7 @@ private static IntPtr GetUnmanagedCallbacks() callbacks[148] = (delegate* unmanaged)&_isRIDClassDomainID; callbacks[149] = (delegate* unmanaged)&_getClassDomainID; callbacks[150] = (delegate* unmanaged)&_getFieldAddress; - callbacks[151] = (delegate* unmanaged)&_getReadonlyStaticFieldValue; + callbacks[151] = (delegate* unmanaged)&_getReadonlyStaticFieldValue; callbacks[152] = (delegate* unmanaged)&_getStaticFieldCurrentClass; callbacks[153] = (delegate* unmanaged)&_getVarArgsHandle; callbacks[154] = (delegate* unmanaged)&_canGetVarArgsHandle; diff --git a/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt b/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt index 11f2c2b997a70..9be26985000fc 100644 --- a/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt +++ b/src/coreclr/tools/Common/JitInterface/ThunkGenerator/ThunkInput.txt @@ -308,7 +308,7 @@ FUNCTIONS bool isRIDClassDomainID(CORINFO_CLASS_HANDLE cls); unsigned getClassDomainID (CORINFO_CLASS_HANDLE cls, void **ppIndirection); void* getFieldAddress(CORINFO_FIELD_HANDLE field, VOIDSTARSTAR ppIndirection); - bool getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t *buffer, int bufferSize, bool ignoreMovableObjects); + bool getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t *buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects); CORINFO_CLASS_HANDLE getStaticFieldCurrentClass(CORINFO_FIELD_HANDLE field, BoolStar pIsSpeculative); CORINFO_VARARGS_HANDLE getVarArgsHandle(CORINFO_SIG_INFO *pSig, void **ppIndirection); bool canGetVarArgsHandle(CORINFO_SIG_INFO *pSig); diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 826beb13d111a..64aeaa51ac4fb 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -2998,7 +2998,7 @@ private int getExactClasses(CORINFO_CLASS_STRUCT_* baseType, int maxExactClasses return 0; } - private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byte* buffer, int bufferSize, bool ignoreMovableObjects) + private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byte* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects) { return false; } diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index 1738831d07fda..bd9a3e7cee04c 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -2213,7 +2213,7 @@ private int getExactClasses(CORINFO_CLASS_STRUCT_* baseType, int maxExactClasses return index; } - private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byte* buffer, int bufferSize, bool ignoreMovableObjects) + private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byte* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects) { Debug.Assert(fieldHandle != null); Debug.Assert(buffer != null); @@ -2222,8 +2222,10 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt FieldDesc field = HandleToObject(fieldHandle); Debug.Assert(field.IsStatic); - if (!field.IsThreadStatic && field.IsInitOnly && field.OwningType is MetadataType owningType) + if (!field.IsThreadStatic && !filed.HasRva && field.IsInitOnly && field.OwningType is MetadataType owningType) { + Debug.Assert(valueOffset == 0); // is only used for RVA (not supported currently) + PreinitializationManager preinitManager = _compilation.NodeFactory.PreinitializationManager; if (preinitManager.IsPreinitialized(owningType)) { diff --git a/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h b/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h index 1fcfd049f6bbb..c37d7aff4e907 100644 --- a/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h +++ b/src/coreclr/tools/aot/jitinterface/jitinterface_generated.h @@ -162,7 +162,7 @@ struct JitInterfaceCallbacks bool (* isRIDClassDomainID)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_CLASS_HANDLE cls); unsigned (* getClassDomainID)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_CLASS_HANDLE cls, void** ppIndirection); void* (* getFieldAddress)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field, void** ppIndirection); - bool (* getReadonlyStaticFieldValue)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects); + bool (* getReadonlyStaticFieldValue)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects); CORINFO_CLASS_HANDLE (* getStaticFieldCurrentClass)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_FIELD_HANDLE field, bool* pIsSpeculative); CORINFO_VARARGS_HANDLE (* getVarArgsHandle)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_SIG_INFO* pSig, void** ppIndirection); bool (* canGetVarArgsHandle)(void * thisHandle, CorInfoExceptionClass** ppException, CORINFO_SIG_INFO* pSig); @@ -1665,10 +1665,11 @@ class JitInterfaceWrapper : public ICorJitInfo CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, + int valueOffset, bool ignoreMovableObjects) { CorInfoExceptionClass* pException = nullptr; - bool temp = _callbacks->getReadonlyStaticFieldValue(_thisHandle, &pException, field, buffer, bufferSize, ignoreMovableObjects); + bool temp = _callbacks->getReadonlyStaticFieldValue(_thisHandle, &pException, field, buffer, bufferSize, valueOffset, ignoreMovableObjects); if (pException != nullptr) throw pException; return temp; } diff --git a/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h b/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h index a936ac6dad8d3..4bb576b284b77 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/lwmlist.h @@ -78,7 +78,7 @@ LWM(GetDelegateCtor, Agnostic_GetDelegateCtorIn, Agnostic_GetDelegateCtorOut) LWM(GetEEInfo, DWORD, Agnostic_CORINFO_EE_INFO) LWM(GetEHinfo, DLD, Agnostic_CORINFO_EH_CLAUSE) LWM(GetFieldAddress, DWORDLONG, Agnostic_GetFieldAddress) -LWM(GetReadonlyStaticFieldValue, DLDD, DD) +LWM(GetReadonlyStaticFieldValue, DLDDD, DD) LWM(GetStaticFieldCurrentClass, DWORDLONG, Agnostic_GetStaticFieldCurrentClass) LWM(GetFieldClass, DWORDLONG, DWORDLONG) LWM(GetFieldInClass, DLD, DWORDLONG) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index eef0d7f715a7b..fd078673e717d 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -3569,16 +3569,17 @@ void* MethodContext::repGetFieldAddress(CORINFO_FIELD_HANDLE field, void** ppInd return (void*)value.fieldAddress; } -void MethodContext::recGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects, bool result) +void MethodContext::recGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects, bool result) { if (GetReadonlyStaticFieldValue == nullptr) - GetReadonlyStaticFieldValue = new LightWeightMap(); + GetReadonlyStaticFieldValue = new LightWeightMap(); - DLDD key; + DLDDD key; ZeroMemory(&key, sizeof(key)); key.A = CastHandle(field); key.B = (DWORD)bufferSize; key.C = (DWORD)ignoreMovableObjects; + key.D = (DWORD)valueOffset; DWORD tmpBuf = (DWORD)-1; if (buffer != nullptr && result) @@ -3591,18 +3592,19 @@ void MethodContext::recGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, u GetReadonlyStaticFieldValue->Add(key, value); DEBUG_REC(dmpGetReadonlyStaticFieldValue(key, value)); } -void MethodContext::dmpGetReadonlyStaticFieldValue(DLDD key, DD value) +void MethodContext::dmpGetReadonlyStaticFieldValue(DLDDD key, DD value) { - printf("GetReadonlyStaticFieldValue key fld-%016llX bufSize-%u, ignoremovable-%u, result-%u", key.A, key.B, key.C, value.A); + printf("GetReadonlyStaticFieldValue key fld-%016llX bufSize-%u, ignoremovable-%u, valOffset-%u result-%u", key.A, key.B, key.C, key.D, value.A); GetReadonlyStaticFieldValue->Unlock(); } -bool MethodContext::repGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects) +bool MethodContext::repGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects) { - DLDD key; + DLDDD key; ZeroMemory(&key, sizeof(key)); key.A = CastHandle(field); key.B = (DWORD)bufferSize; key.C = (DWORD)ignoreMovableObjects; + key.D = (DWORD)valueOffset; DD value = LookupByKeyOrMiss(GetReadonlyStaticFieldValue, key, ": key %016llX", key.A); diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h index b1ef506a95c3c..d943d3f7f3950 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.h @@ -492,9 +492,9 @@ class MethodContext void dmpGetFieldAddress(DWORDLONG key, const Agnostic_GetFieldAddress& value); void* repGetFieldAddress(CORINFO_FIELD_HANDLE field, void** ppIndirection); - void recGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects, bool result); - void dmpGetReadonlyStaticFieldValue(DLDD key, DD value); - bool repGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects); + void recGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects, bool result); + void dmpGetReadonlyStaticFieldValue(DLDDD key, DD value); + bool repGetReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects); void recGetStaticFieldCurrentClass(CORINFO_FIELD_HANDLE field, bool isSpeculative, CORINFO_CLASS_HANDLE result); void dmpGetStaticFieldCurrentClass(DWORDLONG key, const Agnostic_GetStaticFieldCurrentClass& value); diff --git a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp index dc7e4bf02952a..90b5b4d448e92 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-collector/icorjitinfo.cpp @@ -1713,11 +1713,11 @@ void* interceptor_ICJI::getFieldAddress(CORINFO_FIELD_HANDLE field, void** ppInd return temp; } -bool interceptor_ICJI::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects) +bool interceptor_ICJI::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects) { mc->cr->AddCall("getReadonlyStaticFieldValue"); - bool result = original_ICorJitInfo->getReadonlyStaticFieldValue(field, buffer, bufferSize, ignoreMovableObjects); - mc->recGetReadonlyStaticFieldValue(field, buffer, bufferSize, ignoreMovableObjects, result); + bool result = original_ICorJitInfo->getReadonlyStaticFieldValue(field, buffer, bufferSize, valueOffset, ignoreMovableObjects); + mc->recGetReadonlyStaticFieldValue(field, buffer, bufferSize, valueOffset, ignoreMovableObjects, result); return result; } diff --git a/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp b/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp index 50212182b6864..9bea437e950ef 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-counter/icorjitinfo_generated.cpp @@ -1220,10 +1220,11 @@ bool interceptor_ICJI::getReadonlyStaticFieldValue( CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, + int valueOffset, bool ignoreMovableObjects) { mcs->AddCall("getReadonlyStaticFieldValue"); - return original_ICorJitInfo->getReadonlyStaticFieldValue(field, buffer, bufferSize, ignoreMovableObjects); + return original_ICorJitInfo->getReadonlyStaticFieldValue(field, buffer, bufferSize, valueOffset, ignoreMovableObjects); } CORINFO_CLASS_HANDLE interceptor_ICJI::getStaticFieldCurrentClass( diff --git a/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp b/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp index fd3edaa53f2f2..b9c55b95d98d0 100644 --- a/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shim-simple/icorjitinfo_generated.cpp @@ -1069,9 +1069,10 @@ bool interceptor_ICJI::getReadonlyStaticFieldValue( CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, + int valueOffset, bool ignoreMovableObjects) { - return original_ICorJitInfo->getReadonlyStaticFieldValue(field, buffer, bufferSize, ignoreMovableObjects); + return original_ICorJitInfo->getReadonlyStaticFieldValue(field, buffer, bufferSize, valueOffset, ignoreMovableObjects); } CORINFO_CLASS_HANDLE interceptor_ICJI::getStaticFieldCurrentClass( diff --git a/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp b/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp index f5970f4f80a45..9dc9f922b527e 100644 --- a/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp +++ b/src/coreclr/tools/superpmi/superpmi/icorjitinfo.cpp @@ -1488,10 +1488,10 @@ void* MyICJI::getFieldAddress(CORINFO_FIELD_HANDLE field, void** ppIndirection) return jitInstance->mc->repGetFieldAddress(field, ppIndirection); } -bool MyICJI::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects) +bool MyICJI::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE field, uint8_t* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects) { jitInstance->mc->cr->AddCall("getReadonlyStaticFieldValue"); - return jitInstance->mc->repGetReadonlyStaticFieldValue(field, buffer, bufferSize, ignoreMovableObjects); + return jitInstance->mc->repGetReadonlyStaticFieldValue(field, buffer, bufferSize, valueOffset, ignoreMovableObjects); } // return the class handle for the current value of a static field diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 6d37ff818217f..289ded6f5f05c 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -11975,7 +11975,7 @@ void* CEEJitInfo::getFieldAddress(CORINFO_FIELD_HANDLE fieldHnd, return result; } -bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t* buffer, int bufferSize, bool ignoreMovableObjects) +bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects) { CONTRACTL { THROWS; @@ -11993,7 +11993,7 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t FieldDesc* field = (FieldDesc*)fieldHnd; _ASSERTE(field->IsStatic()); - _ASSERTE((unsigned)bufferSize == field->GetSize()); + _ASSERTE((unsigned)bufferSize == field->GetSize() || field->IsRVA()); MethodTable* pEnclosingMT = field->GetEnclosingMethodTable(); _ASSERTE(!pEnclosingMT->IsSharedByGenericInstantiations()); @@ -12007,8 +12007,20 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t if (!field->IsThreadStatic() && pEnclosingMT->IsClassInited() && IsFdInitOnly(field->GetAttributes())) { GCX_COOP(); - if (field->IsObjRef()) + if (field->IsRVA()) { + _ASSERT(!field->IsObjRef()); + size_t baseAddr = (size_t)field->GetCurrentStaticAddress(); + size_t size = field->GetSize(); + if (valueOffset >= 0 && valueOffset <= size - bufferSize) + { + memcpy(buffer, (uint8_t*)baseAddr + valueOffset, bufferSize); + result = true; + } + } + else if (field->IsObjRef()) + { + _ASSERT(valueOffset == 0); // is only used for RVA currently OBJECTREF fieldObj = field->GetStaticOBJECTREF(); if (fieldObj != NULL) { @@ -12039,6 +12051,7 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t } else { + _ASSERT(valueOffset == 0); // is only used for RVA currently void* fldAddr = field->GetCurrentStaticAddress(); _ASSERTE(fldAddr != nullptr); memcpy(buffer, fldAddr, bufferSize); From 0282bfe929f56d9c837b6d7800f0cc204c4da52f Mon Sep 17 00:00:00 2001 From: EgorBo Date: Wed, 23 Nov 2022 22:27:54 +0100 Subject: [PATCH 02/19] fix build --- src/coreclr/jit/valuenum.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 9a7e6eb2b5bd2..bc142afb342b5 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8591,7 +8591,9 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) tree->gtVNPair.SetBoth(vnStore->VNForLongCon(val)); return true; } - // We can add support for fp, structs and SIMD if needed + // We can add support for fp, structs and SIMD if needed + default: + break; } } } From 9780a9ad69bbeb25aef22aecdb3063274f97825a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 24 Nov 2022 00:06:58 +0100 Subject: [PATCH 03/19] Address feedback --- .../JitInterface/CorInfoImpl.RyuJit.cs | 2 +- src/coreclr/vm/jitinterface.cpp | 32 ++++++++----------- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index bd9a3e7cee04c..d736e027530be 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -2222,7 +2222,7 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt FieldDesc field = HandleToObject(fieldHandle); Debug.Assert(field.IsStatic); - if (!field.IsThreadStatic && !filed.HasRva && field.IsInitOnly && field.OwningType is MetadataType owningType) + if (!field.IsThreadStatic && !field.HasRva && field.IsInitOnly && field.OwningType is MetadataType owningType) { Debug.Assert(valueOffset == 0); // is only used for RVA (not supported currently) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 289ded6f5f05c..dd1ee61720ee8 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -11993,7 +11993,6 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t FieldDesc* field = (FieldDesc*)fieldHnd; _ASSERTE(field->IsStatic()); - _ASSERTE((unsigned)bufferSize == field->GetSize() || field->IsRVA()); MethodTable* pEnclosingMT = field->GetEnclosingMethodTable(); _ASSERTE(!pEnclosingMT->IsSharedByGenericInstantiations()); @@ -12007,20 +12006,10 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t if (!field->IsThreadStatic() && pEnclosingMT->IsClassInited() && IsFdInitOnly(field->GetAttributes())) { GCX_COOP(); - if (field->IsRVA()) + if (field->IsObjRef()) { - _ASSERT(!field->IsObjRef()); - size_t baseAddr = (size_t)field->GetCurrentStaticAddress(); - size_t size = field->GetSize(); - if (valueOffset >= 0 && valueOffset <= size - bufferSize) - { - memcpy(buffer, (uint8_t*)baseAddr + valueOffset, bufferSize); - result = true; - } - } - else if (field->IsObjRef()) - { - _ASSERT(valueOffset == 0); // is only used for RVA currently + _ASSERT(!field->IsRVA()); + _ASSERT(valueOffset == 0); // there is no point in returning a chunk of a gc handle OBJECTREF fieldObj = field->GetStaticOBJECTREF(); if (fieldObj != NULL) { @@ -12051,11 +12040,16 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t } else { - _ASSERT(valueOffset == 0); // is only used for RVA currently - void* fldAddr = field->GetCurrentStaticAddress(); - _ASSERTE(fldAddr != nullptr); - memcpy(buffer, fldAddr, bufferSize); - result = true; + // Either RVA, primitve or struct (or even part of that struct) + size_t baseAddr = (size_t)field->GetCurrentStaticAddress(); + size_t size = field->GetSize(); + _ASSERTE(baseAddr > 0); + _ASSERTE(size > 0); + if (valueOffset >= 0 && valueOffset <= size - bufferSize) + { + memcpy(buffer, (uint8_t*)baseAddr + valueOffset, bufferSize); + result = true; + } } } From ae56d28ecec3c60fd6540b7533aee82c7a905e69 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 24 Nov 2022 00:26:12 +0100 Subject: [PATCH 04/19] Clean up --- src/coreclr/jit/valuenum.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index bc142afb342b5..0ee994a299bc8 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8529,11 +8529,12 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) CORINFO_FIELD_HANDLE fieldHandle = fieldSeq->GetFieldHandle(); assert(fieldSeq->GetKind() == FieldSeq::FieldKind::SimpleStaticKnownAddress); - ssize_t byteOffset = tree->gtGetOp1()->AsIntCon()->IconValue() - fieldSeq->GetOffset(); - int size = (int)genTypeSize(tree->TypeGet()); - if ((size > 0) && (size <= sizeof(int64_t) && (byteOffset >= 0) && (byteOffset < INT_MAX))) + ssize_t byteOffset = tree->gtGetOp1()->AsIntCon()->IconValue() - fieldSeq->GetOffset(); + int size = (int)genTypeSize(tree->TypeGet()); + const int maxElementSize = sizeof(int64_t); + if ((size > 0) && (size <= maxElementSize) && (byteOffset >= 0) && (byteOffset < INT_MAX)) { - uint8_t buffer[sizeof(int64_t)] = {0}; + uint8_t buffer[maxElementSize] = {0}; if (info.compCompHnd->getReadonlyStaticFieldValue(fieldHandle, (uint8_t*)&buffer, size, (int)byteOffset)) { switch (tree->TypeGet()) From e35ac7cb15838e5dd9de6585fd59f7340f3944de Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 24 Nov 2022 01:00:44 +0100 Subject: [PATCH 05/19] fix compilation --- src/coreclr/vm/jitinterface.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index dd1ee61720ee8..1881d9b7b4d5f 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -12010,6 +12010,8 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t { _ASSERT(!field->IsRVA()); _ASSERT(valueOffset == 0); // there is no point in returning a chunk of a gc handle + _ASSERT(bufferSize == field->GetSize()); + OBJECTREF fieldObj = field->GetStaticOBJECTREF(); if (fieldObj != NULL) { @@ -12042,10 +12044,11 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t { // Either RVA, primitve or struct (or even part of that struct) size_t baseAddr = (size_t)field->GetCurrentStaticAddress(); - size_t size = field->GetSize(); + UINT size = field->GetSize(); _ASSERTE(baseAddr > 0); _ASSERTE(size > 0); - if (valueOffset >= 0 && valueOffset <= size - bufferSize) + + if (size >= (UINT)bufferSize && valueOffset >= 0 && (UINT)valueOffset <= size - (UINT)bufferSize) { memcpy(buffer, (uint8_t*)baseAddr + valueOffset, bufferSize); result = true; From b8e2dd57c649a9829a55c23d5f5a91c0a002542b Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 24 Nov 2022 02:29:10 +0100 Subject: [PATCH 06/19] NativeAOT support (jit side) --- src/coreclr/jit/assertionprop.cpp | 4 ++- src/coreclr/jit/valuenum.cpp | 34 +++++++++++++------ .../JitInterface/CorInfoImpl.RyuJit.cs | 13 +++++-- src/coreclr/vm/jitinterface.cpp | 2 +- 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 30aaa8c5be1af..385fdd1953ff7 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3364,7 +3364,9 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, case O2K_CONST_INT: // Don't propagate handles if we need to report relocs. - if (opts.compReloc && curAssertion->op2.HasIconFlag()) + // Allow for static field handles. + if (opts.compReloc && curAssertion->op2.HasIconFlag() && + (curAssertion->op2.GetIconFlag() != GTF_ICON_STATIC_HDL)) { return nullptr; } diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 0ee994a299bc8..465e8fef1a680 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8494,6 +8494,26 @@ void Compiler::fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl) } } +static FieldSeq* GetFieldSeqFromTree(GenTree* tree, ssize_t* address) +{ + if (tree->IsCnsIntOrI()) + { + *address = tree->AsIntCon()->IconValue(); + return tree->AsIntCon()->gtFieldSeq; + } + else if (tree->OperIs(GT_ADD) && tree->gtGetOp2()->IsCnsIntOrI()) + { + ssize_t addr = 0; + FieldSeq* seq = GetFieldSeqFromTree(tree->gtGetOp1(), &addr); + if (seq != nullptr && !tree->gtGetOp2()->IsIconHandle()) + { + *address = addr + tree->gtGetOp2()->AsIntCon()->IconValue(); + return seq; + } + } + return nullptr; +} + //---------------------------------------------------------------------------------- // fgValueNumberConstLoad: Try to detect const_immutable_array[cns_index] tree // and apply a constant VN representing given element at cns_index in that array. @@ -8513,23 +8533,15 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) ValueNum addrVN = tree->gtGetOp1()->gtVNPair.GetLiberal(); - // First, let's see if we have IND(INT_CNS) field (RVA specifically) - FieldSeq* fieldSeq = nullptr; - if (vnStore->IsVNHandle(addrVN) && (vnStore->GetHandleFlags(addrVN) == GTF_ICON_FIELD_SEQ)) - { - fieldSeq = vnStore->FieldSeqVNToFieldSeq(addrVN); - } - else if (tree->gtGetOp1()->IsCnsIntOrI()) - { - fieldSeq = tree->gtGetOp1()->AsIntCon()->gtFieldSeq; - } + ssize_t address = 0; + FieldSeq* fieldSeq = GetFieldSeqFromTree(tree->gtGetOp1(), &address); if (fieldSeq != nullptr) { CORINFO_FIELD_HANDLE fieldHandle = fieldSeq->GetFieldHandle(); assert(fieldSeq->GetKind() == FieldSeq::FieldKind::SimpleStaticKnownAddress); - ssize_t byteOffset = tree->gtGetOp1()->AsIntCon()->IconValue() - fieldSeq->GetOffset(); + ssize_t byteOffset = address - fieldSeq->GetOffset(); int size = (int)genTypeSize(tree->TypeGet()); const int maxElementSize = sizeof(int64_t); if ((size > 0) && (size <= maxElementSize) && (byteOffset >= 0) && (byteOffset < INT_MAX)) diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index d736e027530be..698e8b78d4b03 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -2222,9 +2222,18 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt FieldDesc field = HandleToObject(fieldHandle); Debug.Assert(field.IsStatic); - if (!field.IsThreadStatic && !field.HasRva && field.IsInitOnly && field.OwningType is MetadataType owningType) + + if (!field.IsThreadStatic && field.IsInitOnly && field.OwningType is MetadataType owningType) { - Debug.Assert(valueOffset == 0); // is only used for RVA (not supported currently) + if (field.HasRva) + { + // TODO: get data + //ISymbolNode fieldRvaData = _compilation.GetFieldRvaData(field); + //byte* ptr = (byte*)ObjectToHandle(fieldRvaData); -- indirect + return false; + } + + Debug.Assert(valueOffset == 0); // is only used for RVA at the moment PreinitializationManager preinitManager = _compilation.NodeFactory.PreinitializationManager; if (preinitManager.IsPreinitialized(owningType)) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 1881d9b7b4d5f..6a930d19fcc99 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -12010,7 +12010,7 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t { _ASSERT(!field->IsRVA()); _ASSERT(valueOffset == 0); // there is no point in returning a chunk of a gc handle - _ASSERT(bufferSize == field->GetSize()); + _ASSERT((UINT)bufferSize == field->GetSize()); OBJECTREF fieldObj = field->GetStaticOBJECTREF(); if (fieldObj != NULL) From aaf129e41b45d4cb2f6dc694eb7cf39ea4c9a67d Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 24 Nov 2022 02:29:34 +0100 Subject: [PATCH 07/19] Update src/coreclr/vm/jitinterface.cpp Co-authored-by: Jan Kotas --- src/coreclr/vm/jitinterface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index 6a930d19fcc99..ad1b83b174de3 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -12042,7 +12042,7 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t } else { - // Either RVA, primitve or struct (or even part of that struct) + // Either RVA, primitive or struct (or even part of that struct) size_t baseAddr = (size_t)field->GetCurrentStaticAddress(); UINT size = field->GetSize(); _ASSERTE(baseAddr > 0); From f7b0d4e15c332758a3158fa87bb84c92fda797dd Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 24 Nov 2022 02:43:44 +0100 Subject: [PATCH 08/19] Implement NativeAOT side --- .../JitInterface/CorInfoImpl.RyuJit.cs | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index 698e8b78d4b03..09dd5cd1b1847 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -13,6 +13,8 @@ using ILCompiler; using ILCompiler.DependencyAnalysis; +using Internal.TypeSystem.Ecma; +using System.Drawing; #if SUPPORT_JIT using MethodCodeNode = Internal.Runtime.JitSupport.JitMethodCodeNode; @@ -2227,9 +2229,15 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt { if (field.HasRva) { - // TODO: get data - //ISymbolNode fieldRvaData = _compilation.GetFieldRvaData(field); - //byte* ptr = (byte*)ObjectToHandle(fieldRvaData); -- indirect + if (field is EcmaField ecmaField) + { + ReadOnlySpan rvaData = ecmaField.GetFieldRvaData(); + if (rvaData.Length >= bufferSize && valueOffset >= 0 && valueOffset <= rvaData.Length - bufferSize) + { + rvaData.Slice(valueOffset, bufferSize).CopyTo(new Span(buffer, bufferSize)); + return true; + } + } return false; } From 29f4e9d3724e7327d274c70c0fb372a5f00671af Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 24 Nov 2022 11:09:12 +0100 Subject: [PATCH 09/19] Update assertionprop.cpp --- src/coreclr/jit/assertionprop.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 385fdd1953ff7..30aaa8c5be1af 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3364,9 +3364,7 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, case O2K_CONST_INT: // Don't propagate handles if we need to report relocs. - // Allow for static field handles. - if (opts.compReloc && curAssertion->op2.HasIconFlag() && - (curAssertion->op2.GetIconFlag() != GTF_ICON_STATIC_HDL)) + if (opts.compReloc && curAssertion->op2.HasIconFlag()) { return nullptr; } From 8d471c130105beae44939fb841c5454d70c15c30 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 24 Nov 2022 16:43:10 +0100 Subject: [PATCH 10/19] Add tests, clean up --- src/coreclr/jit/valuenum.cpp | 39 +++--- .../JIT/opt/ValueNumbering/ConstIndexRVA.cs | 112 ++++++++++++++++++ .../opt/ValueNumbering/ConstIndexRVA.csproj | 9 ++ 3 files changed, 136 insertions(+), 24 deletions(-) create mode 100644 src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs create mode 100644 src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.csproj diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 465e8fef1a680..206d8ed52dd8b 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8494,26 +8494,6 @@ void Compiler::fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl) } } -static FieldSeq* GetFieldSeqFromTree(GenTree* tree, ssize_t* address) -{ - if (tree->IsCnsIntOrI()) - { - *address = tree->AsIntCon()->IconValue(); - return tree->AsIntCon()->gtFieldSeq; - } - else if (tree->OperIs(GT_ADD) && tree->gtGetOp2()->IsCnsIntOrI()) - { - ssize_t addr = 0; - FieldSeq* seq = GetFieldSeqFromTree(tree->gtGetOp1(), &addr); - if (seq != nullptr && !tree->gtGetOp2()->IsIconHandle()) - { - *address = addr + tree->gtGetOp2()->AsIntCon()->IconValue(); - return seq; - } - } - return nullptr; -} - //---------------------------------------------------------------------------------- // fgValueNumberConstLoad: Try to detect const_immutable_array[cns_index] tree // and apply a constant VN representing given element at cns_index in that array. @@ -8531,10 +8511,20 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) return false; } - ValueNum addrVN = tree->gtGetOp1()->gtVNPair.GetLiberal(); - + // First, let's check if we can detect RVA[const_index] pattern to fold, e.g.: + // + // static ReadOnlySpan RVA => new sbyte[] { -100, 100 } + // + // sbyte GetVal() => RVA[1]; // fold to '100' + // ssize_t address = 0; - FieldSeq* fieldSeq = GetFieldSeqFromTree(tree->gtGetOp1(), &address); + FieldSeq* fieldSeq = nullptr; + + if (tree->gtGetOp1()->IsCnsIntOrI()) + { + fieldSeq = tree->gtGetOp1()->AsIntCon()->gtFieldSeq; + address = tree->gtGetOp1()->AsIntCon()->IconValue(); + } if (fieldSeq != nullptr) { @@ -8549,6 +8539,7 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) uint8_t buffer[maxElementSize] = {0}; if (info.compCompHnd->getReadonlyStaticFieldValue(fieldHandle, (uint8_t*)&buffer, size, (int)byteOffset)) { + // For now we only support these primitives, we can extend this list to FP, SIMD and structs in future. switch (tree->TypeGet()) { #define READ_VALUE(typ) \ @@ -8604,7 +8595,6 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) tree->gtVNPair.SetBoth(vnStore->VNForLongCon(val)); return true; } - // We can add support for fp, structs and SIMD if needed default: break; } @@ -8618,6 +8608,7 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) return false; } + ValueNum addrVN = tree->gtGetOp1()->gtVNPair.GetLiberal(); VNFuncApp funcApp; if (!vnStore->GetVNFunc(addrVN, &funcApp)) { diff --git a/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs b/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs new file mode 100644 index 0000000000000..791d5d684c042 --- /dev/null +++ b/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs @@ -0,0 +1,112 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Reflection; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; + +class RvaTests +{ + static int Main() + { + if (!BitConverter.IsLittleEndian) + { + // Test is not BE friendly + return 100; + } + + int testMethods = 0; + foreach (MethodInfo mth in typeof(RvaTests) + .GetMethods(BindingFlags.Static | BindingFlags.NonPublic) + .Where(m => m.Name.StartsWith("Test_"))) + { + mth.Invoke(null, null); + testMethods++; + } + return testMethods == 26 ? 100 : -100; + } + + static ReadOnlySpan RVA1 => new byte[] + { + 0x9c, 0x00, 0x01, 0x10, + 0x80, 0xAA, 0xAB, 0xFF, + 0x9b, 0x02, 0x03, 0x14, + 0x85, 0xA6, 0xA7, 0xF9, + }; + static ReadOnlySpan RVA2 => new sbyte[] { -100, 100, 0, -128, 127 }; + static ReadOnlySpan RVA3 => new [] { true, false }; + + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_1() => AssertEquals((int)RVA1[0], (int)0x9c); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_2() => AssertEquals(RVA1[1], 0x00); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_3() => AssertEquals(RVA1[4], 0x80); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_4() => AssertEquals(RVA1[RVA1.Length -1], 0xF9); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_5() => ThrowsOOB(() => Consume(RVA1[-1])); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_6() => ThrowsOOB(() => Consume(RVA1[0xFF])); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_7() => ThrowsOOB(() => Consume(RVA1[RVA1.Length])); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_8() => AssertEquals((long)RVA2[0], -100); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_9() => AssertEquals(RVA2[1], 100); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_11() => AssertEquals(RVA2[^1], 127); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_12() => ThrowsOOB(() => Consume(RVA2[-int.MaxValue])); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_13() => ThrowsOOB(() => Consume(RVA2[0xFF])); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_14() => ThrowsOOB(() => Consume(RVA2[RVA2.Length])); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_15() => AssertEquals(RVA3[0], true); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_16() => AssertEquals(RVA3[1], false); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_17() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 0)), 268501148); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_18() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 1)), 2148532480); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_19() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 11)), -1482259180); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_20() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 0)), 156); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_21() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 1)), 11240891943721369856); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_22() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 4)), 1441999174721317504); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_23() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 4)), new MyStruct(1441999174721317504)); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_24() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 4)), new Guid("ffabaa80-029b-1403-85a6-a7f900000000")); + + [MethodImpl(MethodImplOptions.NoInlining)] + static int Test_25() // AssertProp test + { + byte x = RVA1[1]; + if (x > 100) + { + Consume(x); + } + return x; + } + [MethodImpl(MethodImplOptions.NoInlining)] + static int Test_26() // AssertProp test + { + sbyte x = RVA2[0]; + if (x > 100) + { + Consume(x); + } + return x; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void ThrowsOOB(Action action) + { + try + { + action(); + } + catch (IndexOutOfRangeException) + { + return; + } + throw new InvalidOperationException("IndexOutOfRangeException was expected"); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void AssertEquals(T actual, T expected, [CallerLineNumber] int line = 0) + { + if (!actual.Equals(expected)) + { + throw new InvalidOperationException($"Line:{line} actual={actual}, expected={expected}"); + } + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void Consume(T _) {} + + public record struct MyStruct(long a); +} diff --git a/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.csproj b/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.csproj new file mode 100644 index 0000000000000..6946bed81bfd5 --- /dev/null +++ b/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.csproj @@ -0,0 +1,9 @@ + + + Exe + True + + + + + From 7f402fc10010d022b9adaa728df0e75ea6f923b4 Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Thu, 24 Nov 2022 17:35:30 +0100 Subject: [PATCH 11/19] Update ConstIndexRVA.cs --- src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs b/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs index 791d5d684c042..9bd8bd3ece306 100644 --- a/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs +++ b/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; From 14743b6e342057f8223d2b281fc1990a4694309d Mon Sep 17 00:00:00 2001 From: EgorBo Date: Thu, 24 Nov 2022 21:25:17 +0100 Subject: [PATCH 12/19] fix test --- .../JIT/opt/ValueNumbering/ConstIndexRVA.cs | 43 ++++++++++--------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs b/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs index 791d5d684c042..e16edb9f1a67b 100644 --- a/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs +++ b/src/tests/JIT/opt/ValueNumbering/ConstIndexRVA.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Linq; using System.Reflection; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -24,45 +25,45 @@ static int Main() mth.Invoke(null, null); testMethods++; } - return testMethods == 26 ? 100 : -100; + return testMethods == 25 ? 100 : -100; } static ReadOnlySpan RVA1 => new byte[] { - 0x9c, 0x00, 0x01, 0x10, + 0x9c, 0x00, 0x01, 0x10, 0x80, 0xAA, 0xAB, 0xFF, 0x9b, 0x02, 0x03, 0x14, 0x85, 0xA6, 0xA7, 0xF9, }; static ReadOnlySpan RVA2 => new sbyte[] { -100, 100, 0, -128, 127 }; - static ReadOnlySpan RVA3 => new [] { true, false }; + static ReadOnlySpan RVA3 => new[] { true, false }; [MethodImpl(MethodImplOptions.NoInlining)] static void Test_1() => AssertEquals((int)RVA1[0], (int)0x9c); [MethodImpl(MethodImplOptions.NoInlining)] static void Test_2() => AssertEquals(RVA1[1], 0x00); [MethodImpl(MethodImplOptions.NoInlining)] static void Test_3() => AssertEquals(RVA1[4], 0x80); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_4() => AssertEquals(RVA1[RVA1.Length -1], 0xF9); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_4() => AssertEquals(RVA1[RVA1.Length - 1], 0xF9); [MethodImpl(MethodImplOptions.NoInlining)] static void Test_5() => ThrowsOOB(() => Consume(RVA1[-1])); [MethodImpl(MethodImplOptions.NoInlining)] static void Test_6() => ThrowsOOB(() => Consume(RVA1[0xFF])); [MethodImpl(MethodImplOptions.NoInlining)] static void Test_7() => ThrowsOOB(() => Consume(RVA1[RVA1.Length])); [MethodImpl(MethodImplOptions.NoInlining)] static void Test_8() => AssertEquals((long)RVA2[0], -100); [MethodImpl(MethodImplOptions.NoInlining)] static void Test_9() => AssertEquals(RVA2[1], 100); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_11() => AssertEquals(RVA2[^1], 127); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_12() => ThrowsOOB(() => Consume(RVA2[-int.MaxValue])); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_13() => ThrowsOOB(() => Consume(RVA2[0xFF])); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_14() => ThrowsOOB(() => Consume(RVA2[RVA2.Length])); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_15() => AssertEquals(RVA3[0], true); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_16() => AssertEquals(RVA3[1], false); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_17() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 0)), 268501148); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_18() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 1)), 2148532480); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_19() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 11)), -1482259180); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_20() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 0)), 156); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_21() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 1)), 11240891943721369856); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_22() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 4)), 1441999174721317504); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_23() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 4)), new MyStruct(1441999174721317504)); - [MethodImpl(MethodImplOptions.NoInlining)] static void Test_24() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 4)), new Guid("ffabaa80-029b-1403-85a6-a7f900000000")); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_10() => AssertEquals(RVA2[^1], 127); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_11() => ThrowsOOB(() => Consume(RVA2[-int.MaxValue])); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_12() => ThrowsOOB(() => Consume(RVA2[0xFF])); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_13() => ThrowsOOB(() => Consume(RVA2[RVA2.Length])); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_14() => AssertEquals(RVA3[0], true); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_15() => AssertEquals(RVA3[1], false); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_16() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 0)), 268501148); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_17() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 1)), 2148532480); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_18() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 10)), -1501228029); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_19() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 0)), 156); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_20() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 1)), 11240891943721369856); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_21() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 4)), 1441999174721317504); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_22() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 0)), new MyStruct(-23737906019368804)); + [MethodImpl(MethodImplOptions.NoInlining)] static void Test_23() => AssertEquals(Unsafe.ReadUnaligned(ref Unsafe.Add(ref MemoryMarshal.GetReference(RVA1), 0)), new Guid("1001009c-aa80-ffab-9b02-031485a6a7f9")); [MethodImpl(MethodImplOptions.NoInlining)] - static int Test_25() // AssertProp test + static int Test_24() // AssertProp test { byte x = RVA1[1]; if (x > 100) @@ -72,7 +73,7 @@ static int Test_25() // AssertProp test return x; } [MethodImpl(MethodImplOptions.NoInlining)] - static int Test_26() // AssertProp test + static int Test_25() // AssertProp test { sbyte x = RVA2[0]; if (x > 100) @@ -106,7 +107,7 @@ static void AssertEquals(T actual, T expected, [CallerLineNumber] int line = } [MethodImpl(MethodImplOptions.NoInlining)] - static void Consume(T _) {} + static void Consume(T _) { } public record struct MyStruct(long a); } From 142ad612a4e835d7ae8eede3b5215c328925deeb Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 26 Nov 2022 01:57:00 +0100 Subject: [PATCH 13/19] Address feedback --- src/coreclr/jit/valuenum.cpp | 68 ++++++++++++++++--- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 17 +++++ .../JitInterface/CorInfoImpl.RyuJit.cs | 19 +++--- src/coreclr/vm/jitinterface.cpp | 1 + 4 files changed, 88 insertions(+), 17 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 206d8ed52dd8b..6bc5138101a74 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8494,6 +8494,65 @@ void Compiler::fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl) } } +//---------------------------------------------------------------------------------- +// fgGetFieldSeqAndAddress: Try to obtain a constant address with a FieldSeq from the +// given tree. It can be either INT_CNS or e.g. ADD(INT_CNS, ADD(INT_CNS, INT_CNS)) +// tree where only one of the constants is expected to have a field sequence. +// +// Arguments: +// tree - tree node to inspect +// pAddress - [Out] resulting address with all offsets combined +// pFseq - [Out] field sequence +// +// Return Value: +// true if the pattern was recognized and a new VN is assigned +// +static bool fgGetFieldSeqAndAddress(GenTree* tree, ssize_t* pAddress, FieldSeq** pFseq) +{ + if (tree->IsCnsIntOrI()) + { + ssize_t val = tree->AsIntCon()->IconValue(); + FieldSeq* fseq = tree->AsIntCon()->gtFieldSeq; + if (fseq != nullptr) + { + *pFseq = fseq; + *pAddress = val; + return true; + } + return false; + } + + ssize_t val = 0; + while (tree->OperIs(GT_ADD)) + { + GenTree* op1 = tree->gtGetOp1(); + GenTree* op2 = tree->gtGetOp2(); + if (op1->IsCnsIntOrI() && (op1->AsIntCon()->gtFieldSeq == nullptr)) + { + val += op1->AsIntCon()->IconValue(); + tree = op2; + } + else if (op2->IsCnsIntOrI() && (op2->AsIntCon()->gtFieldSeq == nullptr)) + { + val += op2->AsIntCon()->IconValue(); + tree = op1; + } + else + { + // Unsupported tree + return false; + } + } + + if (tree->IsCnsIntOrI() && (tree->AsIntCon()->gtFieldSeq != nullptr)) + { + *pFseq = tree->AsIntCon()->gtFieldSeq; + *pAddress = tree->AsIntCon()->IconValue() + val; + return true; + } + return false; +} + //---------------------------------------------------------------------------------- // fgValueNumberConstLoad: Try to detect const_immutable_array[cns_index] tree // and apply a constant VN representing given element at cns_index in that array. @@ -8519,14 +8578,7 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) // ssize_t address = 0; FieldSeq* fieldSeq = nullptr; - - if (tree->gtGetOp1()->IsCnsIntOrI()) - { - fieldSeq = tree->gtGetOp1()->AsIntCon()->gtFieldSeq; - address = tree->gtGetOp1()->AsIntCon()->IconValue(); - } - - if (fieldSeq != nullptr) + if (fgGetFieldSeqAndAddress(tree->gtGetOp1(), &address, &fieldSeq)) { CORINFO_FIELD_HANDLE fieldHandle = fieldSeq->GetFieldHandle(); assert(fieldSeq->GetKind() == FieldSeq::FieldKind::SimpleStaticKnownAddress); diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 64aeaa51ac4fb..7b14ac346e2a7 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -3000,6 +3000,23 @@ private int getExactClasses(CORINFO_CLASS_STRUCT_* baseType, int maxExactClasses private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byte* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects) { + Debug.Assert(fieldHandle != null); + Debug.Assert(buffer != null); + Debug.Assert(bufferSize > 0); + Debug.Assert(valueOffset >= 0); + + FieldDesc field = HandleToObject(fieldHandle); + Debug.Assert(field.IsStatic); + + if (!field.IsThreadStatic && field.IsInitOnly && field.HasRva && field is EcmaField ecmaField) + { + ReadOnlySpan rvaData = ecmaField.GetFieldRvaData(); + if (rvaData.Length >= bufferSize && valueOffset <= rvaData.Length - bufferSize) + { + rvaData.Slice(valueOffset, bufferSize).CopyTo(new Span(buffer, bufferSize)); + return true; + } + } return false; } diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index 09dd5cd1b1847..e31ba2189b486 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -14,7 +14,6 @@ using ILCompiler; using ILCompiler.DependencyAnalysis; using Internal.TypeSystem.Ecma; -using System.Drawing; #if SUPPORT_JIT using MethodCodeNode = Internal.Runtime.JitSupport.JitMethodCodeNode; @@ -2220,6 +2219,7 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt Debug.Assert(fieldHandle != null); Debug.Assert(buffer != null); Debug.Assert(bufferSize > 0); + Debug.Assert(valueOffset >= 0); FieldDesc field = HandleToObject(fieldHandle); Debug.Assert(field.IsStatic); @@ -2232,7 +2232,7 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt if (field is EcmaField ecmaField) { ReadOnlySpan rvaData = ecmaField.GetFieldRvaData(); - if (rvaData.Length >= bufferSize && valueOffset >= 0 && valueOffset <= rvaData.Length - bufferSize) + if (rvaData.Length >= bufferSize && valueOffset <= rvaData.Length - bufferSize) { rvaData.Slice(valueOffset, bufferSize).CopyTo(new Span(buffer, bufferSize)); return true; @@ -2241,8 +2241,6 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt return false; } - Debug.Assert(valueOffset == 0); // is only used for RVA at the moment - PreinitializationManager preinitManager = _compilation.NodeFactory.PreinitializationManager; if (preinitManager.IsPreinitialized(owningType)) { @@ -2253,6 +2251,7 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt if (value == null) { + Debug.Assert(valueOffset == 0); Debug.Assert(bufferSize == targetPtrSize); // Write "null" to buffer @@ -2265,13 +2264,15 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt switch (data) { case byte[] bytes: - Debug.Assert(bufferSize == bytes.Length); - - // Ensure we have enough room in the buffer, it can be a large struct - bytes.AsSpan().CopyTo(new Span(buffer, bufferSize)); - return true; + if (bytes.Length >= bufferSize && valueOffset <= bytes.Length - bufferSize) + { + bytes.AsSpan(valueOffset, bufferSize).CopyTo(new Span(buffer, bufferSize)); + return true; + } + return false; case FrozenObjectNode or FrozenStringNode: + Debug.Assert(valueOffset == 0); Debug.Assert(bufferSize == targetPtrSize); // save handle's value to buffer diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index ac4a58f7cf7df..a1dd582f55420 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -11968,6 +11968,7 @@ bool CEEInfo::getReadonlyStaticFieldValue(CORINFO_FIELD_HANDLE fieldHnd, uint8_t _ASSERT(fieldHnd != NULL); _ASSERT(buffer != NULL); _ASSERT(bufferSize > 0); + _ASSERT(valueOffset >= 0); bool result = false; From 66e481c46b2fd93b4c0fb91c9e60fd173a687cfd Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 26 Nov 2022 02:44:11 +0100 Subject: [PATCH 14/19] fix assert --- src/coreclr/jit/valuenum.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 6bc5138101a74..a9ba4bafeee4a 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8578,7 +8578,8 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) // ssize_t address = 0; FieldSeq* fieldSeq = nullptr; - if (fgGetFieldSeqAndAddress(tree->gtGetOp1(), &address, &fieldSeq)) + if (fgGetFieldSeqAndAddress(tree->gtGetOp1(), &address, &fieldSeq) && + (fieldSeq->GetKind() == FieldSeq::FieldKind::SimpleStaticKnownAddress)) { CORINFO_FIELD_HANDLE fieldHandle = fieldSeq->GetFieldHandle(); assert(fieldSeq->GetKind() == FieldSeq::FieldKind::SimpleStaticKnownAddress); From c4aa231a1291b6d6d3da42cf39b30587007dd4bb Mon Sep 17 00:00:00 2001 From: Egor Bogatov Date: Sat, 26 Nov 2022 11:46:48 +0100 Subject: [PATCH 15/19] Update src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs Co-authored-by: Jan Kotas --- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index 7b14ac346e2a7..ceefa72813d51 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -3008,6 +3008,9 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt FieldDesc field = HandleToObject(fieldHandle); Debug.Assert(field.IsStatic); + if (!_compilation.NodeFactory.CompilationModuleGroup.VersionsWithType(field.OwningType)) + return false; + if (!field.IsThreadStatic && field.IsInitOnly && field.HasRva && field is EcmaField ecmaField) { ReadOnlySpan rvaData = ecmaField.GetFieldRvaData(); From 8164730e1306b1024ab3e2ec8437df9f39097bb0 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Sat, 26 Nov 2022 11:56:11 +0100 Subject: [PATCH 16/19] Fix AOT --- src/coreclr/jit/assertionprop.cpp | 19 ++++++++++++++++--- src/coreclr/jit/codegencommon.cpp | 6 +++++- src/coreclr/jit/valuenum.cpp | 3 +-- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/assertionprop.cpp b/src/coreclr/jit/assertionprop.cpp index 30aaa8c5be1af..59cf0dcb2992d 100644 --- a/src/coreclr/jit/assertionprop.cpp +++ b/src/coreclr/jit/assertionprop.cpp @@ -3335,7 +3335,8 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, return nullptr; } - GenTree* newTree = tree; + GenTree* newTree = tree; + bool propagateType = false; // Update 'newTree' with the new value from our table // Typically newTree == tree and we are updating the node in place @@ -3364,9 +3365,16 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, case O2K_CONST_INT: // Don't propagate handles if we need to report relocs. - if (opts.compReloc && curAssertion->op2.HasIconFlag()) + if (opts.compReloc && curAssertion->op2.HasIconFlag() && curAssertion->op2.u1.iconVal != 0) { - return nullptr; + if (curAssertion->op2.GetIconFlag() == GTF_ICON_STATIC_HDL) + { + propagateType = true; + } + else + { + return nullptr; + } } // We assume that we do not try to do assertion prop on mismatched @@ -3394,6 +3402,11 @@ GenTree* Compiler::optConstantAssertionProp(AssertionDsc* curAssertion, // Conservatively don't allow propagation of ICON TYP_REF into BYREF return nullptr; } + propagateType = true; + } + + if (propagateType) + { newTree->ChangeType(tree->TypeGet()); } } diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp index 1601d9fd566c4..32d5ec4c934bc 100644 --- a/src/coreclr/jit/codegencommon.cpp +++ b/src/coreclr/jit/codegencommon.cpp @@ -1107,7 +1107,11 @@ bool CodeGen::genCreateAddrMode( if (op2->IsIntCnsFitsInI32() && (op2->gtType != TYP_REF) && FitsIn(cns + op2->AsIntConCommon()->IconValue())) { // We should not be building address modes out of non-foldable constants - assert(op2->AsIntConCommon()->ImmedValCanBeFolded(compiler, addr->OperGet())); + if (!op2->AsIntConCommon()->ImmedValCanBeFolded(compiler, addr->OperGet())) + { + assert(compiler->opts.compReloc); + return false; + } /* We're adding a constant */ diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index a9ba4bafeee4a..d0860f7636d77 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8582,12 +8582,11 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) (fieldSeq->GetKind() == FieldSeq::FieldKind::SimpleStaticKnownAddress)) { CORINFO_FIELD_HANDLE fieldHandle = fieldSeq->GetFieldHandle(); - assert(fieldSeq->GetKind() == FieldSeq::FieldKind::SimpleStaticKnownAddress); ssize_t byteOffset = address - fieldSeq->GetOffset(); int size = (int)genTypeSize(tree->TypeGet()); const int maxElementSize = sizeof(int64_t); - if ((size > 0) && (size <= maxElementSize) && (byteOffset >= 0) && (byteOffset < INT_MAX)) + if ((fieldHandle != nullptr) && (size > 0) && (size <= maxElementSize) && ((size_t)byteOffset < INT_MAX)) { uint8_t buffer[maxElementSize] = {0}; if (info.compCompHnd->getReadonlyStaticFieldValue(fieldHandle, (uint8_t*)&buffer, size, (int)byteOffset)) From d71881654d6ae2f09f8b4008e0963163ce5d2240 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 28 Nov 2022 12:15:14 +0100 Subject: [PATCH 17/19] Address feedback --- src/coreclr/jit/valuenum.cpp | 28 ++++++------------- .../tools/Common/JitInterface/CorInfoImpl.cs | 20 +++++++++++++ .../JitInterface/CorInfoImpl.ReadyToRun.cs | 18 ++---------- .../JitInterface/CorInfoImpl.RyuJit.cs | 11 +------- 4 files changed, 33 insertions(+), 44 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index d0860f7636d77..d861c3e5f394f 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8495,7 +8495,7 @@ void Compiler::fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl) } //---------------------------------------------------------------------------------- -// fgGetFieldSeqAndAddress: Try to obtain a constant address with a FieldSeq from the +// fgGetStaticFieldSeqAndAddress: Try to obtain a constant address with a FieldSeq from the // given tree. It can be either INT_CNS or e.g. ADD(INT_CNS, ADD(INT_CNS, INT_CNS)) // tree where only one of the constants is expected to have a field sequence. // @@ -8507,22 +8507,10 @@ void Compiler::fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl) // Return Value: // true if the pattern was recognized and a new VN is assigned // -static bool fgGetFieldSeqAndAddress(GenTree* tree, ssize_t* pAddress, FieldSeq** pFseq) +static bool fgGetStaticFieldSeqAndAddress(GenTree* tree, ssize_t* pAddress, FieldSeq** pFseq) { - if (tree->IsCnsIntOrI()) - { - ssize_t val = tree->AsIntCon()->IconValue(); - FieldSeq* fseq = tree->AsIntCon()->gtFieldSeq; - if (fseq != nullptr) - { - *pFseq = fseq; - *pAddress = val; - return true; - } - return false; - } - ssize_t val = 0; + // Accumulate final offset while (tree->OperIs(GT_ADD)) { GenTree* op1 = tree->gtGetOp1(); @@ -8539,12 +8527,14 @@ static bool fgGetFieldSeqAndAddress(GenTree* tree, ssize_t* pAddress, FieldSeq** } else { - // Unsupported tree + // We only inspect constants and additions return false; } } - if (tree->IsCnsIntOrI() && (tree->AsIntCon()->gtFieldSeq != nullptr)) + // Base address is expected to be static field's address + if ((tree->IsCnsIntOrI()) && (tree->AsIntCon()->gtFieldSeq != nullptr) && + (tree->AsIntCon()->gtFieldSeq->GetKind() == FieldSeq::FieldKind::SimpleStaticKnownAddress)) { *pFseq = tree->AsIntCon()->gtFieldSeq; *pAddress = tree->AsIntCon()->IconValue() + val; @@ -8578,9 +8568,9 @@ bool Compiler::fgValueNumberConstLoad(GenTreeIndir* tree) // ssize_t address = 0; FieldSeq* fieldSeq = nullptr; - if (fgGetFieldSeqAndAddress(tree->gtGetOp1(), &address, &fieldSeq) && - (fieldSeq->GetKind() == FieldSeq::FieldKind::SimpleStaticKnownAddress)) + if (fgGetStaticFieldSeqAndAddress(tree->gtGetOp1(), &address, &fieldSeq)) { + assert(fieldSeq->GetKind() == FieldSeq::FieldKind::SimpleStaticKnownAddress); CORINFO_FIELD_HANDLE fieldHandle = fieldSeq->GetFieldHandle(); ssize_t byteOffset = address - fieldSeq->GetOffset(); diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 87eacb8f9c418..2c605d72ae424 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -4079,5 +4079,25 @@ private bool notifyInstructionSetUsage(InstructionSet instructionSet, bool suppo return supportEnabled ? _compilation.InstructionSetSupport.IsInstructionSetSupported(instructionSet) : false; } #endif + + private bool tryReadRvaFieldData(FieldDesc field, byte* buffer, int bufferSize, int valueOffset) + { + Debug.Assert(buffer != null); + Debug.Assert(bufferSize > 0); + Debug.Assert(valueOffset >= 0); + Debug.Assert(field.IsStatic); + Debug.Assert(field.HasRva); + + if (!field.IsThreadStatic && field.IsInitOnly && field is EcmaField ecmaField) + { + ReadOnlySpan rvaData = ecmaField.GetFieldRvaData(); + if (rvaData.Length >= bufferSize && valueOffset <= rvaData.Length - bufferSize) + { + rvaData.Slice(valueOffset, bufferSize).CopyTo(new Span(buffer, bufferSize)); + return true; + } + } + return false; + } } } diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index ceefa72813d51..fa5f62e8e909e 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -3001,24 +3001,12 @@ private int getExactClasses(CORINFO_CLASS_STRUCT_* baseType, int maxExactClasses private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byte* buffer, int bufferSize, int valueOffset, bool ignoreMovableObjects) { Debug.Assert(fieldHandle != null); - Debug.Assert(buffer != null); - Debug.Assert(bufferSize > 0); - Debug.Assert(valueOffset >= 0); - FieldDesc field = HandleToObject(fieldHandle); - Debug.Assert(field.IsStatic); - - if (!_compilation.NodeFactory.CompilationModuleGroup.VersionsWithType(field.OwningType)) - return false; - if (!field.IsThreadStatic && field.IsInitOnly && field.HasRva && field is EcmaField ecmaField) + // For crossgen2 we only support RVA fields + if (_compilation.NodeFactory.CompilationModuleGroup.VersionsWithType(field.OwningType) && field.HasRva) { - ReadOnlySpan rvaData = ecmaField.GetFieldRvaData(); - if (rvaData.Length >= bufferSize && valueOffset <= rvaData.Length - bufferSize) - { - rvaData.Slice(valueOffset, bufferSize).CopyTo(new Span(buffer, bufferSize)); - return true; - } + return tryReadRvaFieldData(field, buffer, bufferSize, valueOffset); } return false; } diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index e31ba2189b486..c8e749dfc1f30 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -2229,16 +2229,7 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt { if (field.HasRva) { - if (field is EcmaField ecmaField) - { - ReadOnlySpan rvaData = ecmaField.GetFieldRvaData(); - if (rvaData.Length >= bufferSize && valueOffset <= rvaData.Length - bufferSize) - { - rvaData.Slice(valueOffset, bufferSize).CopyTo(new Span(buffer, bufferSize)); - return true; - } - } - return false; + return tryReadRvaFieldData(field, buffer, bufferSize, valueOffset); } PreinitializationManager preinitManager = _compilation.NodeFactory.PreinitializationManager; From f8051a889ac0ea39059152775231cc6c7a03cd38 Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 28 Nov 2022 13:09:44 +0100 Subject: [PATCH 18/19] make tryReadRvaFieldData static --- src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index 2c605d72ae424..b55397e7011c3 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -4080,7 +4080,7 @@ private bool notifyInstructionSetUsage(InstructionSet instructionSet, bool suppo } #endif - private bool tryReadRvaFieldData(FieldDesc field, byte* buffer, int bufferSize, int valueOffset) + private static bool tryReadRvaFieldData(FieldDesc field, byte* buffer, int bufferSize, int valueOffset) { Debug.Assert(buffer != null); Debug.Assert(bufferSize > 0); From f45a76ed34e408e9b5912ff04a60c65f7368ae1a Mon Sep 17 00:00:00 2001 From: EgorBo Date: Mon, 28 Nov 2022 16:30:04 +0100 Subject: [PATCH 19/19] address feedback --- src/coreclr/jit/valuenum.cpp | 2 +- src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs | 2 +- .../JitInterface/CorInfoImpl.ReadyToRun.cs | 2 +- .../aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index d861c3e5f394f..2ba4bd7f8c0af 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -8505,7 +8505,7 @@ void Compiler::fgValueNumberSsaVarDef(GenTreeLclVarCommon* lcl) // pFseq - [Out] field sequence // // Return Value: -// true if the pattern was recognized and a new VN is assigned +// true if the given tree is a static field address // static bool fgGetStaticFieldSeqAndAddress(GenTree* tree, ssize_t* pAddress, FieldSeq** pFseq) { diff --git a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs index b55397e7011c3..fa33a55b7098d 100644 --- a/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs +++ b/src/coreclr/tools/Common/JitInterface/CorInfoImpl.cs @@ -4080,7 +4080,7 @@ private bool notifyInstructionSetUsage(InstructionSet instructionSet, bool suppo } #endif - private static bool tryReadRvaFieldData(FieldDesc field, byte* buffer, int bufferSize, int valueOffset) + private static bool TryReadRvaFieldData(FieldDesc field, byte* buffer, int bufferSize, int valueOffset) { Debug.Assert(buffer != null); Debug.Assert(bufferSize > 0); diff --git a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs index fa5f62e8e909e..263815aa54574 100644 --- a/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs +++ b/src/coreclr/tools/aot/ILCompiler.ReadyToRun/JitInterface/CorInfoImpl.ReadyToRun.cs @@ -3006,7 +3006,7 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt // For crossgen2 we only support RVA fields if (_compilation.NodeFactory.CompilationModuleGroup.VersionsWithType(field.OwningType) && field.HasRva) { - return tryReadRvaFieldData(field, buffer, bufferSize, valueOffset); + return TryReadRvaFieldData(field, buffer, bufferSize, valueOffset); } return false; } diff --git a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs index c8e749dfc1f30..55c08d549de0b 100644 --- a/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs +++ b/src/coreclr/tools/aot/ILCompiler.RyuJit/JitInterface/CorInfoImpl.RyuJit.cs @@ -2229,7 +2229,7 @@ private bool getReadonlyStaticFieldValue(CORINFO_FIELD_STRUCT_* fieldHandle, byt { if (field.HasRva) { - return tryReadRvaFieldData(field, buffer, bufferSize, valueOffset); + return TryReadRvaFieldData(field, buffer, bufferSize, valueOffset); } PreinitializationManager preinitManager = _compilation.NodeFactory.PreinitializationManager;