From de36f0b39cc7ef1f3609eadb1950f2d3363341a4 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 6 Jun 2024 17:54:52 -0700 Subject: [PATCH 1/7] initial workup; need to handle uninitialized fields somehow --- src/coreclr/jit/compiler.cpp | 12 +++++++-- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/gentree.h | 1 + src/coreclr/jit/jitconfigvalues.h | 1 + src/coreclr/jit/morph.cpp | 6 ++--- src/coreclr/jit/objectalloc.cpp | 35 ++++++++++++++++++-------- src/coreclr/jit/objectalloc.h | 28 +++++++++++++++++++-- src/coreclr/jit/utils.cpp | 12 +++++---- src/coreclr/jit/utils.h | 8 ++++++ src/coreclr/jit/valuenum.cpp | 42 +++++++++++++++++++++++++------ src/coreclr/jit/valuenumfuncs.h | 1 + 11 files changed, 118 insertions(+), 30 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 54be15ed7f20b..f1a0ef86a9106 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4717,9 +4717,17 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // local variable allocation on the stack. ObjectAllocator objectAllocator(this); // PHASE_ALLOCATE_OBJECTS - if (compObjectStackAllocation() && opts.OptimizationEnabled()) + if (opts.OptimizationEnabled()) { - objectAllocator.EnableObjectStackAllocation(); + if (JitConfig.JitObjectStackAllocationAnalysis() > 0) + { + objectAllocator.EnableObjectStackAllocationAnalysis(); + } + + if (compObjectStackAllocation()) + { + objectAllocator.EnableObjectStackAllocation(); + } } objectAllocator.Run(); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index daa0348eaccad..5f5690d9dfcc8 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6497,7 +6497,7 @@ class Compiler GenTree* fgMorphCastIntoHelper(GenTree* tree, int helper, GenTree* oper); - GenTree* fgMorphIntoHelperCall( + GenTreeCall* fgMorphIntoHelperCall( GenTree* tree, int helper, bool morphArgs, GenTree* arg1 = nullptr, GenTree* arg2 = nullptr); // A "MorphAddrContext" carries information from the surrounding context. If we are evaluating a byref address, diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 578b2f43b349b..177e57d83d8f9 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4171,6 +4171,7 @@ enum GenTreeCallFlags : unsigned int GTF_CALL_M_CAST_CAN_BE_EXPANDED = 0x04000000, // this cast (helper call) can be expanded if it's profitable. To be removed. GTF_CALL_M_CAST_OBJ_NONNULL = 0x08000000, // if we expand this specific cast we don't need to check the input object for null // NOTE: if needed, this flag can be removed, and we can introduce new _NONNUL cast helpers + GTF_CALL_M_NO_ESCAPE = 0x10000000, // the object allocated by this call does not escape allocating thread }; inline constexpr GenTreeCallFlags operator ~(GenTreeCallFlags a) diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index dbd56791d88d8..1730a4e324ab2 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -651,6 +651,7 @@ RELEASE_CONFIG_INTEGER(JitExtDefaultPolicyProfScale, W("JitExtDefaultPolicyProfS RELEASE_CONFIG_INTEGER(JitInlinePolicyModel, W("JitInlinePolicyModel"), 0) RELEASE_CONFIG_INTEGER(JitInlinePolicyProfile, W("JitInlinePolicyProfile"), 0) RELEASE_CONFIG_INTEGER(JitInlinePolicyProfileThreshold, W("JitInlinePolicyProfileThreshold"), 40) +RELEASE_CONFIG_INTEGER(JitObjectStackAllocationAnalysis, W("JitObjectStackAllocationAnalysis"), 1) RELEASE_CONFIG_INTEGER(JitObjectStackAllocation, W("JitObjectStackAllocation"), 0) RELEASE_CONFIG_INTEGER(JitEECallTimingInfo, W("JitEECallTimingInfo"), 0) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 89fae6f436709..bc8605eb39241 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -180,7 +180,7 @@ class SharedTempsScope // Return value: // The call (which is the same as `tree`). // -GenTree* Compiler::fgMorphIntoHelperCall(GenTree* tree, int helper, bool morphArgs, GenTree* arg1, GenTree* arg2) +GenTreeCall* Compiler::fgMorphIntoHelperCall(GenTree* tree, int helper, bool morphArgs, GenTree* arg1, GenTree* arg2) { // The helper call ought to be semantically equivalent to the original node, so preserve its VN. tree->ChangeOper(GT_CALL, GenTree::PRESERVE_VN); @@ -250,10 +250,10 @@ GenTree* Compiler::fgMorphIntoHelperCall(GenTree* tree, int helper, bool morphAr if (morphArgs) { SharedTempsScope scope(this); - tree = fgMorphArgs(call); + call = fgMorphArgs(call); } - return tree; + return call; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 3d6206e0f723c..beb7f584327cd 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -36,7 +36,9 @@ PhaseStatus ObjectAllocator::DoPhase() return PhaseStatus::MODIFIED_NOTHING; } - if (IsObjectStackAllocationEnabled()) + const bool analyze = IsObjectStackAllocationAnalysisEnabled(); + + if (analyze) { JITDUMP("enabled, analyzing...\n"); DoAnalysis(); @@ -117,7 +119,7 @@ void ObjectAllocator::AddConnGraphEdge(unsigned int sourceLclNum, unsigned int t void ObjectAllocator::DoAnalysis() { - assert(m_IsObjectStackAllocationEnabled); + assert(m_IsObjectStackAllocationAnalysisEnabled); assert(!m_AnalysisDone); if (comp->lvaCount > 0) @@ -413,9 +415,20 @@ bool ObjectAllocator::MorphAllocObjNodes() JITDUMP("Allocating local variable V%02u on the heap\n", lclNum); } - data = MorphAllocObjNodeIntoHelperCall(asAllocObj); + // Some new helpers directly cause escape (eg add to finalizer queue) + // + const bool doesNotEscape = !CanLclVarEscape(lclNum) && !asAllocObj->gtHelperHasSideEffects; + GenTreeCall* const helper = MorphAllocObjNodeIntoHelperCall(asAllocObj); + + data = helper; stmtExpr->AsLclVar()->Data() = data; stmtExpr->AddAllEffectsFlags(data); + + if (doesNotEscape) + { + JITDUMP("ALLOCOBJ at [%06u] does not escape\n", comp->dspTreeID(asAllocObj)); + helper->gtCallMoreFlags |= GTF_CALL_M_NO_ESCAPE; + } } } #ifdef DEBUG @@ -444,7 +457,7 @@ bool ObjectAllocator::MorphAllocObjNodes() // Notes: // Must update parents flags after this. -GenTree* ObjectAllocator::MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* allocObj) +GenTreeCall* ObjectAllocator::MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* allocObj) { assert(allocObj != nullptr); @@ -460,11 +473,11 @@ GenTree* ObjectAllocator::MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* alloc } #endif - const bool morphArgs = false; - GenTree* helperCall = comp->fgMorphIntoHelperCall(allocObj, allocObj->gtNewHelper, morphArgs, arg); + const bool morphArgs = false; + GenTreeCall* const helperCall = comp->fgMorphIntoHelperCall(allocObj, allocObj->gtNewHelper, morphArgs, arg); if (helperHasSideEffects) { - helperCall->AsCall()->gtCallMoreFlags |= GTF_CALL_M_ALLOC_SIDE_EFFECTS; + helperCall->gtCallMoreFlags |= GTF_CALL_M_ALLOC_SIDE_EFFECTS; } #ifdef FEATURE_READYTORUN @@ -616,6 +629,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_QMARK: case GT_ADD: case GT_FIELD_ADDR: + case GT_BOX: // Check whether the local escapes via its grandparent. ++parentIndex; keepChecking = true; @@ -642,9 +656,10 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent // TODO-ObjectStackAllocation: Special-case helpers here that // 1. Don't make objects escape. // 2. Protect objects as interior (GCPROTECT_BEGININTERIOR() instead of GCPROTECT_BEGIN()). - // 3. Don't check that the object is in the heap in ValidateInner. - - canLclVarEscapeViaParentStack = true; + // 3. Don't check that the object is in the heap in ValidateInner + // + canLclVarEscapeViaParentStack = + !Compiler::s_helperCallProperties.IsNoEscape(comp->eeGetHelperNum(asCall->gtCallMethHnd)); } break; } diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 07307161da002..330fe30c093d1 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -25,6 +25,7 @@ class ObjectAllocator final : public Phase //=============================================================================== // Data members + bool m_IsObjectStackAllocationAnalysisEnabled; bool m_IsObjectStackAllocationEnabled; bool m_AnalysisDone; BitVecTraits m_bitVecTraits; @@ -41,7 +42,9 @@ class ObjectAllocator final : public Phase public: ObjectAllocator(Compiler* comp); bool IsObjectStackAllocationEnabled() const; + bool IsObjectStackAllocationAnalysisEnabled() const; void EnableObjectStackAllocation(); + void EnableObjectStackAllocationAnalysis(); protected: virtual PhaseStatus DoPhase() override; @@ -61,7 +64,7 @@ class ObjectAllocator final : public Phase void ComputeStackObjectPointers(BitVecTraits* bitVecTraits); bool MorphAllocObjNodes(); void RewriteUses(); - GenTree* MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* allocObj); + GenTreeCall* MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* allocObj); unsigned int MorphAllocObjNodeIntoStackAlloc(GenTreeAllocObj* allocObj, BasicBlock* block, Statement* stmt); struct BuildConnGraphVisitorCallbackData; bool CanLclVarEscapeViaParentStack(ArrayStack* parentStack, unsigned int lclNum); @@ -74,6 +77,7 @@ class ObjectAllocator final : public Phase inline ObjectAllocator::ObjectAllocator(Compiler* comp) : Phase(comp, PHASE_ALLOCATE_OBJECTS) + , m_IsObjectStackAllocationAnalysisEnabled(false) , m_IsObjectStackAllocationEnabled(false) , m_AnalysisDone(false) , m_bitVecTraits(comp->lvaCount, comp) @@ -96,12 +100,32 @@ inline bool ObjectAllocator::IsObjectStackAllocationEnabled() const return m_IsObjectStackAllocationEnabled; } +//------------------------------------------------------------------------ +// IsObjectStackAllocationAnalysisEnabled: Returns true iff object stack allocation analysis is enabled +// +// Return Value: +// Returns true iff object stack allocation analysis is enabled + +inline bool ObjectAllocator::IsObjectStackAllocationAnalysisEnabled() const +{ + return m_IsObjectStackAllocationAnalysisEnabled; +} + +//------------------------------------------------------------------------ +// EnableObjectStackAllocationAnalysis: Enable object stack allocation analysis. + +inline void ObjectAllocator::EnableObjectStackAllocationAnalysis() +{ + m_IsObjectStackAllocationAnalysisEnabled = true; +} + //------------------------------------------------------------------------ // EnableObjectStackAllocation: Enable object stack allocation. inline void ObjectAllocator::EnableObjectStackAllocation() { - m_IsObjectStackAllocationEnabled = true; + m_IsObjectStackAllocationEnabled = true; + m_IsObjectStackAllocationAnalysisEnabled = true; } //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/utils.cpp b/src/coreclr/jit/utils.cpp index 7ea584267e900..6cdc56a65306b 100644 --- a/src/coreclr/jit/utils.cpp +++ b/src/coreclr/jit/utils.cpp @@ -1520,6 +1520,7 @@ 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 isNoEscape = false; // true if none of the GC ref arguments can escape switch (helper) { @@ -1644,8 +1645,9 @@ void HelperCallProperties::init() case CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPE: case CORINFO_HELP_TYPEHANDLE_TO_RUNTIMETYPEHANDLE: - isPure = true; - noThrow = true; // These return null for a failing cast + isNoEscape = true; + isPure = true; + noThrow = true; // These return null for a failing cast break; case CORINFO_HELP_GETCURRENTMANAGEDTHREADID: @@ -1669,8 +1671,8 @@ void HelperCallProperties::init() // helpers returning addresses, these can also throw case CORINFO_HELP_UNBOX: case CORINFO_HELP_LDELEMA_REF: - - isPure = true; + isNoEscape = true; + isPure = true; break; // GETREFANY is pure up to the value of the struct argument. We @@ -1742,7 +1744,6 @@ void HelperCallProperties::init() case CORINFO_HELP_ASSIGN_REF_ENSURE_NONHEAP: case CORINFO_HELP_ASSIGN_BYREF: case CORINFO_HELP_BULK_WRITEBARRIER: - mutatesHeap = true; break; @@ -1821,6 +1822,7 @@ void HelperCallProperties::init() m_isAllocator[helper] = isAllocator; m_mutatesHeap[helper] = mutatesHeap; m_mayRunCctor[helper] = mayRunCctor; + m_isNoEscape[helper] = isNoEscape; } } diff --git a/src/coreclr/jit/utils.h b/src/coreclr/jit/utils.h index f665a4e813eff..29a1d7d29c933 100644 --- a/src/coreclr/jit/utils.h +++ b/src/coreclr/jit/utils.h @@ -590,6 +590,7 @@ class HelperCallProperties bool m_isAllocator[CORINFO_HELP_COUNT]; bool m_mutatesHeap[CORINFO_HELP_COUNT]; bool m_mayRunCctor[CORINFO_HELP_COUNT]; + bool m_isNoEscape[CORINFO_HELP_COUNT]; void init(); @@ -647,6 +648,13 @@ class HelperCallProperties assert(helperId < CORINFO_HELP_COUNT); return m_mayRunCctor[helperId]; } + + bool IsNoEscape(CorInfoHelpFunc helperId) + { + assert(helperId > CORINFO_HELP_UNDEF); + assert(helperId < CORINFO_HELP_COUNT); + return m_isNoEscape[helperId]; + } }; //***************************************************************************** diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 58a5285b2e350..8387cce16c969 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -5909,7 +5909,20 @@ void Compiler::fgValueNumberFieldLoad(GenTree* loadTree, GenTree* baseAddr, Fiel ValueNum loadValueVN = vnStore->VNForLoad(VNK_Liberal, fieldValueVN, fieldSize, loadType, offset, loadSize); loadTree->gtVNPair.SetLiberal(loadValueVN); - loadTree->gtVNPair.SetConservative(vnStore->VNForExpr(compCurBB, loadType)); + + // If the object we're writing the field of has not escaped, it is thread private. + // So we can set the conservative VN more aggressively. + // + VNFuncApp funcApp; + if ((baseAddr != nullptr) && vnStore->GetVNFunc(fieldValueSelectorVN, &funcApp) && + (funcApp.m_func == VNF_JitNewNoEscape)) + { + loadTree->gtVNPair.SetConservative(loadValueVN); + } + else + { + loadTree->gtVNPair.SetConservative(vnStore->VNForExpr(compCurBB, loadType)); + } } //------------------------------------------------------------------------ @@ -11440,7 +11453,9 @@ void Compiler::fgValueNumberTree(GenTree* tree) // Then, let's see if we can find JitNew at least VNFuncApp funcApp; const bool addrIsVNFunc = vnStore->GetVNFunc(addrNvnp.GetLiberal(), &funcApp); - if (addrIsVNFunc && (funcApp.m_func == VNF_JitNew) && addrNvnp.BothEqual()) + if (addrIsVNFunc && + ((funcApp.m_func == VNF_JitNew) || (funcApp.m_func == VNF_JitNewNoEscape)) && + addrNvnp.BothEqual()) { tree->gtVNPair = vnStore->VNPWithExc(ValueNumPair(funcApp.m_args[0], funcApp.m_args[0]), addrXvnp); @@ -12481,6 +12496,7 @@ void Compiler::fgValueNumberHelperCallFunc(GenTreeCall* call, VNFunc vnf, ValueN switch (vnf) { case VNF_JitNew: + case VNF_JitNewNoEscape: { generateUniqueVN = true; vnpExc = ValueNumStore::VNPForEmptyExcSet(); @@ -12750,8 +12766,15 @@ void Compiler::fgValueNumberCall(GenTreeCall* call) if (modHeap) { - // For now, arbitrary side effect on GcHeap/ByrefExposed. - fgMutateGcHeap(call DEBUGARG("HELPER - modifies heap")); + if ((call->gtCallMoreFlags & GTF_CALL_M_NO_ESCAPE) != 0) + { + // zero obj?? + } + else + { + // For now, arbitrary side effect on GcHeap/ByrefExposed. + fgMutateGcHeap(call DEBUGARG("HELPER - modifies heap")); + } } } else @@ -13323,6 +13346,11 @@ bool Compiler::fgValueNumberHelperCall(GenTreeCall* call) } } + if ((vnf == VNF_JitNew) && ((call->gtCallMoreFlags & GTF_CALL_M_NO_ESCAPE) != 0)) + { + vnf = VNF_JitNewNoEscape; + } + fgValueNumberHelperCallFunc(call, vnf, vnpExc); return modHeap; } @@ -14144,15 +14172,15 @@ CORINFO_CLASS_HANDLE ValueNumStore::GetObjectType(ValueNum vn, bool* pIsExact, b // CastClass/IsInstanceOf/JitNew all have the class handle as the first argument const VNFunc func = funcApp.m_func; - if ((func == VNF_CastClass) || (func == VNF_IsInstanceOf) || (func == VNF_JitNew)) + if ((func == VNF_CastClass) || (func == VNF_IsInstanceOf) || (func == VNF_JitNew) || (func == VNF_JitNewNoEscape)) { ssize_t clsHandle; ValueNum clsVN = funcApp.m_args[0]; if (IsVNTypeHandle(clsVN) && EmbeddedHandleMapLookup(ConstantValue(clsVN), &clsHandle)) { // JitNew returns an exact and non-null obj, castclass and isinst do not have this guarantee. - *pIsNonNull = func == VNF_JitNew; - *pIsExact = func == VNF_JitNew; + *pIsNonNull = (func == VNF_JitNew) || (func == VNF_JitNewNoEscape); + *pIsExact = (func == VNF_JitNew) || (func == VNF_JitNewNoEscape); return (CORINFO_CLASS_HANDLE)clsHandle; } } diff --git a/src/coreclr/jit/valuenumfuncs.h b/src/coreclr/jit/valuenumfuncs.h index 392cd58611e6e..46dd7cf0f7654 100644 --- a/src/coreclr/jit/valuenumfuncs.h +++ b/src/coreclr/jit/valuenumfuncs.h @@ -147,6 +147,7 @@ ValueNumFuncDef(ReadyToRunGenericHandle, 2, false, true, false, false) ValueNumFuncDef(GetStaticAddrTLS, 1, false, true, false, false) ValueNumFuncDef(JitNew, 2, false, true, false, false) +ValueNumFuncDef(JitNewNoEscape, 2, false, true, false, false) ValueNumFuncDef(JitNewArr, 3, false, true, false, false) ValueNumFuncDef(JitNewMdArr, 4, false, true, false, false) ValueNumFuncDef(JitReadyToRunNew, 2, false, true, false, false) From f3b07030a126bae0f5a621374b5c5c3908e9cbf3 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 6 Jun 2024 19:48:28 -0700 Subject: [PATCH 2/7] don't ask about can escape if we haven't analyzed --- src/coreclr/jit/objectalloc.cpp | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index beb7f584327cd..b155d99c82772 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -417,17 +417,20 @@ bool ObjectAllocator::MorphAllocObjNodes() // Some new helpers directly cause escape (eg add to finalizer queue) // - const bool doesNotEscape = !CanLclVarEscape(lclNum) && !asAllocObj->gtHelperHasSideEffects; - GenTreeCall* const helper = MorphAllocObjNodeIntoHelperCall(asAllocObj); + GenTreeCall* const helper = MorphAllocObjNodeIntoHelperCall(asAllocObj); data = helper; stmtExpr->AsLclVar()->Data() = data; stmtExpr->AddAllEffectsFlags(data); - if (doesNotEscape) + if (IsObjectStackAllocationAnalysisEnabled()) { - JITDUMP("ALLOCOBJ at [%06u] does not escape\n", comp->dspTreeID(asAllocObj)); - helper->gtCallMoreFlags |= GTF_CALL_M_NO_ESCAPE; + const bool doesNotEscape = !CanLclVarEscape(lclNum) && !asAllocObj->gtHelperHasSideEffects; + if (doesNotEscape) + { + JITDUMP("ALLOCOBJ at [%06u] does not escape\n", comp->dspTreeID(asAllocObj)); + helper->gtCallMoreFlags |= GTF_CALL_M_NO_ESCAPE; + } } } } From a50a4f6d15be5788cc7be044d0e29ff8aad75163 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 6 Jun 2024 19:53:51 -0700 Subject: [PATCH 3/7] not quite ready for this --- src/coreclr/jit/objectalloc.cpp | 2 +- src/coreclr/jit/valuenum.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index b155d99c82772..a92fb8cd75ba6 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -418,7 +418,7 @@ bool ObjectAllocator::MorphAllocObjNodes() // Some new helpers directly cause escape (eg add to finalizer queue) // - GenTreeCall* const helper = MorphAllocObjNodeIntoHelperCall(asAllocObj); + GenTreeCall* const helper = MorphAllocObjNodeIntoHelperCall(asAllocObj); data = helper; stmtExpr->AsLclVar()->Data() = data; stmtExpr->AddAllEffectsFlags(data); diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 8387cce16c969..6b0419105d783 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -12766,11 +12766,11 @@ void Compiler::fgValueNumberCall(GenTreeCall* call) if (modHeap) { - if ((call->gtCallMoreFlags & GTF_CALL_M_NO_ESCAPE) != 0) - { - // zero obj?? - } - else + // if ((call->gtCallMoreFlags & GTF_CALL_M_NO_ESCAPE) != 0) + //{ + // // zero obj?? + // } + // else { // For now, arbitrary side effect on GcHeap/ByrefExposed. fgMutateGcHeap(call DEBUGARG("HELPER - modifies heap")); From a541865638fde97891006ad162210304fd396276 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 7 Jun 2024 08:30:53 -0700 Subject: [PATCH 4/7] may need to rewrite call arg types; fix test now that we get one more stack allocation --- src/coreclr/jit/objectalloc.cpp | 1 + src/coreclr/jit/utils.cpp | 1 + .../ObjectStackAllocation/ObjectStackAllocationTests.cs | 9 +++++---- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index a92fb8cd75ba6..a78767a6d3b54 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -751,6 +751,7 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p break; case GT_IND: + case GT_CALL: break; default: diff --git a/src/coreclr/jit/utils.cpp b/src/coreclr/jit/utils.cpp index 6cdc56a65306b..d59627b683909 100644 --- a/src/coreclr/jit/utils.cpp +++ b/src/coreclr/jit/utils.cpp @@ -1665,6 +1665,7 @@ void HelperCallProperties::init() // These throw for a failing cast // But if given a null input arg will return null + // if they throw they cause the object to escape isPure = true; break; diff --git a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs index 9faf9b4cce4f0..bc3e2fb4b3d19 100644 --- a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs +++ b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs @@ -152,14 +152,15 @@ public static int TestEntryPoint() CallTestAndVerifyAllocation(AllocateClassWithGcFieldAndInt, 5, expectedAllocationKind); + // This test calls CORINFO_HELP_ISINSTANCEOFCLASS + CallTestAndVerifyAllocation(AllocateSimpleClassAndCheckTypeHelper, 1, expectedAllocationKind); + // The remaining tests currently never allocate on the stack - if (expectedAllocationKind == AllocationKind.Stack) { + if (expectedAllocationKind == AllocationKind.Stack) + { expectedAllocationKind = AllocationKind.Heap; } - // This test calls CORINFO_HELP_ISINSTANCEOFCLASS - CallTestAndVerifyAllocation(AllocateSimpleClassAndCheckTypeHelper, 1, expectedAllocationKind); - // This test calls CORINFO_HELP_CHKCASTCLASS_SPECIAL CallTestAndVerifyAllocation(AllocateSimpleClassAndCast, 7, expectedAllocationKind); From 8cdce7307367ac215e3844a795a8e019dcb3fe99 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 7 Jun 2024 09:51:58 -0700 Subject: [PATCH 5/7] metrics --- src/coreclr/jit/jitmetadatalist.h | 2 ++ src/coreclr/jit/objectalloc.cpp | 3 +++ 2 files changed, 5 insertions(+) diff --git a/src/coreclr/jit/jitmetadatalist.h b/src/coreclr/jit/jitmetadatalist.h index 209133f4324b0..5d140d6b6f257 100644 --- a/src/coreclr/jit/jitmetadatalist.h +++ b/src/coreclr/jit/jitmetadatalist.h @@ -71,6 +71,8 @@ JITMETADATAMETRIC(ProfileInconsistentInlineeScale, int, 0) JITMETADATAMETRIC(ProfileInconsistentInlinee, int, 0) JITMETADATAMETRIC(ProfileInconsistentNoReturnInlinee, int, 0) JITMETADATAMETRIC(ProfileInconsistentMayThrowInlinee, int, 0) +JITMETADATAMETRIC(NewHelperCalls, int, 0) +JITMETADATAMETRIC(NonEscapingNewHelperCalls, int, 0) #undef JITMETADATA #undef JITMETADATAINFO diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index a78767a6d3b54..96e0130e2b08f 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -380,6 +380,8 @@ bool ObjectAllocator::MorphAllocObjNodes() if (canonicalAllocObjFound) { assert(basicBlockHasNewObj); + + comp->Metrics.NewHelperCalls++; //------------------------------------------------------------------------ // We expect the following expression tree at this point // STMTx (IL 0x... ???) @@ -430,6 +432,7 @@ bool ObjectAllocator::MorphAllocObjNodes() { JITDUMP("ALLOCOBJ at [%06u] does not escape\n", comp->dspTreeID(asAllocObj)); helper->gtCallMoreFlags |= GTF_CALL_M_NO_ESCAPE; + comp->Metrics.NonEscapingNewHelperCalls++; } } } From e7db5342d7b090d2dab6c4fb45e4457c24263635 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 7 Jun 2024 15:58:01 -0700 Subject: [PATCH 6/7] some fixes to escape analysis --- src/coreclr/jit/jitmetadatalist.h | 6 ++++-- src/coreclr/jit/objectalloc.cpp | 35 +++++++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/jitmetadatalist.h b/src/coreclr/jit/jitmetadatalist.h index 5d140d6b6f257..ee5b212faadba 100644 --- a/src/coreclr/jit/jitmetadatalist.h +++ b/src/coreclr/jit/jitmetadatalist.h @@ -71,8 +71,10 @@ JITMETADATAMETRIC(ProfileInconsistentInlineeScale, int, 0) JITMETADATAMETRIC(ProfileInconsistentInlinee, int, 0) JITMETADATAMETRIC(ProfileInconsistentNoReturnInlinee, int, 0) JITMETADATAMETRIC(ProfileInconsistentMayThrowInlinee, int, 0) -JITMETADATAMETRIC(NewHelperCalls, int, 0) -JITMETADATAMETRIC(NonEscapingNewHelperCalls, int, 0) +JITMETADATAMETRIC(NewRefClassHelperCalls, int, 0) +JITMETADATAMETRIC(NonEscapingNewRefClassHelperCalls, int, 0) +JITMETADATAMETRIC(NewBoxedValueClassHelperCalls, int, 0) +JITMETADATAMETRIC(NonEscapingNewBoxedValueClassHelperCalls, int, 0) #undef JITMETADATA #undef JITMETADATAINFO diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 96e0130e2b08f..0046e13f036f2 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -381,7 +381,6 @@ bool ObjectAllocator::MorphAllocObjNodes() { assert(basicBlockHasNewObj); - comp->Metrics.NewHelperCalls++; //------------------------------------------------------------------------ // We expect the following expression tree at this point // STMTx (IL 0x... ???) @@ -394,6 +393,18 @@ bool ObjectAllocator::MorphAllocObjNodes() unsigned int lclNum = stmtExpr->AsLclVar()->GetLclNum(); CORINFO_CLASS_HANDLE clsHnd = data->AsAllocObj()->gtAllocObjClsHnd; + DWORD classAttribs = comp->info.compCompHnd->getClassAttribs(clsHnd); + const bool isValueClass = (classAttribs & CORINFO_FLG_VALUECLASS) != 0; + + if (isValueClass) + { + comp->Metrics.NewBoxedValueClassHelperCalls++; + } + else + { + comp->Metrics.NewRefClassHelperCalls++; + } + // Don't attempt to do stack allocations inside basic blocks that may be in a loop. if (IsObjectStackAllocationEnabled() && !basicBlockHasBackwardJump && CanAllocateLclVarOnStack(lclNum, clsHnd)) @@ -419,7 +430,6 @@ bool ObjectAllocator::MorphAllocObjNodes() // Some new helpers directly cause escape (eg add to finalizer queue) // - GenTreeCall* const helper = MorphAllocObjNodeIntoHelperCall(asAllocObj); data = helper; stmtExpr->AsLclVar()->Data() = data; @@ -432,7 +442,19 @@ bool ObjectAllocator::MorphAllocObjNodes() { JITDUMP("ALLOCOBJ at [%06u] does not escape\n", comp->dspTreeID(asAllocObj)); helper->gtCallMoreFlags |= GTF_CALL_M_NO_ESCAPE; - comp->Metrics.NonEscapingNewHelperCalls++; + + if (isValueClass) + { + comp->Metrics.NonEscapingNewBoxedValueClassHelperCalls++; + } + else + { + comp->Metrics.NonEscapingNewRefClassHelperCalls++; + } + } + else + { + JITDUMP("ALLOCOBJ at [%06u] escapes\n", comp->dspTreeID(asAllocObj)); } } } @@ -620,13 +642,16 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_EQ: case GT_NE: + case GT_NULLCHECK: canLclVarEscapeViaParentStack = false; break; case GT_COMMA: + case GT_STORE_BLK: if (parent->AsOp()->gtGetOp1() == parentStack->Top(parentIndex - 1)) { - // Left child of GT_COMMA, it will be discarded + // Left child of GT_COMMA will be discarded + // Left childof GT_STORE_BLK is an address to write to canLclVarEscapeViaParentStack = false; break; } @@ -635,6 +660,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_QMARK: case GT_ADD: case GT_FIELD_ADDR: + case GT_LCL_ADDR: case GT_BOX: // Check whether the local escapes via its grandparent. ++parentIndex; @@ -730,6 +756,7 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p case GT_QMARK: case GT_ADD: case GT_FIELD_ADDR: + case GT_LCL_ADDR: if (parent->TypeGet() == TYP_REF) { parent->ChangeType(newType); From e65a20afde563da4f21cf6b0ce92fa8843c60dd7 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Fri, 7 Jun 2024 18:05:05 -0700 Subject: [PATCH 7/7] temp suppress value class accounting to avoid missing context data --- src/coreclr/jit/objectalloc.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 0046e13f036f2..c2e5bfdfb7744 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -393,7 +393,8 @@ bool ObjectAllocator::MorphAllocObjNodes() unsigned int lclNum = stmtExpr->AsLclVar()->GetLclNum(); CORINFO_CLASS_HANDLE clsHnd = data->AsAllocObj()->gtAllocObjClsHnd; - DWORD classAttribs = comp->info.compCompHnd->getClassAttribs(clsHnd); + DWORD classAttribs = 0; + // comp->info.compCompHnd->getClassAttribs(clsHnd); const bool isValueClass = (classAttribs & CORINFO_FLG_VALUECLASS) != 0; if (isValueClass)