From eea057675de7914af12c6168db0f3bebea35be5b Mon Sep 17 00:00:00 2001 From: Eugene Rozenfeld Date: Sun, 16 Dec 2018 12:07:42 -0800 Subject: Improve removal of dead calls to allocator helpers. This change improves detection of allocators with side effects. Allocators can cause side effects if the allocated object may have a finalizer. This change adds a pHasSideEffects parameter to getNewHelper JitEE interface method. It's used by the jit to check for allocator side effects instead of guessing from helper ids. Fixes #21530. --- .../superpmi/superpmi-shared/icorjitinfoimpl.h | 2 +- src/ToolBox/superpmi/superpmi-shared/lwmlist.h | 2 +- .../superpmi/superpmi-shared/methodcontext.cpp | 30 +++- .../superpmi/superpmi-shared/methodcontext.h | 5 +- .../superpmi-shim-collector/icorjitinfo.cpp | 7 +- .../superpmi/superpmi-shim-counter/icorjitinfo.cpp | 5 +- .../superpmi/superpmi-shim-simple/icorjitinfo.cpp | 5 +- src/ToolBox/superpmi/superpmi/icorjitinfo.cpp | 4 +- src/inc/corinfo.h | 13 +- src/jit/ICorJitInfo_API_wrapper.hpp | 5 +- src/jit/compiler.h | 3 +- src/jit/compiler.hpp | 17 +- src/jit/gentree.cpp | 19 ++- src/jit/gentree.h | 6 +- src/jit/objectalloc.cpp | 9 +- src/jit/utils.cpp | 24 +-- src/jit/utils.h | 8 - src/vm/jitinterface.cpp | 30 +++- src/vm/jitinterface.h | 4 +- src/vm/methodtable.cpp | 26 +++ src/vm/methodtable.h | 5 + src/zap/zapinfo.cpp | 184 +++++++++++---------- src/zap/zapinfo.h | 2 +- 23 files changed, 238 insertions(+), 177 deletions(-) (limited to 'src') diff --git a/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h b/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h index b6a66a3e06..223e1d89c7 100644 --- a/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h +++ b/src/ToolBox/superpmi/superpmi-shared/icorjitinfoimpl.h @@ -360,7 +360,7 @@ CORINFO_FIELD_HANDLE getFieldInClass(CORINFO_CLASS_HANDLE clsHnd, INT num); BOOL checkMethodModifier(CORINFO_METHOD_HANDLE hMethod, LPCSTR modifier, BOOL fOptional); // returns the "NEW" helper optimized for "newCls." -CorInfoHelpFunc getNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle); +CorInfoHelpFunc getNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, bool* pHasSideEffects = NULL /* OUT */); // returns the newArr (1-Dim array) helper optimized for "arrayCls." CorInfoHelpFunc getNewArrHelper(CORINFO_CLASS_HANDLE arrayCls); diff --git a/src/ToolBox/superpmi/superpmi-shared/lwmlist.h b/src/ToolBox/superpmi/superpmi-shared/lwmlist.h index 137fbe111d..d034bdcd8f 100644 --- a/src/ToolBox/superpmi/superpmi-shared/lwmlist.h +++ b/src/ToolBox/superpmi/superpmi-shared/lwmlist.h @@ -114,7 +114,7 @@ LWM(GetMethodSig, DLDL, Agnostic_CORINFO_SIG_INFO) LWM(GetMethodSync, DWORDLONG, DLDL) LWM(GetMethodVTableOffset, DWORDLONG, DDD) LWM(GetNewArrHelper, DWORDLONG, DWORD) -LWM(GetNewHelper, Agnostic_GetNewHelper, DWORD) +LWM(GetNewHelper, Agnostic_GetNewHelper, DD) LWM(GetParentType, DWORDLONG, DWORDLONG) LWM(GetPInvokeUnmanagedTarget, DWORDLONG, DLDL) LWM(GetProfilingHandle, DWORD, Agnostic_GetProfilingHandle) diff --git a/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp b/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp index 4883dd4f38..5f4861d544 100644 --- a/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/ToolBox/superpmi/superpmi-shared/methodcontext.cpp @@ -2900,10 +2900,11 @@ bool MethodContext::repGetMethodInfo(CORINFO_METHOD_HANDLE ftn, CORINFO_METHOD_I void MethodContext::recGetNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, + bool* pHasSideEffects, CorInfoHelpFunc result) { if (GetNewHelper == nullptr) - GetNewHelper = new LightWeightMap(); + GetNewHelper = new LightWeightMap(); Agnostic_GetNewHelper key; ZeroMemory(&key, sizeof(Agnostic_GetNewHelper)); // We use the input structs as a key and use memcmp to compare.. so @@ -2911,15 +2912,20 @@ void MethodContext::recGetNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, key.hClass = (DWORDLONG)pResolvedToken->hClass; key.callerHandle = (DWORDLONG)callerHandle; - GetNewHelper->Add(key, (DWORD)result); - DEBUG_REC(dmpGetNewHelper(key, (DWORD)result)); + DD value; + value.A = (pHasSideEffects != nullptr) ? (DWORD)(*pHasSideEffects ? 1 : 0) : (DWORD)0; + value.B = (DWORD)result; + + GetNewHelper->Add(key, value); + DEBUG_REC(dmpGetNewHelper(key, value)); } -void MethodContext::dmpGetNewHelper(const Agnostic_GetNewHelper& key, DWORD value) +void MethodContext::dmpGetNewHelper(const Agnostic_GetNewHelper& key, DD value) { - printf("GetNewHelper key cls-%016llX chan-%016llX, value res-%u", key.hClass, key.callerHandle, value); + printf("GetNewHelper key cls-%016llX chan-%016llX, hasSideEffects-%u, value res-%u", key.hClass, key.callerHandle, value.A, value.B); } CorInfoHelpFunc MethodContext::repGetNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, - CORINFO_METHOD_HANDLE callerHandle) + CORINFO_METHOD_HANDLE callerHandle, + bool* pHasSideEffects) { Agnostic_GetNewHelper key; ZeroMemory(&key, sizeof(Agnostic_GetNewHelper)); // We use the input structs as a key and use memcmp to compare.. so @@ -2929,9 +2935,17 @@ CorInfoHelpFunc MethodContext::repGetNewHelper(CORINFO_RESOLVED_TOKEN* pResolved AssertCodeMsg(GetNewHelper != nullptr, EXCEPTIONCODE_MC, "Didn't find anything for %016llX", (DWORDLONG)key.hClass); AssertCodeMsg(GetNewHelper->GetIndex(key) != -1, EXCEPTIONCODE_MC, "Didn't find %016llX", (DWORDLONG)key.hClass); - CorInfoHelpFunc value = (CorInfoHelpFunc)GetNewHelper->Get(key); + + DD value; + value = GetNewHelper->Get(key); + if (pHasSideEffects != nullptr) + { + *pHasSideEffects = (value.A == 0) ? false : true; + } + CorInfoHelpFunc result = (CorInfoHelpFunc)value.B; + DEBUG_REP(dmpGetNewHelper(key, value)); - return value; + return result; } void MethodContext::recEmbedGenericHandle(CORINFO_RESOLVED_TOKEN* pResolvedToken, diff --git a/src/ToolBox/superpmi/superpmi-shared/methodcontext.h b/src/ToolBox/superpmi/superpmi-shared/methodcontext.h index 115cd4182d..7e20c2bc1a 100644 --- a/src/ToolBox/superpmi/superpmi-shared/methodcontext.h +++ b/src/ToolBox/superpmi/superpmi-shared/methodcontext.h @@ -878,9 +878,10 @@ public: void recGetNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, + bool* pHasSideEffects, CorInfoHelpFunc result); - void dmpGetNewHelper(const Agnostic_GetNewHelper& key, DWORD value); - CorInfoHelpFunc repGetNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle); + void dmpGetNewHelper(const Agnostic_GetNewHelper& key, DD value); + CorInfoHelpFunc repGetNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, bool * pHasSideEffects); void recEmbedGenericHandle(CORINFO_RESOLVED_TOKEN* pResolvedToken, BOOL fEmbedParent, diff --git a/src/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp index 898f8241cb..44b08dfe71 100644 --- a/src/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp +++ b/src/ToolBox/superpmi/superpmi-shim-collector/icorjitinfo.cpp @@ -768,11 +768,12 @@ BOOL interceptor_ICJI::checkMethodModifier(CORINFO_METHOD_HANDLE hMethod, LPCSTR // returns the "NEW" helper optimized for "newCls." CorInfoHelpFunc interceptor_ICJI::getNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, - CORINFO_METHOD_HANDLE callerHandle) + CORINFO_METHOD_HANDLE callerHandle, + bool* pHasSideEffects) { mc->cr->AddCall("getNewHelper"); - CorInfoHelpFunc temp = original_ICorJitInfo->getNewHelper(pResolvedToken, callerHandle); - mc->recGetNewHelper(pResolvedToken, callerHandle, temp); + CorInfoHelpFunc temp = original_ICorJitInfo->getNewHelper(pResolvedToken, callerHandle, pHasSideEffects); + mc->recGetNewHelper(pResolvedToken, callerHandle, pHasSideEffects, temp); return temp; } diff --git a/src/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp index 3348479a85..5fe6bc367b 100644 --- a/src/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp +++ b/src/ToolBox/superpmi/superpmi-shim-counter/icorjitinfo.cpp @@ -587,10 +587,11 @@ BOOL interceptor_ICJI::checkMethodModifier(CORINFO_METHOD_HANDLE hMethod, LPCSTR // returns the "NEW" helper optimized for "newCls." CorInfoHelpFunc interceptor_ICJI::getNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, - CORINFO_METHOD_HANDLE callerHandle) + CORINFO_METHOD_HANDLE callerHandle, + bool* pHasSideEffects) { mcs->AddCall("getNewHelper"); - return original_ICorJitInfo->getNewHelper(pResolvedToken, callerHandle); + return original_ICorJitInfo->getNewHelper(pResolvedToken, callerHandle, pHasSideEffects); } // returns the newArr (1-Dim array) helper optimized for "arrayCls." diff --git a/src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp index b5a18f855c..0455503658 100644 --- a/src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp +++ b/src/ToolBox/superpmi/superpmi-shim-simple/icorjitinfo.cpp @@ -524,9 +524,10 @@ BOOL interceptor_ICJI::checkMethodModifier(CORINFO_METHOD_HANDLE hMethod, LPCSTR // returns the "NEW" helper optimized for "newCls." CorInfoHelpFunc interceptor_ICJI::getNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, - CORINFO_METHOD_HANDLE callerHandle) + CORINFO_METHOD_HANDLE callerHandle, + bool* pHasSideEffects) { - return original_ICorJitInfo->getNewHelper(pResolvedToken, callerHandle); + return original_ICorJitInfo->getNewHelper(pResolvedToken, callerHandle, pHasSideEffects); } // returns the newArr (1-Dim array) helper optimized for "arrayCls." diff --git a/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp b/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp index bf60a9a0bd..bca4cb04fa 100644 --- a/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp +++ b/src/ToolBox/superpmi/superpmi/icorjitinfo.cpp @@ -644,10 +644,10 @@ BOOL MyICJI::checkMethodModifier(CORINFO_METHOD_HANDLE hMethod, LPCSTR modifier, } // returns the "NEW" helper optimized for "newCls." -CorInfoHelpFunc MyICJI::getNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle) +CorInfoHelpFunc MyICJI::getNewHelper(CORINFO_RESOLVED_TOKEN* pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, bool * pHasSideEffects) { jitInstance->mc->cr->AddCall("getNewHelper"); - return jitInstance->mc->repGetNewHelper(pResolvedToken, callerHandle); + return jitInstance->mc->repGetNewHelper(pResolvedToken, callerHandle, pHasSideEffects); } // returns the newArr (1-Dim array) helper optimized for "arrayCls." diff --git a/src/inc/corinfo.h b/src/inc/corinfo.h index f342b2d219..852bf3168b 100644 --- a/src/inc/corinfo.h +++ b/src/inc/corinfo.h @@ -213,11 +213,11 @@ TODO: Talk about initializing strutures before use #define SELECTANY extern __declspec(selectany) #endif -SELECTANY const GUID JITEEVersionIdentifier = { /* {0BA24443-F3E0-453E-BE58-039CC4510F39} */ - 0xba24443, - 0xf3e0, - 0x453e, - {0xbe, 0x58, 0x3, 0x9c, 0xc4, 0x51, 0xf, 0x39} +SELECTANY const GUID JITEEVersionIdentifier = { /* 8903fe7b-a82a-4e2e-b691-f58430b485d1 */ + 0x8903fe7b, + 0xa82a, + 0x4e2e, + {0xb6, 0x91, 0xf5, 0x84, 0x30, 0xb4, 0x85, 0xd1} }; ////////////////////////////////////////////////////////////////////////////////////////////////////////// @@ -2450,7 +2450,8 @@ public: // returns the "NEW" helper optimized for "newCls." virtual CorInfoHelpFunc getNewHelper( CORINFO_RESOLVED_TOKEN * pResolvedToken, - CORINFO_METHOD_HANDLE callerHandle + CORINFO_METHOD_HANDLE callerHandle, + bool * pHasSideEffects = NULL /* OUT */ ) = 0; // returns the newArr (1-Dim array) helper optimized for "arrayCls." diff --git a/src/jit/ICorJitInfo_API_wrapper.hpp b/src/jit/ICorJitInfo_API_wrapper.hpp index 8a9abb4437..e34ea9539c 100644 --- a/src/jit/ICorJitInfo_API_wrapper.hpp +++ b/src/jit/ICorJitInfo_API_wrapper.hpp @@ -557,10 +557,11 @@ BOOL WrapICorJitInfo::checkMethodModifier( CorInfoHelpFunc WrapICorJitInfo::getNewHelper( CORINFO_RESOLVED_TOKEN * pResolvedToken, - CORINFO_METHOD_HANDLE callerHandle) + CORINFO_METHOD_HANDLE callerHandle, + bool * pHasSideEffects) { API_ENTER(getNewHelper); - CorInfoHelpFunc temp = wrapHnd->getNewHelper(pResolvedToken, callerHandle); + CorInfoHelpFunc temp = wrapHnd->getNewHelper(pResolvedToken, callerHandle, pHasSideEffects); API_LEAVE(getNewHelper); return temp; } diff --git a/src/jit/compiler.h b/src/jit/compiler.h index 3b727741e4..91e294a97c 100644 --- a/src/jit/compiler.h +++ b/src/jit/compiler.h @@ -2465,7 +2465,8 @@ public: GenTreeCast* gtNewCastNodeL(var_types typ, GenTree* op1, bool fromUnsigned, var_types castType); - GenTreeAllocObj* gtNewAllocObjNode(unsigned int helper, CORINFO_CLASS_HANDLE clsHnd, var_types type, GenTree* op1); + GenTreeAllocObj* gtNewAllocObjNode( + unsigned int helper, bool helperHasSideEffects, CORINFO_CLASS_HANDLE clsHnd, var_types type, GenTree* op1); GenTreeAllocObj* gtNewAllocObjNode(CORINFO_RESOLVED_TOKEN* pResolvedToken, BOOL useParent); diff --git a/src/jit/compiler.hpp b/src/jit/compiler.hpp index 29a79f862b..3fceff729a 100644 --- a/src/jit/compiler.hpp +++ b/src/jit/compiler.hpp @@ -1157,21 +1157,20 @@ inline GenTreeCall* Compiler::gtNewHelperCallNode(unsigned helper, var_types typ // gtNewAllocObjNode: A little helper to create an object allocation node. // // Arguments: -// helper - Value returned by ICorJitInfo::getNewHelper -// clsHnd - Corresponding class handle -// type - Tree return type (e.g. TYP_REF) -// op1 - Node containing an address of VtablePtr +// helper - Value returned by ICorJitInfo::getNewHelper +// helperHasSideEffects - True iff allocation helper has side effects +// clsHnd - Corresponding class handle +// type - Tree return type (e.g. TYP_REF) +// op1 - Node containing an address of VtablePtr // // Return Value: // Returns GT_ALLOCOBJ node that will be later morphed into an // allocation helper call or local variable allocation on the stack. -inline GenTreeAllocObj* Compiler::gtNewAllocObjNode(unsigned int helper, - CORINFO_CLASS_HANDLE clsHnd, - var_types type, - GenTree* op1) +inline GenTreeAllocObj* Compiler::gtNewAllocObjNode( + unsigned int helper, bool helperHasSideEffects, CORINFO_CLASS_HANDLE clsHnd, var_types type, GenTree* op1) { - GenTreeAllocObj* node = new (this, GT_ALLOCOBJ) GenTreeAllocObj(type, helper, clsHnd, op1); + GenTreeAllocObj* node = new (this, GT_ALLOCOBJ) GenTreeAllocObj(type, helper, helperHasSideEffects, clsHnd, op1); return node; } diff --git a/src/jit/gentree.cpp b/src/jit/gentree.cpp index f6b6dc8107..9fe1718ad7 100644 --- a/src/jit/gentree.cpp +++ b/src/jit/gentree.cpp @@ -925,7 +925,7 @@ bool GenTreeCall::HasSideEffects(Compiler* compiler, bool ignoreExceptions, bool // If this is not a Pure helper call or an allocator (that will not need to run a finalizer) // then this call has side effects. return !helperProperties.IsPure(helper) && - (!helperProperties.IsAllocator(helper) || helperProperties.MayFinalize(helper)); + (!helperProperties.IsAllocator(helper) || ((gtCallMoreFlags & GTF_CALL_M_ALLOC_SIDE_EFFECTS) != 0)); } //------------------------------------------------------------------------- @@ -6724,8 +6724,15 @@ GenTreeAllocObj* Compiler::gtNewAllocObjNode(CORINFO_RESOLVED_TOKEN* pResolvedTo assert(compDonotInline()); return nullptr; } + } + + bool helperHasSideEffects; + CorInfoHelpFunc helperTemp = + info.compCompHnd->getNewHelper(pResolvedToken, info.compMethodHnd, &helperHasSideEffects); - helper = info.compCompHnd->getNewHelper(pResolvedToken, info.compMethodHnd); + if (!usingReadyToRunHelper) + { + helper = helperTemp; } // TODO: ReadyToRun: When generic dictionary lookups are necessary, replace the lookup call @@ -6735,7 +6742,8 @@ GenTreeAllocObj* Compiler::gtNewAllocObjNode(CORINFO_RESOLVED_TOKEN* pResolvedTo // 3) Allocate and return the new object for boxing // Reason: performance (today, we'll always use the slow helper for the R2R generics case) - GenTreeAllocObj* allocObj = gtNewAllocObjNode(helper, pResolvedToken->hClass, TYP_REF, opHandle); + GenTreeAllocObj* allocObj = + gtNewAllocObjNode(helper, helperHasSideEffects, pResolvedToken->hClass, TYP_REF, opHandle); #ifdef FEATURE_READYTORUN_COMPILER if (usingReadyToRunHelper) @@ -7134,8 +7142,9 @@ GenTree* Compiler::gtCloneExpr( case GT_ALLOCOBJ: { GenTreeAllocObj* asAllocObj = tree->AsAllocObj(); - copy = new (this, GT_ALLOCOBJ) GenTreeAllocObj(tree->TypeGet(), asAllocObj->gtNewHelper, - asAllocObj->gtAllocObjClsHnd, asAllocObj->gtOp1); + copy = new (this, GT_ALLOCOBJ) + GenTreeAllocObj(tree->TypeGet(), asAllocObj->gtNewHelper, asAllocObj->gtHelperHasSideEffects, + asAllocObj->gtAllocObjClsHnd, asAllocObj->gtOp1); } break; diff --git a/src/jit/gentree.h b/src/jit/gentree.h index d2734d7067..9846ceab09 100644 --- a/src/jit/gentree.h +++ b/src/jit/gentree.h @@ -3490,6 +3490,7 @@ struct GenTreeCall final : public GenTree #define GTF_CALL_M_UNBOXED 0x00080000 // GT_CALL -- this call was optimized to use the unboxed entry point #define GTF_CALL_M_GUARDED_DEVIRT 0x00100000 // GT_CALL -- this call is a candidate for guarded devirtualization #define GTF_CALL_M_GUARDED 0x00200000 // GT_CALL -- this call was transformed by guarded devirtualization +#define GTF_CALL_M_ALLOC_SIDE_EFFECTS 0x00400000 // GT_CALL -- this is a call to an allocator with side effects // clang-format on @@ -5560,15 +5561,18 @@ struct GenTreeCopyOrReload : public GenTreeUnOp struct GenTreeAllocObj final : public GenTreeUnOp { unsigned int gtNewHelper; // Value returned by ICorJitInfo::getNewHelper + bool gtHelperHasSideEffects; CORINFO_CLASS_HANDLE gtAllocObjClsHnd; #ifdef FEATURE_READYTORUN_COMPILER CORINFO_CONST_LOOKUP gtEntryPoint; #endif - GenTreeAllocObj(var_types type, unsigned int helper, CORINFO_CLASS_HANDLE clsHnd, GenTree* op) + GenTreeAllocObj( + var_types type, unsigned int helper, bool helperHasSideEffects, CORINFO_CLASS_HANDLE clsHnd, GenTree* op) : GenTreeUnOp(GT_ALLOCOBJ, type, op DEBUGARG(/*largeNode*/ TRUE)) , // This node in most cases will be changed to a call node gtNewHelper(helper) + , gtHelperHasSideEffects(helperHasSideEffects) , gtAllocObjClsHnd(clsHnd) { #ifdef FEATURE_READYTORUN_COMPILER diff --git a/src/jit/objectalloc.cpp b/src/jit/objectalloc.cpp index 736181e115..7b9fac8aa9 100644 --- a/src/jit/objectalloc.cpp +++ b/src/jit/objectalloc.cpp @@ -371,8 +371,9 @@ GenTree* ObjectAllocator::MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* alloc { assert(allocObj != nullptr); - GenTree* op1 = allocObj->gtGetOp1(); - unsigned int helper = allocObj->gtNewHelper; + GenTree* op1 = allocObj->gtGetOp1(); + unsigned int helper = allocObj->gtNewHelper; + bool helperHasSideEffects = allocObj->gtHelperHasSideEffects; GenTreeArgList* args; #ifdef FEATURE_READYTORUN_COMPILER @@ -389,6 +390,10 @@ GenTree* ObjectAllocator::MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* alloc const bool morphArgs = false; GenTree* helperCall = comp->fgMorphIntoHelperCall(allocObj, allocObj->gtNewHelper, args, morphArgs); + if (helperHasSideEffects) + { + helperCall->gtCall.gtCallMoreFlags |= GTF_CALL_M_ALLOC_SIDE_EFFECTS; + } #ifdef FEATURE_READYTORUN_COMPILER if (entryPoint.addr != nullptr) diff --git a/src/jit/utils.cpp b/src/jit/utils.cpp index 8bc85961fd..ae7dd60ad4 100644 --- a/src/jit/utils.cpp +++ b/src/jit/utils.cpp @@ -1170,7 +1170,6 @@ void HelperCallProperties::init() bool isAllocator = false; // true if the result is usually a newly created heap item, or may throw OutOfMemory bool mutatesHeap = false; // true if any previous heap objects [are|can be] modified bool mayRunCctor = false; // true if the helper call may cause a static constructor to be run. - bool mayFinalize = false; // true if the helper call allocates an object that may need to run a finalizer switch (helper) { @@ -1228,19 +1227,13 @@ void HelperCallProperties::init() case CORINFO_HELP_NEWSFAST: case CORINFO_HELP_NEWSFAST_ALIGN8: case CORINFO_HELP_NEWSFAST_ALIGN8_VC: - - isAllocator = true; - nonNullReturn = true; - noThrow = true; // only can throw OutOfMemory - break; - case CORINFO_HELP_NEW_CROSSCONTEXT: case CORINFO_HELP_NEWFAST: case CORINFO_HELP_NEWSFAST_FINALIZE: case CORINFO_HELP_NEWSFAST_ALIGN8_FINALIZE: case CORINFO_HELP_READYTORUN_NEW: + case CORINFO_HELP_BOX: - mayFinalize = true; // These may run a finalizer isAllocator = true; nonNullReturn = true; noThrow = true; // only can throw OutOfMemory @@ -1250,20 +1243,12 @@ void HelperCallProperties::init() // and can throw exceptions other than OOM. case CORINFO_HELP_NEWARR_1_VC: case CORINFO_HELP_NEWARR_1_ALIGN8: - - isAllocator = true; - nonNullReturn = true; - break; - - // These allocation helpers do some checks on the size (and lower bound) inputs, - // and can throw exceptions other than OOM. case CORINFO_HELP_NEW_MDARR: case CORINFO_HELP_NEWARR_1_DIRECT: case CORINFO_HELP_NEWARR_1_OBJ: case CORINFO_HELP_NEWARR_1_R2R_DIRECT: case CORINFO_HELP_READYTORUN_NEWARR_1: - mayFinalize = true; // These may run a finalizer isAllocator = true; nonNullReturn = true; break; @@ -1277,12 +1262,6 @@ void HelperCallProperties::init() noThrow = true; // only can throw OutOfMemory break; - case CORINFO_HELP_BOX: - nonNullReturn = true; - isAllocator = true; - noThrow = true; // only can throw OutOfMemory - break; - case CORINFO_HELP_BOX_NULLABLE: // Box Nullable is not a 'pure' function // It has a Byref argument that it reads the contents of. @@ -1481,7 +1460,6 @@ void HelperCallProperties::init() m_isAllocator[helper] = isAllocator; m_mutatesHeap[helper] = mutatesHeap; m_mayRunCctor[helper] = mayRunCctor; - m_mayFinalize[helper] = mayFinalize; } } diff --git a/src/jit/utils.h b/src/jit/utils.h index fb6e3459ee..ec3b0e3e32 100644 --- a/src/jit/utils.h +++ b/src/jit/utils.h @@ -458,7 +458,6 @@ private: bool m_isAllocator[CORINFO_HELP_COUNT]; bool m_mutatesHeap[CORINFO_HELP_COUNT]; bool m_mayRunCctor[CORINFO_HELP_COUNT]; - bool m_mayFinalize[CORINFO_HELP_COUNT]; void init(); @@ -509,13 +508,6 @@ public: assert(helperId < CORINFO_HELP_COUNT); return m_mayRunCctor[helperId]; } - - bool MayFinalize(CorInfoHelpFunc helperId) - { - assert(helperId > CORINFO_HELP_UNDEF); - assert(helperId < CORINFO_HELP_COUNT); - return m_mayFinalize[helperId]; - } }; //***************************************************************************** diff --git a/src/vm/jitinterface.cpp b/src/vm/jitinterface.cpp index 3e1765a0d7..0730f37641 100644 --- a/src/vm/jitinterface.cpp +++ b/src/vm/jitinterface.cpp @@ -5990,7 +5990,7 @@ bool __stdcall TrackAllocationsEnabled() } /***********************************************************************/ -CorInfoHelpFunc CEEInfo::getNewHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_METHOD_HANDLE callerHandle) +CorInfoHelpFunc CEEInfo::getNewHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, bool * pHasSideEffects) { CONTRACTL { SO_TOLERANT; @@ -6016,7 +6016,7 @@ CorInfoHelpFunc CEEInfo::getNewHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, C } MethodTable* pMT = VMClsHnd.AsMethodTable(); - result = getNewHelperStatic(pMT); + result = getNewHelperStatic(pMT, pHasSideEffects); _ASSERTE(result != CORINFO_HELP_UNDEF); @@ -6026,23 +6026,43 @@ CorInfoHelpFunc CEEInfo::getNewHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, C } /***********************************************************************/ -CorInfoHelpFunc CEEInfo::getNewHelperStatic(MethodTable * pMT) +CorInfoHelpFunc CEEInfo::getNewHelperStatic(MethodTable * pMT, bool * pHasSideEffects) { STANDARD_VM_CONTRACT; // Slow helper is the default CorInfoHelpFunc helper = CORINFO_HELP_NEWFAST; + bool hasFinalizer = pMT->HasFinalizer(); + bool isComObjectType = pMT->IsComObjectType(); + if (pHasSideEffects != nullptr) + { + if (isComObjectType) + { + *pHasSideEffects = true; + } + else +#ifdef FEATURE_READYTORUN_COMPILER + if (IsReadyToRunCompilation()) + { + *pHasSideEffects = hasFinalizer || !pMT->IsInheritanceChainFixedInCurrentVersionBubble(); + } + else +#endif + { + *pHasSideEffects = hasFinalizer; + } + } - if (pMT->IsComObjectType()) + if (isComObjectType) { // Use slow helper _ASSERTE(helper == CORINFO_HELP_NEWFAST); } else if ((pMT->GetBaseSize() >= LARGE_OBJECT_SIZE) || - pMT->HasFinalizer()) + hasFinalizer) { // Use slow helper _ASSERTE(helper == CORINFO_HELP_NEWFAST); diff --git a/src/vm/jitinterface.h b/src/vm/jitinterface.h index 4c23de9b51..1a949fec31 100644 --- a/src/vm/jitinterface.h +++ b/src/vm/jitinterface.h @@ -516,8 +516,8 @@ public: // considered when checking visibility rules. - CorInfoHelpFunc getNewHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_METHOD_HANDLE callerHandle); - static CorInfoHelpFunc getNewHelperStatic(MethodTable * pMT); + CorInfoHelpFunc getNewHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, bool * pHasSideEffects = NULL); + static CorInfoHelpFunc getNewHelperStatic(MethodTable * pMT, bool * pHasSideEffects = NULL); CorInfoHelpFunc getNewArrHelper(CORINFO_CLASS_HANDLE arrayCls); static CorInfoHelpFunc getNewArrHelperStatic(TypeHandle clsHnd); diff --git a/src/vm/methodtable.cpp b/src/vm/methodtable.cpp index 91f6edd258..d7e98ffa48 100644 --- a/src/vm/methodtable.cpp +++ b/src/vm/methodtable.cpp @@ -10236,4 +10236,30 @@ BOOL MethodTable::IsInheritanceChainLayoutFixedInCurrentVersionBubble() return TRUE; } + +// +// Is the inheritance chain fixed within the current version bubble? +// +BOOL MethodTable::IsInheritanceChainFixedInCurrentVersionBubble() +{ + STANDARD_VM_CONTRACT; + + MethodTable * pMT = this; + + if (pMT->IsValueType()) + { + return pMT->GetModule()->IsInCurrentVersionBubble(); + } + + while ((pMT != g_pObjectClass) && (pMT != NULL)) + { + if (!pMT->GetModule()->IsInCurrentVersionBubble()) + return FALSE; + + pMT = pMT->GetParentMethodTable(); + } + + return TRUE; +} + #endif // FEATURE_READYTORUN_COMPILER diff --git a/src/vm/methodtable.h b/src/vm/methodtable.h index 395d0502f8..bbdfc90623 100644 --- a/src/vm/methodtable.h +++ b/src/vm/methodtable.h @@ -4159,6 +4159,11 @@ public: // Is field layout of the inheritance chain fixed within the current version bubble? // BOOL IsInheritanceChainLayoutFixedInCurrentVersionBubble(); + + // + // Is the inheritance chain fixed within the current version bubble? + // + BOOL IsInheritanceChainFixedInCurrentVersionBubble(); #endif }; // class MethodTable diff --git a/src/zap/zapinfo.cpp b/src/zap/zapinfo.cpp index a37531c735..f34e1146d5 100644 --- a/src/zap/zapinfo.cpp +++ b/src/zap/zapinfo.cpp @@ -3390,149 +3390,151 @@ unsigned ZapInfo::getClassNumInstanceFields(CORINFO_CLASS_HANDLE cls) return m_pEEJitInfo->getClassNumInstanceFields(cls); } - -CorInfoHelpFunc ZapInfo::getNewHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_METHOD_HANDLE callerHandle) +CorInfoHelpFunc ZapInfo::getNewHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, bool * pHasSideEffects) { - if (IsReadyToRunCompilation()) - return CORINFO_HELP_NEWFAST; + if (!IsReadyToRunCompilation()) + { + classMustBeLoadedBeforeCodeIsRun(pResolvedToken->hClass); + } + + CorInfoHelpFunc helper = m_pEEJitInfo->getNewHelper(pResolvedToken, callerHandle, pHasSideEffects); - classMustBeLoadedBeforeCodeIsRun(pResolvedToken->hClass); - return m_pEEJitInfo->getNewHelper(pResolvedToken, callerHandle); + return IsReadyToRunCompilation() ? CORINFO_HELP_NEWFAST : helper; } CorInfoHelpFunc ZapInfo::getSharedCCtorHelper(CORINFO_CLASS_HANDLE clsHnd) { - return m_pEEJitInfo->getSharedCCtorHelper(clsHnd); + return m_pEEJitInfo->getSharedCCtorHelper(clsHnd); } CorInfoHelpFunc ZapInfo::getSecurityPrologHelper(CORINFO_METHOD_HANDLE ftn) { - return m_pEEJitInfo->getSecurityPrologHelper(ftn); + return m_pEEJitInfo->getSecurityPrologHelper(ftn); } CORINFO_CLASS_HANDLE ZapInfo::getTypeForBox(CORINFO_CLASS_HANDLE cls) { - return m_pEEJitInfo->getTypeForBox(cls); + return m_pEEJitInfo->getTypeForBox(cls); } CorInfoHelpFunc ZapInfo::getBoxHelper(CORINFO_CLASS_HANDLE cls) { - return m_pEEJitInfo->getBoxHelper(cls); + return m_pEEJitInfo->getBoxHelper(cls); } CorInfoHelpFunc ZapInfo::getUnBoxHelper(CORINFO_CLASS_HANDLE cls) { - return m_pEEJitInfo->getUnBoxHelper(cls); + return m_pEEJitInfo->getUnBoxHelper(cls); } CorInfoHelpFunc ZapInfo::getCastingHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, bool fThrowing) { - if (IsReadyToRunCompilation()) - return (fThrowing ? CORINFO_HELP_CHKCASTANY : CORINFO_HELP_ISINSTANCEOFANY); + if (IsReadyToRunCompilation()) + return (fThrowing ? CORINFO_HELP_CHKCASTANY : CORINFO_HELP_ISINSTANCEOFANY); - return m_pEEJitInfo->getCastingHelper(pResolvedToken, fThrowing); + return m_pEEJitInfo->getCastingHelper(pResolvedToken, fThrowing); } CorInfoHelpFunc ZapInfo::getNewArrHelper(CORINFO_CLASS_HANDLE arrayCls) { - if (IsReadyToRunCompilation()) - return CORINFO_HELP_NEWARR_1_R2R_DIRECT; + if (IsReadyToRunCompilation()) + return CORINFO_HELP_NEWARR_1_R2R_DIRECT; - return m_pEEJitInfo->getNewArrHelper(arrayCls); + return m_pEEJitInfo->getNewArrHelper(arrayCls); } bool ZapInfo::getReadyToRunHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, - CORINFO_LOOKUP_KIND * pGenericLookupKind, - CorInfoHelpFunc id, - CORINFO_CONST_LOOKUP * pLookup) + CORINFO_LOOKUP_KIND * pGenericLookupKind, + CorInfoHelpFunc id, + CORINFO_CONST_LOOKUP * pLookup) { #ifdef FEATURE_READYTORUN_COMPILER - _ASSERTE(IsReadyToRunCompilation()); + _ASSERTE(IsReadyToRunCompilation()); - ZapImport * pImport = NULL; + ZapImport * pImport = NULL; - DWORD fAtypicalCallsite = (id & CORINFO_HELP_READYTORUN_ATYPICAL_CALLSITE); - id = (CorInfoHelpFunc)(id & ~CORINFO_HELP_READYTORUN_ATYPICAL_CALLSITE); + DWORD fAtypicalCallsite = (id & CORINFO_HELP_READYTORUN_ATYPICAL_CALLSITE); + id = (CorInfoHelpFunc)(id & ~CORINFO_HELP_READYTORUN_ATYPICAL_CALLSITE); - switch (id) - { - case CORINFO_HELP_READYTORUN_NEW: + switch (id) + { + case CORINFO_HELP_READYTORUN_NEW: // Call CEEInfo::getNewHelper to validate the request (e.g., check for abstract class). m_pEEJitInfo->getNewHelper(pResolvedToken, m_currentMethodHandle); - if ((getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) != 0) - return false; // Requires runtime lookup. - pImport = m_pImage->GetImportTable()->GetDynamicHelperCell( - (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_NEW_HELPER | fAtypicalCallsite), pResolvedToken->hClass); - break; - - case CORINFO_HELP_READYTORUN_NEWARR_1: - if ((getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) != 0) - return false; // Requires runtime lookup. - pImport = m_pImage->GetImportTable()->GetDynamicHelperCell( - (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_NEW_ARRAY_HELPER | fAtypicalCallsite), pResolvedToken->hClass); - break; - - case CORINFO_HELP_READYTORUN_ISINSTANCEOF: - if ((getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) != 0) - return false; // Requires runtime lookup. - pImport = m_pImage->GetImportTable()->GetDynamicHelperCell( - (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_ISINSTANCEOF_HELPER | fAtypicalCallsite), pResolvedToken->hClass); - break; - - case CORINFO_HELP_READYTORUN_CHKCAST: - if ((getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) != 0) - return false; // Requires runtime lookup. - pImport = m_pImage->GetImportTable()->GetDynamicHelperCell( - (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_CHKCAST_HELPER | fAtypicalCallsite), pResolvedToken->hClass); - break; - - case CORINFO_HELP_READYTORUN_STATIC_BASE: - if ((getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) != 0) - return false; // Requires runtime lookup. - if (m_pImage->GetCompileInfo()->IsInCurrentVersionBubble(m_pEEJitInfo->getClassModule(pResolvedToken->hClass))) - { - pImport = m_pImage->GetImportTable()->GetDynamicHelperCell( - (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_CCTOR_TRIGGER | fAtypicalCallsite), pResolvedToken->hClass); - } - else - { - // READYTORUN: FUTURE: Cross-module static cctor triggers - m_zapper->Warning(W("ReadyToRun: Cross-module static cctor triggers not supported\n")); - ThrowHR(E_NOTIMPL); - } - break; + if ((getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) != 0) + return false; // Requires runtime lookup. + pImport = m_pImage->GetImportTable()->GetDynamicHelperCell( + (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_NEW_HELPER | fAtypicalCallsite), pResolvedToken->hClass); + break; - case CORINFO_HELP_READYTORUN_GENERIC_HANDLE: - _ASSERTE(pGenericLookupKind != NULL && pGenericLookupKind->needsRuntimeLookup); - if (pGenericLookupKind->runtimeLookupKind == CORINFO_LOOKUP_METHODPARAM) - { - pImport = m_pImage->GetImportTable()->GetDictionaryLookupCell( - (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_DICTIONARY_LOOKUP_METHOD | fAtypicalCallsite), pResolvedToken, pGenericLookupKind); - } + case CORINFO_HELP_READYTORUN_NEWARR_1: + if ((getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) != 0) + return false; // Requires runtime lookup. + pImport = m_pImage->GetImportTable()->GetDynamicHelperCell( + (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_NEW_ARRAY_HELPER | fAtypicalCallsite), pResolvedToken->hClass); + break; + + case CORINFO_HELP_READYTORUN_ISINSTANCEOF: + if ((getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) != 0) + return false; // Requires runtime lookup. + pImport = m_pImage->GetImportTable()->GetDynamicHelperCell( + (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_ISINSTANCEOF_HELPER | fAtypicalCallsite), pResolvedToken->hClass); + break; + + case CORINFO_HELP_READYTORUN_CHKCAST: + if ((getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) != 0) + return false; // Requires runtime lookup. + pImport = m_pImage->GetImportTable()->GetDynamicHelperCell( + (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_CHKCAST_HELPER | fAtypicalCallsite), pResolvedToken->hClass); + break; + + case CORINFO_HELP_READYTORUN_STATIC_BASE: + if ((getClassAttribs(pResolvedToken->hClass) & CORINFO_FLG_SHAREDINST) != 0) + return false; // Requires runtime lookup. + if (m_pImage->GetCompileInfo()->IsInCurrentVersionBubble(m_pEEJitInfo->getClassModule(pResolvedToken->hClass))) + { + pImport = m_pImage->GetImportTable()->GetDynamicHelperCell( + (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_CCTOR_TRIGGER | fAtypicalCallsite), pResolvedToken->hClass); + } + else + { + // READYTORUN: FUTURE: Cross-module static cctor triggers + m_zapper->Warning(W("ReadyToRun: Cross-module static cctor triggers not supported\n")); + ThrowHR(E_NOTIMPL); + } + break; + + case CORINFO_HELP_READYTORUN_GENERIC_HANDLE: + _ASSERTE(pGenericLookupKind != NULL && pGenericLookupKind->needsRuntimeLookup); + if (pGenericLookupKind->runtimeLookupKind == CORINFO_LOOKUP_METHODPARAM) + { + pImport = m_pImage->GetImportTable()->GetDictionaryLookupCell( + (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_DICTIONARY_LOOKUP_METHOD | fAtypicalCallsite), pResolvedToken, pGenericLookupKind); + } else if (pGenericLookupKind->runtimeLookupKind == CORINFO_LOOKUP_THISOBJ) { pImport = m_pImage->GetImportTable()->GetDictionaryLookupCell( (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_DICTIONARY_LOOKUP_THISOBJ | fAtypicalCallsite), pResolvedToken, pGenericLookupKind); } - else - { - _ASSERTE(pGenericLookupKind->runtimeLookupKind == CORINFO_LOOKUP_CLASSPARAM); - pImport = m_pImage->GetImportTable()->GetDictionaryLookupCell( - (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_DICTIONARY_LOOKUP_TYPE | fAtypicalCallsite), pResolvedToken, pGenericLookupKind); - } - break; + else + { + _ASSERTE(pGenericLookupKind->runtimeLookupKind == CORINFO_LOOKUP_CLASSPARAM); + pImport = m_pImage->GetImportTable()->GetDictionaryLookupCell( + (CORCOMPILE_FIXUP_BLOB_KIND)(ENCODE_DICTIONARY_LOOKUP_TYPE | fAtypicalCallsite), pResolvedToken, pGenericLookupKind); + } + break; - default: - _ASSERTE(false); - ThrowHR(E_NOTIMPL); - } + default: + _ASSERTE(false); + ThrowHR(E_NOTIMPL); + } - pLookup->accessType = IAT_PVALUE; - pLookup->addr = pImport; - return true; + pLookup->accessType = IAT_PVALUE; + pLookup->addr = pImport; + return true; #else - return false; + return false; #endif } diff --git a/src/zap/zapinfo.h b/src/zap/zapinfo.h index a79319e7a2..eadefe8555 100644 --- a/src/zap/zapinfo.h +++ b/src/zap/zapinfo.h @@ -550,7 +550,7 @@ public: unsigned getClassNumInstanceFields(CORINFO_CLASS_HANDLE cls); - CorInfoHelpFunc getNewHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_METHOD_HANDLE callerHandle); + CorInfoHelpFunc getNewHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, CORINFO_METHOD_HANDLE callerHandle, bool * pHasSideEffects = NULL); CorInfoHelpFunc getCastingHelper(CORINFO_RESOLVED_TOKEN * pResolvedToken, bool fThrowing); CorInfoHelpFunc getNewArrHelper(CORINFO_CLASS_HANDLE arrayCls); CorInfoHelpFunc getSharedCCtorHelper(CORINFO_CLASS_HANDLE clsHnd); -- cgit v1.2.3