From 1b0e3d3b8e27a39d3e8585ec6d344dcf79315b43 Mon Sep 17 00:00:00 2001 From: Steven He Date: Mon, 15 Jul 2024 20:23:24 +0900 Subject: [PATCH 01/65] initial prototype --- src/coreclr/jit/objectalloc.cpp | 153 +++++++++++++++++++++++++++++++- src/coreclr/jit/objectalloc.h | 34 ++++++- 2 files changed, 183 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 89e28c5978c6d..1ef558791f6e6 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -30,9 +30,9 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX // PhaseStatus ObjectAllocator::DoPhase() { - if ((comp->optMethodFlags & OMF_HAS_NEWOBJ) == 0) + if ((comp->optMethodFlags & OMF_HAS_NEWOBJ) == 0 && (comp->optMethodFlags & OMF_HAS_NEWARRAY) == 0) { - JITDUMP("no newobjs in this method; punting\n"); + JITDUMP("no newobjs or newarr in this method; punting\n"); return PhaseStatus::MODIFIED_NOTHING; } @@ -391,6 +391,7 @@ bool ObjectAllocator::MorphAllocObjNodes() GenTree* data = nullptr; bool canonicalAllocObjFound = false; + bool canonicalAllocArrFound = false; if (stmtExpr->OperIs(GT_STORE_LCL_VAR) && stmtExpr->TypeIs(TYP_REF)) { @@ -400,6 +401,75 @@ bool ObjectAllocator::MorphAllocObjNodes() { canonicalAllocObjFound = true; } + else if (data->IsHelperCall()) + { + GenTreeCall* call = data->AsCall(); + if (call->GetHelperNum() == CORINFO_HELP_NEWARR_1_VC) + { + canonicalAllocArrFound = true; + } + } + } + + if (canonicalAllocArrFound) + { + GenTreeCall* asCall = data->AsCall(); + assert(asCall->GetHelperNum() == CORINFO_HELP_NEWARR_1_VC); + //------------------------------------------------------------------------ + // We expect the following expression tree at this point + // STMTx (IL 0x... ???) + // * STORE_LCL_VAR ref + // \--* CALL help ref CORINFO_HELP_NEWARR_1_VC + // +--* CNS_INT(h) long + // \--* * + //------------------------------------------------------------------------ + + unsigned int lclNum = stmtExpr->AsLclVar()->GetLclNum(); + CallArg* arg = asCall->gtArgs.GetArgByIndex(0); + GenTree* node = arg->GetNode(); + CORINFO_CLASS_HANDLE clsHnd = (CORINFO_CLASS_HANDLE)node->AsIntConCommon()->IntegralValue(); + GenTree* len = arg->GetNext()->GetNode(); + const char* onHeapReason = nullptr; + unsigned int structSize = 0; + bool canStack = false; + + // Don't attempt to do stack allocations inside basic blocks that may be in a loop. + // + if (!IsObjectStackAllocationEnabled()) + { + onHeapReason = "[object stack allocation disabled]"; + canStack = false; + } + else if (basicBlockHasBackwardJump) + { + onHeapReason = "[alloc in loop]"; + canStack = false; + } + else if (!len->IsCnsIntOrI()) + { + onHeapReason = "[non-constant size]"; + canStack = false; + } + else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, (unsigned int)len->AsIntCon()->gtIconVal, + &structSize, &onHeapReason)) + { + // reason set by the call + canStack = false; + } + else + { + JITDUMP("Allocating V%02u on the stack\n", lclNum); + canStack = true; + const unsigned int stackLclNum = MorphNewArrNodeIntoStackAlloc(asCall, clsHnd, structSize, block, stmt); + m_HeapLocalToStackLocalMap.AddOrUpdate(lclNum, stackLclNum); + // We keep the set of possibly-stack-pointing pointers as a superset of the set of + // definitely-stack-pointing pointers. All definitely-stack-pointing pointers are in both sets. + MarkLclVarAsDefinitelyStackPointing(lclNum); + MarkLclVarAsPossiblyStackPointing(lclNum); + stmt->GetRootNode()->gtBashToNOP(); + comp->optMethodFlags |= OMF_HAS_OBJSTACKALLOC; + didStackAllocate = true; + } } if (canonicalAllocObjFound) @@ -443,7 +513,7 @@ bool ObjectAllocator::MorphAllocObjNodes() onHeapReason = "[alloc in loop]"; canStack = false; } - else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, &onHeapReason)) + else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, 0, NULL, &onHeapReason)) { // reason set by the call canStack = false; @@ -555,6 +625,77 @@ GenTree* ObjectAllocator::MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* alloc return helperCall; } +unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* newArr, + CORINFO_CLASS_HANDLE clsHnd, + unsigned int structSize, + BasicBlock* block, + Statement* stmt) +{ + assert(newArr != nullptr); + assert(m_AnalysisDone); + assert(clsHnd != NO_CLASS_HANDLE); + + const bool shortLifetime = false; + const unsigned int lclNum = comp->lvaGrabTemp(shortLifetime DEBUGARG("stack allocated array temp")); + + comp->lvaSetStruct(lclNum, comp->typGetBlkLayout(structSize), /* unsafeValueClsCheck */ false); + + // Initialize the object memory if necessary. + bool bbInALoop = block->HasFlag(BBF_BACKWARD_JUMP); + bool bbIsReturn = block->KindIs(BBJ_RETURN); + LclVarDsc* const lclDsc = comp->lvaGetDesc(lclNum); + lclDsc->lvStackAllocatedBox = false; + if (comp->fgVarNeedsExplicitZeroInit(lclNum, bbInALoop, bbIsReturn)) + { + //------------------------------------------------------------------------ + // STMTx (IL 0x... ???) + // * STORE_LCL_VAR struct + // \--* CNS_INT int 0 + //------------------------------------------------------------------------ + + GenTree* init = comp->gtNewStoreLclVarNode(lclNum, comp->gtNewIconNode(0)); + Statement* initStmt = comp->gtNewStmt(init); + + comp->fgInsertStmtBefore(block, stmt, initStmt); + } + else + { + JITDUMP("\nSuppressing zero-init for V%02u -- expect to zero in prolog\n", lclNum); + lclDsc->lvSuppressedZeroInit = 1; + comp->compSuppressedZeroInit = true; + } + + // Initialize the vtable slot. + // + //------------------------------------------------------------------------ + // STMTx (IL 0x... ???) + // * STORE_LCL_FLD long + // \--* CNS_INT(h) long + //------------------------------------------------------------------------ + + // Initialize the method table pointer. + GenTree* init = comp->gtNewStoreLclFldNode(lclNum, TYP_I_IMPL, 0, newArr->gtArgs.GetArgByIndex(0)->GetNode()); + Statement* initStmt = comp->gtNewStmt(init); + + comp->fgInsertStmtBefore(block, stmt, initStmt); + + // Initialize the length of array. + // + //------------------------------------------------------------------------ + // STMTx (IL 0x... ???) + // * STORE_LCL_FLD long + // \--* CNS_INT long + //------------------------------------------------------------------------ + + // Pass the length of the array. + GenTree* len = comp->gtNewStoreLclFldNode(lclNum, TYP_I_IMPL, 8, newArr->gtArgs.GetArgByIndex(1)->GetNode()); + Statement* lenStmt = comp->gtNewStmt(len); + + comp->fgInsertStmtBefore(block, stmt, lenStmt); + + return lclNum; +} + //------------------------------------------------------------------------ // MorphAllocObjNodeIntoStackAlloc: Morph a GT_ALLOCOBJ node into stack // allocation. @@ -682,6 +823,9 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_EQ: case GT_NE: case GT_NULLCHECK: + case GT_ARR_LENGTH: + case GT_ARR_ELEM: + case GT_INDEX_ADDR: canLclVarEscapeViaParentStack = false; break; @@ -778,6 +922,9 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p case GT_EQ: case GT_NE: case GT_NULLCHECK: + case GT_ARR_LENGTH: + case GT_ARR_ELEM: + case GT_INDEX_ADDR: break; case GT_COMMA: diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 8a873be8b34ad..03f01e6c7d28d 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -47,7 +47,11 @@ class ObjectAllocator final : public Phase virtual PhaseStatus DoPhase() override; private: - bool CanAllocateLclVarOnStack(unsigned int lclNum, CORINFO_CLASS_HANDLE clsHnd, const char** reason); + bool CanAllocateLclVarOnStack(unsigned int lclNum, + CORINFO_CLASS_HANDLE clsHnd, + unsigned int length, + unsigned int* structSize, + const char** reason); bool CanLclVarEscape(unsigned int lclNum); void MarkLclVarAsPossiblyStackPointing(unsigned int lclNum); void MarkLclVarAsDefinitelyStackPointing(unsigned int lclNum); @@ -64,6 +68,8 @@ class ObjectAllocator final : public Phase GenTree* MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* allocObj); unsigned int MorphAllocObjNodeIntoStackAlloc( GenTreeAllocObj* allocObj, CORINFO_CLASS_HANDLE clsHnd, bool isValueClass, BasicBlock* block, Statement* stmt); + unsigned int MorphNewArrNodeIntoStackAlloc( + GenTreeCall* newArr, CORINFO_CLASS_HANDLE clsHnd, unsigned int structSize, BasicBlock* block, Statement* stmt); struct BuildConnGraphVisitorCallbackData; bool CanLclVarEscapeViaParentStack(ArrayStack* parentStack, unsigned int lclNum); void UpdateAncestorTypes(GenTree* tree, ArrayStack* parentStack, var_types newType); @@ -119,6 +125,8 @@ inline void ObjectAllocator::EnableObjectStackAllocation() // inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNum, CORINFO_CLASS_HANDLE clsHnd, + unsigned int length, + unsigned int* structSize, const char** reason) { assert(m_AnalysisDone); @@ -150,6 +158,25 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNu classSize = comp->info.compCompHnd->getClassSize(clsHnd); } + else if (comp->info.compCompHnd->isSDArray(clsHnd)) + { + CorInfoType type = comp->info.compCompHnd->getChildType(clsHnd, &clsHnd); + if (type != CORINFO_TYPE_UNDEF && clsHnd == NULL) + { + // This is an array of primitive types + classSize = (unsigned int)OFFSETOF__CORINFO_Array__data + genTypeSize(JITtype2varType(type)) * length; + } + else if (comp->info.compCompHnd->isValueClass(clsHnd)) + { + classSize = + (unsigned int)OFFSETOF__CORINFO_Array__data + comp->info.compCompHnd->getClassSize(clsHnd) * length; + } + else + { + *reason = "[array contains gc refs]"; + return false; + } + } else { if (!enableRefClasses) @@ -181,6 +208,11 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNu return false; } + if (structSize != NULL) + { + *structSize = classSize; + } + return true; } From 57b7e42fbe27f51a3a3321f9d2e511a069d4c3da Mon Sep 17 00:00:00 2001 From: Steven He Date: Mon, 15 Jul 2024 22:50:42 +0900 Subject: [PATCH 02/65] Morph ARR_LENGTH and INDEX_ADDR --- src/coreclr/jit/objectalloc.cpp | 65 ++++++++++++++++++++++----------- src/coreclr/jit/objectalloc.h | 21 ++++++----- 2 files changed, 54 insertions(+), 32 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 1ef558791f6e6..f652ea1e5338c 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -415,14 +415,6 @@ bool ObjectAllocator::MorphAllocObjNodes() { GenTreeCall* asCall = data->AsCall(); assert(asCall->GetHelperNum() == CORINFO_HELP_NEWARR_1_VC); - //------------------------------------------------------------------------ - // We expect the following expression tree at this point - // STMTx (IL 0x... ???) - // * STORE_LCL_VAR ref - // \--* CALL help ref CORINFO_HELP_NEWARR_1_VC - // +--* CNS_INT(h) long - // \--* * - //------------------------------------------------------------------------ unsigned int lclNum = stmtExpr->AsLclVar()->GetLclNum(); CallArg* arg = asCall->gtArgs.GetArgByIndex(0); @@ -430,7 +422,7 @@ bool ObjectAllocator::MorphAllocObjNodes() CORINFO_CLASS_HANDLE clsHnd = (CORINFO_CLASS_HANDLE)node->AsIntConCommon()->IntegralValue(); GenTree* len = arg->GetNext()->GetNode(); const char* onHeapReason = nullptr; - unsigned int structSize = 0; + unsigned int blockSize = 0; bool canStack = false; // Don't attempt to do stack allocations inside basic blocks that may be in a loop. @@ -450,8 +442,8 @@ bool ObjectAllocator::MorphAllocObjNodes() onHeapReason = "[non-constant size]"; canStack = false; } - else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, (unsigned int)len->AsIntCon()->gtIconVal, - &structSize, &onHeapReason)) + else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, (unsigned int)len->AsIntCon()->gtIconVal, &blockSize, + &onHeapReason)) { // reason set by the call canStack = false; @@ -459,8 +451,10 @@ bool ObjectAllocator::MorphAllocObjNodes() else { JITDUMP("Allocating V%02u on the stack\n", lclNum); - canStack = true; - const unsigned int stackLclNum = MorphNewArrNodeIntoStackAlloc(asCall, clsHnd, structSize, block, stmt); + canStack = true; + const unsigned int stackLclNum = + MorphNewArrNodeIntoStackAlloc(asCall, clsHnd, (unsigned int)len->AsIntCon()->gtIconVal, + blockSize, block, stmt); m_HeapLocalToStackLocalMap.AddOrUpdate(lclNum, stackLclNum); // We keep the set of possibly-stack-pointing pointers as a superset of the set of // definitely-stack-pointing pointers. All definitely-stack-pointing pointers are in both sets. @@ -627,7 +621,8 @@ GenTree* ObjectAllocator::MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* alloc unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* newArr, CORINFO_CLASS_HANDLE clsHnd, - unsigned int structSize, + unsigned int length, + unsigned int blockSize, BasicBlock* block, Statement* stmt) { @@ -638,7 +633,7 @@ unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* const bool shortLifetime = false; const unsigned int lclNum = comp->lvaGrabTemp(shortLifetime DEBUGARG("stack allocated array temp")); - comp->lvaSetStruct(lclNum, comp->typGetBlkLayout(structSize), /* unsafeValueClsCheck */ false); + comp->lvaSetStruct(lclNum, comp->typGetBlkLayout(blockSize), /* unsafeValueClsCheck */ false); // Initialize the object memory if necessary. bool bbInALoop = block->HasFlag(BBF_BACKWARD_JUMP); @@ -679,18 +674,18 @@ unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* comp->fgInsertStmtBefore(block, stmt, initStmt); - // Initialize the length of array. + // Initialize the length slot. // //------------------------------------------------------------------------ // STMTx (IL 0x... ???) // * STORE_LCL_FLD long - // \--* CNS_INT long + // \--* CNS_INT long //------------------------------------------------------------------------ - // Pass the length of the array. - GenTree* len = comp->gtNewStoreLclFldNode(lclNum, TYP_I_IMPL, 8, newArr->gtArgs.GetArgByIndex(1)->GetNode()); + // Pass the total length of the array. + GenTree* len = comp->gtNewStoreLclFldNode(lclNum, TYP_I_IMPL, genTypeSize(TYP_I_IMPL), + comp->gtNewIconNode(length, TYP_I_IMPL)); Statement* lenStmt = comp->gtNewStmt(len); - comp->fgInsertStmtBefore(block, stmt, lenStmt); return lclNum; @@ -824,7 +819,6 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_NE: case GT_NULLCHECK: case GT_ARR_LENGTH: - case GT_ARR_ELEM: case GT_INDEX_ADDR: canLclVarEscapeViaParentStack = false; break; @@ -923,7 +917,6 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p case GT_NE: case GT_NULLCHECK: case GT_ARR_LENGTH: - case GT_ARR_ELEM: case GT_INDEX_ADDR: break; @@ -1172,6 +1165,34 @@ void ObjectAllocator::RewriteUses() } } } + // Rewrite INDEX_ADDR to ADD for stack-allocated arrays. + else if (tree->OperIs(GT_INDEX_ADDR)) + { + GenTreeIndexAddr* const indexAddr = tree->AsIndexAddr(); + GenTree* const arr = indexAddr->Arr(); + GenTree* const ind = indexAddr->Index(); + + if (arr->OperIs(GT_LCL_ADDR) && ind->IsCnsIntOrI()) + { + JITDUMP("Rewriting INDEX_ADDR to ADD [%06u]\n", m_compiler->dspTreeID(tree)); + GenTree* const add = m_compiler->gtNewOperNode(GT_ADD, TYP_BYREF, arr, ind); + *use = add; + } + } + // Rewrite ARR_LENGTH to LCL_FLD for stack-allocated arrays. + else if (tree->OperIsArrLength()) + { + GenTreeArrLen* const arrLen = tree->AsArrLen(); + GenTree* const arr = arrLen->ArrRef(); + + if (arr->OperIs(GT_LCL_ADDR)) + { + JITDUMP("Rewriting ARR_LENGTH to LCL_FLD [%06u]\n", m_compiler->dspTreeID(tree)); + GenTree* const lclFld = m_compiler->gtNewLclFldNode(arr->AsLclVarCommon()->GetLclNum(), TYP_I_IMPL, + OFFSETOF__CORINFO_Array__length); + *use = lclFld; + } + } return Compiler::fgWalkResult::WALK_CONTINUE; } diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 03f01e6c7d28d..e39f9d666b6b2 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -50,7 +50,7 @@ class ObjectAllocator final : public Phase bool CanAllocateLclVarOnStack(unsigned int lclNum, CORINFO_CLASS_HANDLE clsHnd, unsigned int length, - unsigned int* structSize, + unsigned int* blockSize, const char** reason); bool CanLclVarEscape(unsigned int lclNum); void MarkLclVarAsPossiblyStackPointing(unsigned int lclNum); @@ -68,8 +68,12 @@ class ObjectAllocator final : public Phase GenTree* MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* allocObj); unsigned int MorphAllocObjNodeIntoStackAlloc( GenTreeAllocObj* allocObj, CORINFO_CLASS_HANDLE clsHnd, bool isValueClass, BasicBlock* block, Statement* stmt); - unsigned int MorphNewArrNodeIntoStackAlloc( - GenTreeCall* newArr, CORINFO_CLASS_HANDLE clsHnd, unsigned int structSize, BasicBlock* block, Statement* stmt); + unsigned int MorphNewArrNodeIntoStackAlloc(GenTreeCall* newArr, + CORINFO_CLASS_HANDLE clsHnd, + unsigned int length, + unsigned int blockSize, + BasicBlock* block, + Statement* stmt); struct BuildConnGraphVisitorCallbackData; bool CanLclVarEscapeViaParentStack(ArrayStack* parentStack, unsigned int lclNum); void UpdateAncestorTypes(GenTree* tree, ArrayStack* parentStack, var_types newType); @@ -123,11 +127,8 @@ inline void ObjectAllocator::EnableObjectStackAllocation() // Return Value: // Returns true iff local variable can be allocated on the stack. // -inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNum, - CORINFO_CLASS_HANDLE clsHnd, - unsigned int length, - unsigned int* structSize, - const char** reason) +inline bool ObjectAllocator::CanAllocateLclVarOnStack( + unsigned int lclNum, CORINFO_CLASS_HANDLE clsHnd, unsigned int length, unsigned int* blockSize, const char** reason) { assert(m_AnalysisDone); @@ -208,9 +209,9 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNu return false; } - if (structSize != NULL) + if (blockSize != NULL) { - *structSize = classSize; + *blockSize = (unsigned int)classSize; } return true; From 1b5b25e579940e12a52a857cdfeb2875ef2b2fcb Mon Sep 17 00:00:00 2001 From: Steven He Date: Mon, 15 Jul 2024 23:01:16 +0900 Subject: [PATCH 03/65] Fix incorrect array length storage --- src/coreclr/jit/objectalloc.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index f652ea1e5338c..ab48b96fcef86 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -683,8 +683,8 @@ unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* //------------------------------------------------------------------------ // Pass the total length of the array. - GenTree* len = comp->gtNewStoreLclFldNode(lclNum, TYP_I_IMPL, genTypeSize(TYP_I_IMPL), - comp->gtNewIconNode(length, TYP_I_IMPL)); + GenTree* len = + comp->gtNewStoreLclFldNode(lclNum, TYP_INT, genTypeSize(TYP_I_IMPL), comp->gtNewIconNode(length, TYP_INT)); Statement* lenStmt = comp->gtNewStmt(len); comp->fgInsertStmtBefore(block, stmt, lenStmt); @@ -1175,8 +1175,10 @@ void ObjectAllocator::RewriteUses() if (arr->OperIs(GT_LCL_ADDR) && ind->IsCnsIntOrI()) { JITDUMP("Rewriting INDEX_ADDR to ADD [%06u]\n", m_compiler->dspTreeID(tree)); - GenTree* const add = m_compiler->gtNewOperNode(GT_ADD, TYP_BYREF, arr, ind); - *use = add; + GenTree* const offset = + m_compiler->gtNewIconNode(OFFSETOF__CORINFO_Array__data + ind->AsIntCon()->gtIconVal, TYP_INT); + GenTree* const add = m_compiler->gtNewOperNode(GT_ADD, TYP_BYREF, arr, offset); + *use = add; } } // Rewrite ARR_LENGTH to LCL_FLD for stack-allocated arrays. @@ -1188,9 +1190,9 @@ void ObjectAllocator::RewriteUses() if (arr->OperIs(GT_LCL_ADDR)) { JITDUMP("Rewriting ARR_LENGTH to LCL_FLD [%06u]\n", m_compiler->dspTreeID(tree)); - GenTree* const lclFld = m_compiler->gtNewLclFldNode(arr->AsLclVarCommon()->GetLclNum(), TYP_I_IMPL, + GenTree* const lclFld = m_compiler->gtNewLclFldNode(arr->AsLclVarCommon()->GetLclNum(), TYP_INT, OFFSETOF__CORINFO_Array__length); - *use = lclFld; + *use = lclFld; } } From 395b7352b6ae95261dd0dabcc82c99324fa7aef0 Mon Sep 17 00:00:00 2001 From: Steven He Date: Mon, 15 Jul 2024 23:33:46 +0900 Subject: [PATCH 04/65] Use offset and correct type --- src/coreclr/jit/objectalloc.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index ab48b96fcef86..bb440c1490ce8 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -683,8 +683,8 @@ unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* //------------------------------------------------------------------------ // Pass the total length of the array. - GenTree* len = - comp->gtNewStoreLclFldNode(lclNum, TYP_INT, genTypeSize(TYP_I_IMPL), comp->gtNewIconNode(length, TYP_INT)); + GenTree* len = comp->gtNewStoreLclFldNode(lclNum, TYP_INT, OFFSETOF__CORINFO_Array__length, + comp->gtNewIconNode(length, TYP_INT)); Statement* lenStmt = comp->gtNewStmt(len); comp->fgInsertStmtBefore(block, stmt, lenStmt); From 17de70bfae31adaa22649e0ff2d4b5cb3ff3a7cf Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 01:17:17 +0900 Subject: [PATCH 05/65] handle reassignment --- src/coreclr/jit/objectalloc.cpp | 58 ++++++++++++++++++++++++--------- src/coreclr/jit/objectalloc.h | 2 ++ 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index bb440c1490ce8..a9ab5f691752e 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -89,6 +89,27 @@ void ObjectAllocator::MarkLclVarAsEscaping(unsigned int lclNum) BitVecOps::AddElemD(&m_bitVecTraits, m_EscapingPointers, lclNum); } +//------------------------------------------------------------------------------ +// MarkLclVarHasLocalStore : Mark local variable having local store. +// +// +// Arguments: +// lclNum - Pointing local variable number +// Returns: +// true if the local variable is first marked as having local store +// false if the local variable is already marked as having local store + +bool ObjectAllocator::MarkLclVarHasLocalStore(unsigned int lclNum) +{ + if (BitVecOps::IsMember(&m_bitVecTraits, m_PointersHasLocalStore, lclNum)) + { + return false; + } + + BitVecOps::AddElemD(&m_bitVecTraits, m_PointersHasLocalStore, lclNum); + return true; +} + //------------------------------------------------------------------------------ // MarkLclVarAsPossiblyStackPointing : Mark local variable as possibly pointing // to a stack-allocated object. @@ -140,6 +161,7 @@ void ObjectAllocator::DoAnalysis() if (comp->lvaCount > 0) { m_EscapingPointers = BitVecOps::MakeEmpty(&m_bitVecTraits); + m_PointersHasLocalStore = BitVecOps::MakeEmpty(&m_bitVecTraits); m_ConnGraphAdjacencyMatrix = new (comp->getAllocator(CMK_ObjectAllocator)) BitSetShortLongRep[comp->lvaCount]; MarkEscapingVarsAndBuildConnGraph(); @@ -201,7 +223,9 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() if (tree->OperIsLocalStore()) { - lclEscapes = false; + // conservatively marking locals being reassigned as escaping + // this can happen on arrays + lclEscapes = !m_allocator->MarkLclVarHasLocalStore(lclNum); } else if (tree->OperIs(GT_LCL_VAR) && tree->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL)) { @@ -404,7 +428,8 @@ bool ObjectAllocator::MorphAllocObjNodes() else if (data->IsHelperCall()) { GenTreeCall* call = data->AsCall(); - if (call->GetHelperNum() == CORINFO_HELP_NEWARR_1_VC) + if (call->GetHelperNum() == CORINFO_HELP_NEWARR_1_VC && + call->gtArgs.GetArgByIndex(0)->GetNode()->IsCnsIntOrI()) { canonicalAllocArrFound = true; } @@ -1168,31 +1193,32 @@ void ObjectAllocator::RewriteUses() // Rewrite INDEX_ADDR to ADD for stack-allocated arrays. else if (tree->OperIs(GT_INDEX_ADDR)) { - GenTreeIndexAddr* const indexAddr = tree->AsIndexAddr(); - GenTree* const arr = indexAddr->Arr(); - GenTree* const ind = indexAddr->Index(); + GenTreeIndexAddr* const gtIndexAddr = tree->AsIndexAddr(); + GenTree* const gtArr = gtIndexAddr->Arr(); + GenTree* const gtInd = gtIndexAddr->Index(); - if (arr->OperIs(GT_LCL_ADDR) && ind->IsCnsIntOrI()) + if (gtArr->OperIs(GT_LCL_ADDR) && gtInd->IsCnsIntOrI()) { JITDUMP("Rewriting INDEX_ADDR to ADD [%06u]\n", m_compiler->dspTreeID(tree)); - GenTree* const offset = - m_compiler->gtNewIconNode(OFFSETOF__CORINFO_Array__data + ind->AsIntCon()->gtIconVal, TYP_INT); - GenTree* const add = m_compiler->gtNewOperNode(GT_ADD, TYP_BYREF, arr, offset); - *use = add; + ssize_t offset = + OFFSETOF__CORINFO_Array__data + gtInd->AsIntCon()->gtIconVal * gtIndexAddr->gtElemSize; + GenTree* const gtOffset = m_compiler->gtNewIconNode(offset, TYP_I_IMPL); + GenTree* const gtAdd = m_compiler->gtNewOperNode(GT_ADD, TYP_BYREF, gtArr, gtOffset); + *use = gtAdd; } } // Rewrite ARR_LENGTH to LCL_FLD for stack-allocated arrays. else if (tree->OperIsArrLength()) { - GenTreeArrLen* const arrLen = tree->AsArrLen(); - GenTree* const arr = arrLen->ArrRef(); + GenTreeArrLen* const gtArrLen = tree->AsArrLen(); + GenTree* const gtArr = gtArrLen->ArrRef(); - if (arr->OperIs(GT_LCL_ADDR)) + if (gtArr->OperIs(GT_LCL_ADDR)) { JITDUMP("Rewriting ARR_LENGTH to LCL_FLD [%06u]\n", m_compiler->dspTreeID(tree)); - GenTree* const lclFld = m_compiler->gtNewLclFldNode(arr->AsLclVarCommon()->GetLclNum(), TYP_INT, - OFFSETOF__CORINFO_Array__length); - *use = lclFld; + GenTree* const gtLclFld = m_compiler->gtNewLclFldNode(gtArr->AsLclVarCommon()->GetLclNum(), TYP_INT, + OFFSETOF__CORINFO_Array__length); + *use = gtLclFld; } } diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index e39f9d666b6b2..f7ff0ffcb8bc1 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -33,6 +33,7 @@ class ObjectAllocator final : public Phase // definitely-stack-pointing pointers. All definitely-stack-pointing pointers are in both sets. BitVec m_PossiblyStackPointingPointers; BitVec m_DefinitelyStackPointingPointers; + BitVec m_PointersHasLocalStore; LocalToLocalMap m_HeapLocalToStackLocalMap; BitSetShortLongRep* m_ConnGraphAdjacencyMatrix; @@ -59,6 +60,7 @@ class ObjectAllocator final : public Phase bool DoesLclVarPointToStack(unsigned int lclNum); void DoAnalysis(); void MarkLclVarAsEscaping(unsigned int lclNum); + bool MarkLclVarHasLocalStore(unsigned int lclNum); void MarkEscapingVarsAndBuildConnGraph(); void AddConnGraphEdge(unsigned int sourceLclNum, unsigned int targetLclNum); void ComputeEscapingNodes(BitVecTraits* bitVecTraits, BitVec& escapingNodes); From 5443c422dc23e67dfe0a7f2c0d14ac798ab73963 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 01:55:51 +0900 Subject: [PATCH 06/65] range check --- src/coreclr/jit/objectalloc.cpp | 8 ++++++-- src/coreclr/jit/objectalloc.h | 3 +++ 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index a9ab5f691752e..ca75406fa5111 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -481,6 +481,7 @@ bool ObjectAllocator::MorphAllocObjNodes() MorphNewArrNodeIntoStackAlloc(asCall, clsHnd, (unsigned int)len->AsIntCon()->gtIconVal, blockSize, block, stmt); m_HeapLocalToStackLocalMap.AddOrUpdate(lclNum, stackLclNum); + m_LocalArrToLenMap.AddOrUpdate(stackLclNum, (unsigned int)len->AsIntCon()->gtIconVal); // We keep the set of possibly-stack-pointing pointers as a superset of the set of // definitely-stack-pointing pointers. All definitely-stack-pointing pointers are in both sets. MarkLclVarAsDefinitelyStackPointing(lclNum); @@ -1196,11 +1197,14 @@ void ObjectAllocator::RewriteUses() GenTreeIndexAddr* const gtIndexAddr = tree->AsIndexAddr(); GenTree* const gtArr = gtIndexAddr->Arr(); GenTree* const gtInd = gtIndexAddr->Index(); + unsigned int arrLen = 0; - if (gtArr->OperIs(GT_LCL_ADDR) && gtInd->IsCnsIntOrI()) + if (gtArr->OperIs(GT_LCL_ADDR) && gtInd->IsCnsIntOrI() && + m_allocator->m_LocalArrToLenMap.TryGetValue(gtArr->AsLclVarCommon()->GetLclNum(), &arrLen) && + gtInd->AsIntCon()->gtIconVal < arrLen) { JITDUMP("Rewriting INDEX_ADDR to ADD [%06u]\n", m_compiler->dspTreeID(tree)); - ssize_t offset = + const ssize_t offset = OFFSETOF__CORINFO_Array__data + gtInd->AsIntCon()->gtIconVal * gtIndexAddr->gtElemSize; GenTree* const gtOffset = m_compiler->gtNewIconNode(offset, TYP_I_IMPL); GenTree* const gtAdd = m_compiler->gtNewOperNode(GT_ADD, TYP_BYREF, gtArr, gtOffset); diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index f7ff0ffcb8bc1..65fe381f24230 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -22,6 +22,7 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX class ObjectAllocator final : public Phase { typedef SmallHashTable LocalToLocalMap; + typedef SmallHashTable LocalArrToLenMap; //=============================================================================== // Data members @@ -35,6 +36,7 @@ class ObjectAllocator final : public Phase BitVec m_DefinitelyStackPointingPointers; BitVec m_PointersHasLocalStore; LocalToLocalMap m_HeapLocalToStackLocalMap; + LocalArrToLenMap m_LocalArrToLenMap; BitSetShortLongRep* m_ConnGraphAdjacencyMatrix; //=============================================================================== @@ -91,6 +93,7 @@ inline ObjectAllocator::ObjectAllocator(Compiler* comp) , m_AnalysisDone(false) , m_bitVecTraits(comp->lvaCount, comp) , m_HeapLocalToStackLocalMap(comp->getAllocator()) + , m_LocalArrToLenMap(comp->getAllocator()) { m_EscapingPointers = BitVecOps::UninitVal(); m_PossiblyStackPointingPointers = BitVecOps::UninitVal(); From b2d07da2a86bdbda8a5b913b9de7d148434b65d5 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 02:06:42 +0900 Subject: [PATCH 07/65] throw range check failure --- src/coreclr/jit/objectalloc.cpp | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index ca75406fa5111..028ce76295f5e 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -1200,15 +1200,25 @@ void ObjectAllocator::RewriteUses() unsigned int arrLen = 0; if (gtArr->OperIs(GT_LCL_ADDR) && gtInd->IsCnsIntOrI() && - m_allocator->m_LocalArrToLenMap.TryGetValue(gtArr->AsLclVarCommon()->GetLclNum(), &arrLen) && - gtInd->AsIntCon()->gtIconVal < arrLen) + m_allocator->m_LocalArrToLenMap.TryGetValue(gtArr->AsLclVarCommon()->GetLclNum(), &arrLen)) { - JITDUMP("Rewriting INDEX_ADDR to ADD [%06u]\n", m_compiler->dspTreeID(tree)); - const ssize_t offset = - OFFSETOF__CORINFO_Array__data + gtInd->AsIntCon()->gtIconVal * gtIndexAddr->gtElemSize; - GenTree* const gtOffset = m_compiler->gtNewIconNode(offset, TYP_I_IMPL); - GenTree* const gtAdd = m_compiler->gtNewOperNode(GT_ADD, TYP_BYREF, gtArr, gtOffset); - *use = gtAdd; + if (gtInd->AsIntCon()->gtIconVal < arrLen) + { + JITDUMP("Rewriting INDEX_ADDR to ADD [%06u]\n", m_compiler->dspTreeID(tree)); + const ssize_t offset = + OFFSETOF__CORINFO_Array__data + gtInd->AsIntCon()->gtIconVal * gtIndexAddr->gtElemSize; + GenTree* const gtOffset = m_compiler->gtNewIconNode(offset, TYP_I_IMPL); + GenTree* const gtAdd = m_compiler->gtNewOperNode(GT_ADD, TYP_BYREF, gtArr, gtOffset); + *use = gtAdd; + } + else + { + JITDUMP("Rewriting INDEX_ADDR to RNGCHKFAIL helper call [%06u]\n", m_compiler->dspTreeID(tree)); + GenTree* const gtRngChkFail = + m_compiler->gtNewMustThrowException(CORINFO_HELP_RNGCHKFAIL, gtIndexAddr->gtType, + gtIndexAddr->gtStructElemClass); + *use = gtRngChkFail; + } } } // Rewrite ARR_LENGTH to LCL_FLD for stack-allocated arrays. From b5ae9e72fcd498618a04d7b6c0915944c4ae8b53 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 02:13:12 +0900 Subject: [PATCH 08/65] update comments --- src/coreclr/jit/objectalloc.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 028ce76295f5e..2e170f36ca213 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -700,12 +700,12 @@ unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* comp->fgInsertStmtBefore(block, stmt, initStmt); - // Initialize the length slot. + // Initialize the array length. // //------------------------------------------------------------------------ // STMTx (IL 0x... ???) - // * STORE_LCL_FLD long - // \--* CNS_INT long + // * STORE_LCL_FLD int + // \--* CNS_INT int //------------------------------------------------------------------------ // Pass the total length of the array. From 87b29de78f98c0609a43a731fff874b696355c7a Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 02:15:36 +0900 Subject: [PATCH 09/65] add metrics --- src/coreclr/jit/jitmetadatalist.h | 1 + src/coreclr/jit/objectalloc.cpp | 10 ++++++++++ 2 files changed, 11 insertions(+) diff --git a/src/coreclr/jit/jitmetadatalist.h b/src/coreclr/jit/jitmetadatalist.h index 4aad9abd1b3cc..225892a290d7b 100644 --- a/src/coreclr/jit/jitmetadatalist.h +++ b/src/coreclr/jit/jitmetadatalist.h @@ -76,6 +76,7 @@ JITMETADATAMETRIC(NewRefClassHelperCalls, int, 0) JITMETADATAMETRIC(StackAllocatedRefClasses, int, 0) JITMETADATAMETRIC(NewBoxedValueClassHelperCalls, int, 0) JITMETADATAMETRIC(StackAllocatedBoxedValueClasses, int, 0) +JITMETADATAMETRIC(StackAllocatedArrays, int, 0) #undef JITMETADATA #undef JITMETADATAINFO diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 2e170f36ca213..4241296819515 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -490,6 +490,16 @@ bool ObjectAllocator::MorphAllocObjNodes() comp->optMethodFlags |= OMF_HAS_OBJSTACKALLOC; didStackAllocate = true; } + + if (canStack) + { + comp->Metrics.StackAllocatedArrays++; + } + else + { + assert(onHeapReason != nullptr); + JITDUMP("Allocating V%02u on the heap: %s\n", lclNum, onHeapReason); + } } if (canonicalAllocObjFound) From eeb681da150dfac5033fbe62bbdc750b64c19ff6 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 03:10:35 +0900 Subject: [PATCH 10/65] minor cleanup --- src/coreclr/jit/jitmetadatalist.h | 1 + src/coreclr/jit/objectalloc.cpp | 4 ++-- src/coreclr/jit/objectalloc.h | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/jitmetadatalist.h b/src/coreclr/jit/jitmetadatalist.h index 225892a290d7b..30733553b8564 100644 --- a/src/coreclr/jit/jitmetadatalist.h +++ b/src/coreclr/jit/jitmetadatalist.h @@ -76,6 +76,7 @@ JITMETADATAMETRIC(NewRefClassHelperCalls, int, 0) JITMETADATAMETRIC(StackAllocatedRefClasses, int, 0) JITMETADATAMETRIC(NewBoxedValueClassHelperCalls, int, 0) JITMETADATAMETRIC(StackAllocatedBoxedValueClasses, int, 0) +JITMETADATAMETRIC(NewArrayHelperCalls, int, 0) JITMETADATAMETRIC(StackAllocatedArrays, int, 0) #undef JITMETADATA diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 4241296819515..6483c3b177c7a 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -543,7 +543,7 @@ bool ObjectAllocator::MorphAllocObjNodes() onHeapReason = "[alloc in loop]"; canStack = false; } - else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, 0, NULL, &onHeapReason)) + else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, 0, nullptr, &onHeapReason)) { // reason set by the call canStack = false; @@ -1212,7 +1212,7 @@ void ObjectAllocator::RewriteUses() if (gtArr->OperIs(GT_LCL_ADDR) && gtInd->IsCnsIntOrI() && m_allocator->m_LocalArrToLenMap.TryGetValue(gtArr->AsLclVarCommon()->GetLclNum(), &arrLen)) { - if (gtInd->AsIntCon()->gtIconVal < arrLen) + if ((unsigned int)gtInd->AsIntCon()->gtIconVal < arrLen) { JITDUMP("Rewriting INDEX_ADDR to ADD [%06u]\n", m_compiler->dspTreeID(tree)); const ssize_t offset = diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 65fe381f24230..099db4b3016ac 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -167,7 +167,7 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack( else if (comp->info.compCompHnd->isSDArray(clsHnd)) { CorInfoType type = comp->info.compCompHnd->getChildType(clsHnd, &clsHnd); - if (type != CORINFO_TYPE_UNDEF && clsHnd == NULL) + if (type != CORINFO_TYPE_UNDEF && clsHnd == nullptr) { // This is an array of primitive types classSize = (unsigned int)OFFSETOF__CORINFO_Array__data + genTypeSize(JITtype2varType(type)) * length; @@ -214,7 +214,7 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack( return false; } - if (blockSize != NULL) + if (blockSize != nullptr) { *blockSize = (unsigned int)classSize; } From dee9f389b7d16e30c4cd04f53763723dd89e13d0 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 14:13:23 +0900 Subject: [PATCH 11/65] Introduce new temp and implement local address morphing --- src/coreclr/jit/importer.cpp | 15 ++++++- src/coreclr/jit/lclmorph.cpp | 42 ++++++++++++++++++ src/coreclr/jit/objectalloc.cpp | 79 ++++----------------------------- src/coreclr/jit/objectalloc.h | 5 --- 4 files changed, 64 insertions(+), 77 deletions(-) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index a84f3db3ff473..9503023776524 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -9533,9 +9533,22 @@ void Compiler::impImportBlockCode(BasicBlock* block) // Remember that this function contains 'new' of an SD array. optMethodFlags |= OMF_HAS_NEWARRAY; + // We assign the newly allocated object (by a GT_CALL to newarr node) + // to a temp. Note that the pattern "temp = allocArr" is required + // by ObjectAllocator phase to be able to determine newarr nodes + // without exhaustive walk over all expressions. + lclNum = lvaGrabTemp(true DEBUGARG("NewArr temp")); + + impStoreToTemp(lclNum, op1, CHECK_SPILL_NONE); + + assert(lvaTable[lclNum].lvSingleDef == 0); + lvaTable[lclNum].lvSingleDef = 1; + JITDUMP("Marked V%02u as a single def local\n", lclNum); + lvaSetClass(lclNum, resolvedToken.hClass, true /* is Exact */); + /* Push the result of the call on the stack */ - impPushOnStack(op1, tiRetVal); + impPushOnStack(gtNewLclvNode(lclNum, TYP_REF), tiRetVal); callTyp = TYP_REF; } diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 2a9d0f9a36914..2a03eacefac2f 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -826,6 +826,48 @@ class LocalAddressVisitor final : public GenTreeVisitor switch (node->OperGet()) { + case GT_INDEX_ADDR: + { + assert(TopValue(2).Node() == node); + assert(TopValue(1).Node() == node->gtGetOp1()); + assert(TopValue(0).Node() == node->gtGetOp2()); + + if (node->gtGetOp2()->IsCnsIntOrI()) + { + ssize_t offset = OFFSETOF__CORINFO_Array__data + + node->gtGetOp2()->AsIntCon()->IconValue() * node->AsIndexAddr()->gtElemSize; + if (FitsIn(offset) && TopValue(2).AddOffset(TopValue(1), static_cast(offset))) + { + INDEBUG(TopValue(0).Consume()); + PopValue(); + PopValue(); + break; + } + } + + EscapeValue(TopValue(0), node); + PopValue(); + EscapeValue(TopValue(0), node); + PopValue(); + break; + } + case GT_ARR_LENGTH: + { + Value& arr = TopValue(0); + EscapeValue(arr, node); + PopValue(); + + GenTree* gtArr = arr.Node(); + + if (gtArr->OperIs(GT_LCL_ADDR)) + { + unsigned int lclNum = gtArr->AsLclVarCommon()->GetLclNum(); + GenTree* gtLclFld = m_compiler->gtNewLclFldNode(lclNum, TYP_INT, OFFSETOF__CORINFO_Array__length); + SequenceLocal(gtLclFld->AsLclVarCommon()); + *use = node = gtLclFld; + } + break; + } case GT_STORE_LCL_FLD: if (node->IsPartialLclFld(m_compiler)) { diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 6483c3b177c7a..52565b60599f5 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -89,27 +89,6 @@ void ObjectAllocator::MarkLclVarAsEscaping(unsigned int lclNum) BitVecOps::AddElemD(&m_bitVecTraits, m_EscapingPointers, lclNum); } -//------------------------------------------------------------------------------ -// MarkLclVarHasLocalStore : Mark local variable having local store. -// -// -// Arguments: -// lclNum - Pointing local variable number -// Returns: -// true if the local variable is first marked as having local store -// false if the local variable is already marked as having local store - -bool ObjectAllocator::MarkLclVarHasLocalStore(unsigned int lclNum) -{ - if (BitVecOps::IsMember(&m_bitVecTraits, m_PointersHasLocalStore, lclNum)) - { - return false; - } - - BitVecOps::AddElemD(&m_bitVecTraits, m_PointersHasLocalStore, lclNum); - return true; -} - //------------------------------------------------------------------------------ // MarkLclVarAsPossiblyStackPointing : Mark local variable as possibly pointing // to a stack-allocated object. @@ -161,7 +140,6 @@ void ObjectAllocator::DoAnalysis() if (comp->lvaCount > 0) { m_EscapingPointers = BitVecOps::MakeEmpty(&m_bitVecTraits); - m_PointersHasLocalStore = BitVecOps::MakeEmpty(&m_bitVecTraits); m_ConnGraphAdjacencyMatrix = new (comp->getAllocator(CMK_ObjectAllocator)) BitSetShortLongRep[comp->lvaCount]; MarkEscapingVarsAndBuildConnGraph(); @@ -223,9 +201,7 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() if (tree->OperIsLocalStore()) { - // conservatively marking locals being reassigned as escaping - // this can happen on arrays - lclEscapes = !m_allocator->MarkLclVarHasLocalStore(lclNum); + lclEscapes = false; } else if (tree->OperIs(GT_LCL_VAR) && tree->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL)) { @@ -481,7 +457,6 @@ bool ObjectAllocator::MorphAllocObjNodes() MorphNewArrNodeIntoStackAlloc(asCall, clsHnd, (unsigned int)len->AsIntCon()->gtIconVal, blockSize, block, stmt); m_HeapLocalToStackLocalMap.AddOrUpdate(lclNum, stackLclNum); - m_LocalArrToLenMap.AddOrUpdate(stackLclNum, (unsigned int)len->AsIntCon()->gtIconVal); // We keep the set of possibly-stack-pointing pointers as a superset of the set of // definitely-stack-pointing pointers. All definitely-stack-pointing pointers are in both sets. MarkLclVarAsDefinitelyStackPointing(lclNum); @@ -1096,11 +1071,17 @@ void ObjectAllocator::RewriteUses() if (lclVarDsc->lvType != newType) { - JITDUMP("changing the type of V%02u from %s to %s\n", lclNum, varTypeName(lclVarDsc->lvType), + JITDUMP("Changing the type of V%02u from %s to %s\n", lclNum, varTypeName(lclVarDsc->lvType), varTypeName(newType)); lclVarDsc->lvType = newType; } m_allocator->UpdateAncestorTypes(tree, &m_ancestors, newType); + + if (newLclNum != BAD_VAR_NUM) + { + JITDUMP("Update V%02u to V%02u from use [%06u]\n", lclNum, newLclNum, m_compiler->dspTreeID(tree)); + DISPTREE(tree); + } } return Compiler::fgWalkResult::WALK_CONTINUE; @@ -1201,50 +1182,6 @@ void ObjectAllocator::RewriteUses() } } } - // Rewrite INDEX_ADDR to ADD for stack-allocated arrays. - else if (tree->OperIs(GT_INDEX_ADDR)) - { - GenTreeIndexAddr* const gtIndexAddr = tree->AsIndexAddr(); - GenTree* const gtArr = gtIndexAddr->Arr(); - GenTree* const gtInd = gtIndexAddr->Index(); - unsigned int arrLen = 0; - - if (gtArr->OperIs(GT_LCL_ADDR) && gtInd->IsCnsIntOrI() && - m_allocator->m_LocalArrToLenMap.TryGetValue(gtArr->AsLclVarCommon()->GetLclNum(), &arrLen)) - { - if ((unsigned int)gtInd->AsIntCon()->gtIconVal < arrLen) - { - JITDUMP("Rewriting INDEX_ADDR to ADD [%06u]\n", m_compiler->dspTreeID(tree)); - const ssize_t offset = - OFFSETOF__CORINFO_Array__data + gtInd->AsIntCon()->gtIconVal * gtIndexAddr->gtElemSize; - GenTree* const gtOffset = m_compiler->gtNewIconNode(offset, TYP_I_IMPL); - GenTree* const gtAdd = m_compiler->gtNewOperNode(GT_ADD, TYP_BYREF, gtArr, gtOffset); - *use = gtAdd; - } - else - { - JITDUMP("Rewriting INDEX_ADDR to RNGCHKFAIL helper call [%06u]\n", m_compiler->dspTreeID(tree)); - GenTree* const gtRngChkFail = - m_compiler->gtNewMustThrowException(CORINFO_HELP_RNGCHKFAIL, gtIndexAddr->gtType, - gtIndexAddr->gtStructElemClass); - *use = gtRngChkFail; - } - } - } - // Rewrite ARR_LENGTH to LCL_FLD for stack-allocated arrays. - else if (tree->OperIsArrLength()) - { - GenTreeArrLen* const gtArrLen = tree->AsArrLen(); - GenTree* const gtArr = gtArrLen->ArrRef(); - - if (gtArr->OperIs(GT_LCL_ADDR)) - { - JITDUMP("Rewriting ARR_LENGTH to LCL_FLD [%06u]\n", m_compiler->dspTreeID(tree)); - GenTree* const gtLclFld = m_compiler->gtNewLclFldNode(gtArr->AsLclVarCommon()->GetLclNum(), TYP_INT, - OFFSETOF__CORINFO_Array__length); - *use = gtLclFld; - } - } return Compiler::fgWalkResult::WALK_CONTINUE; } diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 099db4b3016ac..790c53d8cab66 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -22,7 +22,6 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX class ObjectAllocator final : public Phase { typedef SmallHashTable LocalToLocalMap; - typedef SmallHashTable LocalArrToLenMap; //=============================================================================== // Data members @@ -34,9 +33,7 @@ class ObjectAllocator final : public Phase // definitely-stack-pointing pointers. All definitely-stack-pointing pointers are in both sets. BitVec m_PossiblyStackPointingPointers; BitVec m_DefinitelyStackPointingPointers; - BitVec m_PointersHasLocalStore; LocalToLocalMap m_HeapLocalToStackLocalMap; - LocalArrToLenMap m_LocalArrToLenMap; BitSetShortLongRep* m_ConnGraphAdjacencyMatrix; //=============================================================================== @@ -62,7 +59,6 @@ class ObjectAllocator final : public Phase bool DoesLclVarPointToStack(unsigned int lclNum); void DoAnalysis(); void MarkLclVarAsEscaping(unsigned int lclNum); - bool MarkLclVarHasLocalStore(unsigned int lclNum); void MarkEscapingVarsAndBuildConnGraph(); void AddConnGraphEdge(unsigned int sourceLclNum, unsigned int targetLclNum); void ComputeEscapingNodes(BitVecTraits* bitVecTraits, BitVec& escapingNodes); @@ -93,7 +89,6 @@ inline ObjectAllocator::ObjectAllocator(Compiler* comp) , m_AnalysisDone(false) , m_bitVecTraits(comp->lvaCount, comp) , m_HeapLocalToStackLocalMap(comp->getAllocator()) - , m_LocalArrToLenMap(comp->getAllocator()) { m_EscapingPointers = BitVecOps::UninitVal(); m_PossiblyStackPointingPointers = BitVecOps::UninitVal(); From 94c103bec16ad52d39e9fc3718ce25bb0e782e11 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 16:32:50 +0900 Subject: [PATCH 12/65] handle index out-of-range --- src/coreclr/jit/lclmorph.cpp | 62 +++++++++++++++++++++++---------- src/coreclr/jit/objectalloc.cpp | 6 ++-- 2 files changed, 47 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 2a03eacefac2f..de4c2b2112e6f 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -832,16 +832,34 @@ class LocalAddressVisitor final : public GenTreeVisitor assert(TopValue(1).Node() == node->gtGetOp1()); assert(TopValue(0).Node() == node->gtGetOp2()); - if (node->gtGetOp2()->IsCnsIntOrI()) + if (node->gtGetOp1()->OperIs(GT_LCL_VAR, GT_LCL_ADDR) && node->gtGetOp2()->IsCnsIntOrI()) { - ssize_t offset = OFFSETOF__CORINFO_Array__data + - node->gtGetOp2()->AsIntCon()->IconValue() * node->AsIndexAddr()->gtElemSize; - if (FitsIn(offset) && TopValue(2).AddOffset(TopValue(1), static_cast(offset))) + if (TopValue(1).IsAddress() && + m_compiler->lvaGetDesc(TopValue(1).LclNum())->GetLayout()->IsBlockLayout()) { - INDEBUG(TopValue(0).Consume()); - PopValue(); - PopValue(); - break; + ssize_t offset = node->AsIndexAddr()->gtElemOffset + + node->gtGetOp2()->AsIntCon()->IconValue() * node->AsIndexAddr()->gtElemSize; + + if (offset < m_compiler->lvaLclSize(TopValue(1).LclNum())) + { + if (FitsIn(offset) && + TopValue(2).AddOffset(TopValue(1), static_cast(offset))) + { + INDEBUG(TopValue(0).Consume()); + PopValue(); + PopValue(); + break; + } + } + else + { + GenTree* gtThrow = + m_compiler->gtNewMustThrowException(CORINFO_HELP_RNGCHKFAIL, node->TypeGet(), + node->AsIndexAddr()->gtStructElemClass); + m_compiler->lvaGetDesc(gtThrow->AsOp()->gtOp2->AsLclVarCommon())->incLvRefCnt(1, RCS_EARLY); + *use = gtThrow; + m_stmtModified = true; + } } } @@ -853,19 +871,27 @@ class LocalAddressVisitor final : public GenTreeVisitor } case GT_ARR_LENGTH: { - Value& arr = TopValue(0); - EscapeValue(arr, node); - PopValue(); - - GenTree* gtArr = arr.Node(); + assert(TopValue(1).Node() == node); + assert(TopValue(0).Node() == node->gtGetOp1()); - if (gtArr->OperIs(GT_LCL_ADDR)) + if (node->gtGetOp1()->OperIs(GT_LCL_VAR, GT_LCL_ADDR)) { - unsigned int lclNum = gtArr->AsLclVarCommon()->GetLclNum(); - GenTree* gtLclFld = m_compiler->gtNewLclFldNode(lclNum, TYP_INT, OFFSETOF__CORINFO_Array__length); - SequenceLocal(gtLclFld->AsLclVarCommon()); - *use = node = gtLclFld; + if (TopValue(0).IsAddress() && + m_compiler->lvaGetDesc(TopValue(0).LclNum())->GetLayout()->IsBlockLayout()) + { + GenTree* gtLclFld = + m_compiler->gtNewLclFldNode(TopValue(0).LclNum(), TYP_INT, OFFSETOF__CORINFO_Array__length); + SequenceLocal(gtLclFld->AsLclVarCommon()); + *use = gtLclFld; + m_stmtModified = true; + INDEBUG(TopValue(0).Consume()); + PopValue(); + break; + } } + + EscapeValue(TopValue(0), node); + PopValue(); break; } case GT_STORE_LCL_FLD: diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 52565b60599f5..cf2767325f72e 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -443,8 +443,8 @@ bool ObjectAllocator::MorphAllocObjNodes() onHeapReason = "[non-constant size]"; canStack = false; } - else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, (unsigned int)len->AsIntCon()->gtIconVal, &blockSize, - &onHeapReason)) + else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, (unsigned int)len->AsIntCon()->IconValue(), + &blockSize, &onHeapReason)) { // reason set by the call canStack = false; @@ -454,7 +454,7 @@ bool ObjectAllocator::MorphAllocObjNodes() JITDUMP("Allocating V%02u on the stack\n", lclNum); canStack = true; const unsigned int stackLclNum = - MorphNewArrNodeIntoStackAlloc(asCall, clsHnd, (unsigned int)len->AsIntCon()->gtIconVal, + MorphNewArrNodeIntoStackAlloc(asCall, clsHnd, (unsigned int)len->AsIntCon()->IconValue(), blockSize, block, stmt); m_HeapLocalToStackLocalMap.AddOrUpdate(lclNum, stackLclNum); // We keep the set of possibly-stack-pointing pointers as a superset of the set of From 12b297ba427591b68d264b6d131d94445cf65864 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 16:47:24 +0900 Subject: [PATCH 13/65] Refactor to remove duplicates --- src/coreclr/jit/objectalloc.cpp | 237 +++++++++++++++----------------- src/coreclr/jit/objectalloc.h | 6 + 2 files changed, 119 insertions(+), 124 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index cf2767325f72e..8b747df0e0bc5 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -390,8 +390,7 @@ bool ObjectAllocator::MorphAllocObjNodes() GenTree* stmtExpr = stmt->GetRootNode(); GenTree* data = nullptr; - bool canonicalAllocObjFound = false; - bool canonicalAllocArrFound = false; + ObjectAllocationType allocType = OAT_NONE; if (stmtExpr->OperIs(GT_STORE_LCL_VAR) && stmtExpr->TypeIs(TYP_REF)) { @@ -399,7 +398,7 @@ bool ObjectAllocator::MorphAllocObjNodes() if (data->OperGet() == GT_ALLOCOBJ) { - canonicalAllocObjFound = true; + allocType = OAT_NEWOBJ; } else if (data->IsHelperCall()) { @@ -407,24 +406,16 @@ bool ObjectAllocator::MorphAllocObjNodes() if (call->GetHelperNum() == CORINFO_HELP_NEWARR_1_VC && call->gtArgs.GetArgByIndex(0)->GetNode()->IsCnsIntOrI()) { - canonicalAllocArrFound = true; + allocType = OAT_NEWARR; } } } - if (canonicalAllocArrFound) + if (allocType != OAT_NONE) { - GenTreeCall* asCall = data->AsCall(); - assert(asCall->GetHelperNum() == CORINFO_HELP_NEWARR_1_VC); - - unsigned int lclNum = stmtExpr->AsLclVar()->GetLclNum(); - CallArg* arg = asCall->gtArgs.GetArgByIndex(0); - GenTree* node = arg->GetNode(); - CORINFO_CLASS_HANDLE clsHnd = (CORINFO_CLASS_HANDLE)node->AsIntConCommon()->IntegralValue(); - GenTree* len = arg->GetNext()->GetNode(); - const char* onHeapReason = nullptr; - unsigned int blockSize = 0; - bool canStack = false; + bool canStack = false; + const char* onHeapReason = nullptr; + unsigned int lclNum = stmtExpr->AsLclVar()->GetLclNum(); // Don't attempt to do stack allocations inside basic blocks that may be in a loop. // @@ -438,131 +429,129 @@ bool ObjectAllocator::MorphAllocObjNodes() onHeapReason = "[alloc in loop]"; canStack = false; } - else if (!len->IsCnsIntOrI()) - { - onHeapReason = "[non-constant size]"; - canStack = false; - } - else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, (unsigned int)len->AsIntCon()->IconValue(), - &blockSize, &onHeapReason)) - { - // reason set by the call - canStack = false; - } else { - JITDUMP("Allocating V%02u on the stack\n", lclNum); - canStack = true; - const unsigned int stackLclNum = - MorphNewArrNodeIntoStackAlloc(asCall, clsHnd, (unsigned int)len->AsIntCon()->IconValue(), - blockSize, block, stmt); - m_HeapLocalToStackLocalMap.AddOrUpdate(lclNum, stackLclNum); - // We keep the set of possibly-stack-pointing pointers as a superset of the set of - // definitely-stack-pointing pointers. All definitely-stack-pointing pointers are in both sets. - MarkLclVarAsDefinitelyStackPointing(lclNum); - MarkLclVarAsPossiblyStackPointing(lclNum); - stmt->GetRootNode()->gtBashToNOP(); - comp->optMethodFlags |= OMF_HAS_OBJSTACKALLOC; - didStackAllocate = true; - } - - if (canStack) - { - comp->Metrics.StackAllocatedArrays++; - } - else - { - assert(onHeapReason != nullptr); - JITDUMP("Allocating V%02u on the heap: %s\n", lclNum, onHeapReason); - } - } + if (allocType == OAT_NEWARR) + { + assert(data->AsCall()->GetHelperNum() == CORINFO_HELP_NEWARR_1_VC); + //------------------------------------------------------------------------ + // We expect the following expression tree at this point + // STMTx (IL 0x... ???) + // * STORE_LCL_VAR ref + // \--* CALL helper ref + // +--* CNS_INT(h) long + // \--* CNS_INT long + //------------------------------------------------------------------------ + + CallArg* arg = data->AsCall()->gtArgs.GetArgByIndex(0); + GenTree* node = arg->GetNode(); + CORINFO_CLASS_HANDLE clsHnd = (CORINFO_CLASS_HANDLE)node->AsIntConCommon()->IntegralValue(); + GenTree* len = arg->GetNext()->GetNode(); + unsigned int blockSize = 0; + + // Don't attempt to do stack allocations inside basic blocks that may be in a loop. + // + if (!len->IsCnsIntOrI()) + { + onHeapReason = "[non-constant size]"; + canStack = false; + } + else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, (unsigned int)len->AsIntCon()->IconValue(), + &blockSize, &onHeapReason)) + { + // reason set by the call + canStack = false; + } + else + { + JITDUMP("Allocating V%02u on the stack\n", lclNum); + canStack = true; + const unsigned int stackLclNum = + MorphNewArrNodeIntoStackAlloc(data->AsCall(), clsHnd, + (unsigned int)len->AsIntCon()->IconValue(), blockSize, + block, stmt); + m_HeapLocalToStackLocalMap.AddOrUpdate(lclNum, stackLclNum); + comp->Metrics.StackAllocatedArrays++; + } + } + else if (allocType == OAT_NEWOBJ) + { + assert(basicBlockHasNewObj); + //------------------------------------------------------------------------ + // We expect the following expression tree at this point + // STMTx (IL 0x... ???) + // * STORE_LCL_VAR ref + // \--* ALLOCOBJ ref + // \--* CNS_INT(h) long + //------------------------------------------------------------------------ + + CORINFO_CLASS_HANDLE clsHnd = data->AsAllocObj()->gtAllocObjClsHnd; + CORINFO_CLASS_HANDLE stackClsHnd = clsHnd; + const bool isValueClass = comp->info.compCompHnd->isValueClass(clsHnd); + + if (isValueClass) + { + comp->Metrics.NewBoxedValueClassHelperCalls++; + stackClsHnd = comp->info.compCompHnd->getTypeForBoxOnStack(clsHnd); + } + else + { + comp->Metrics.NewRefClassHelperCalls++; + } - if (canonicalAllocObjFound) - { - assert(basicBlockHasNewObj); - //------------------------------------------------------------------------ - // We expect the following expression tree at this point - // STMTx (IL 0x... ???) - // * STORE_LCL_VAR ref - // \--* ALLOCOBJ ref - // \--* CNS_INT(h) long - //------------------------------------------------------------------------ - - GenTreeAllocObj* asAllocObj = data->AsAllocObj(); - unsigned int lclNum = stmtExpr->AsLclVar()->GetLclNum(); - CORINFO_CLASS_HANDLE clsHnd = data->AsAllocObj()->gtAllocObjClsHnd; - CORINFO_CLASS_HANDLE stackClsHnd = clsHnd; - const bool isValueClass = comp->info.compCompHnd->isValueClass(clsHnd); - const char* onHeapReason = nullptr; - bool canStack = false; - - if (isValueClass) - { - comp->Metrics.NewBoxedValueClassHelperCalls++; - stackClsHnd = comp->info.compCompHnd->getTypeForBoxOnStack(clsHnd); - } - else - { - comp->Metrics.NewRefClassHelperCalls++; + if (!CanAllocateLclVarOnStack(lclNum, clsHnd, 0, nullptr, &onHeapReason)) + { + // reason set by the call + canStack = false; + } + else if (stackClsHnd == NO_CLASS_HANDLE) + { + assert(isValueClass); + onHeapReason = "[no class handle for this boxed value class]"; + canStack = false; + } + else + { + JITDUMP("Allocating V%02u on the stack\n", lclNum); + canStack = true; + const unsigned int stackLclNum = + MorphAllocObjNodeIntoStackAlloc(data->AsAllocObj(), stackClsHnd, isValueClass, block, + stmt); + m_HeapLocalToStackLocalMap.AddOrUpdate(lclNum, stackLclNum); + + if (isValueClass) + { + comp->Metrics.StackAllocatedBoxedValueClasses++; + } + else + { + comp->Metrics.StackAllocatedRefClasses++; + } + } + } } - // Don't attempt to do stack allocations inside basic blocks that may be in a loop. - // - if (!IsObjectStackAllocationEnabled()) - { - onHeapReason = "[object stack allocation disabled]"; - canStack = false; - } - else if (basicBlockHasBackwardJump) - { - onHeapReason = "[alloc in loop]"; - canStack = false; - } - else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, 0, nullptr, &onHeapReason)) - { - // reason set by the call - canStack = false; - } - else if (stackClsHnd == NO_CLASS_HANDLE) - { - assert(isValueClass); - onHeapReason = "[no class handle for this boxed value class]"; - canStack = false; - } - else + if (canStack) { - JITDUMP("Allocating V%02u on the stack\n", lclNum); - canStack = true; - const unsigned int stackLclNum = - MorphAllocObjNodeIntoStackAlloc(asAllocObj, stackClsHnd, isValueClass, block, stmt); - m_HeapLocalToStackLocalMap.AddOrUpdate(lclNum, stackLclNum); // We keep the set of possibly-stack-pointing pointers as a superset of the set of - // definitely-stack-pointing pointers. All definitely-stack-pointing pointers are in both sets. + // definitely-stack-pointing pointers. All definitely-stack-pointing pointers are in both + // sets. MarkLclVarAsDefinitelyStackPointing(lclNum); MarkLclVarAsPossiblyStackPointing(lclNum); stmt->GetRootNode()->gtBashToNOP(); comp->optMethodFlags |= OMF_HAS_OBJSTACKALLOC; didStackAllocate = true; } - - if (canStack) - { - if (isValueClass) - { - comp->Metrics.StackAllocatedBoxedValueClasses++; - } - else - { - comp->Metrics.StackAllocatedRefClasses++; - } - } else { assert(onHeapReason != nullptr); JITDUMP("Allocating V%02u on the heap: %s\n", lclNum, onHeapReason); - data = MorphAllocObjNodeIntoHelperCall(asAllocObj); - stmtExpr->AsLclVar()->Data() = data; - stmtExpr->AddAllEffectsFlags(data); + if (allocType == OAT_NEWOBJ) + { + data = MorphAllocObjNodeIntoHelperCall(data->AsAllocObj()); + stmtExpr->AsLclVar()->Data() = data; + stmtExpr->AddAllEffectsFlags(data); + } } } #ifdef DEBUG diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 790c53d8cab66..e068f1a381540 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -22,6 +22,12 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX class ObjectAllocator final : public Phase { typedef SmallHashTable LocalToLocalMap; + enum ObjectAllocationType + { + OAT_NONE, + OAT_NEWOBJ, + OAT_NEWARR + }; //=============================================================================== // Data members From e0fa91e9e8254f0273c39e0d9014123af01e1cfc Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 17:11:26 +0900 Subject: [PATCH 14/65] Remove invalid asserts --- src/coreclr/jit/gentree.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 5b7bc8278cab7..e2c2fbb799255 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -7458,12 +7458,11 @@ struct GenTreeArrAddr : GenTreeUnOp public: GenTreeArrAddr(GenTree* addr, var_types elemType, CORINFO_CLASS_HANDLE elemClassHandle, uint8_t firstElemOffset) - : GenTreeUnOp(GT_ARR_ADDR, TYP_BYREF, addr DEBUGARG(/* largeNode */ false)) + : GenTreeUnOp(GT_ARR_ADDR, addr->TypeGet(), addr DEBUGARG(/* largeNode */ false)) , m_elemClassHandle(elemClassHandle) , m_elemType(elemType) , m_firstElemOffset(firstElemOffset) { - assert(addr->TypeIs(TYP_BYREF)); assert(((elemType == TYP_STRUCT) && (elemClassHandle != NO_CLASS_HANDLE)) || (elemClassHandle == NO_CLASS_HANDLE)); } From 9e0a04fc31b22d95900e7239165fd11736b7d609 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 17:12:44 +0900 Subject: [PATCH 15/65] make compiler happy --- src/coreclr/jit/lclmorph.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index de4c2b2112e6f..3a783951f8eb0 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -840,7 +840,7 @@ class LocalAddressVisitor final : public GenTreeVisitor ssize_t offset = node->AsIndexAddr()->gtElemOffset + node->gtGetOp2()->AsIntCon()->IconValue() * node->AsIndexAddr()->gtElemSize; - if (offset < m_compiler->lvaLclSize(TopValue(1).LclNum())) + if (offset < static_cast(m_compiler->lvaLclSize(TopValue(1).LclNum()))) { if (FitsIn(offset) && TopValue(2).AddOffset(TopValue(1), static_cast(offset))) From ae822f81d93bda42daa1034bdd833908d05ebab6 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 19:03:51 +0900 Subject: [PATCH 16/65] Address review feedbacks --- src/coreclr/jit/lclmorph.cpp | 135 ++++++++++++++++---------------- src/coreclr/jit/objectalloc.cpp | 50 ++++++++---- src/coreclr/jit/objectalloc.h | 20 ++--- 3 files changed, 111 insertions(+), 94 deletions(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 3a783951f8eb0..406e49e500975 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -826,74 +826,6 @@ class LocalAddressVisitor final : public GenTreeVisitor switch (node->OperGet()) { - case GT_INDEX_ADDR: - { - assert(TopValue(2).Node() == node); - assert(TopValue(1).Node() == node->gtGetOp1()); - assert(TopValue(0).Node() == node->gtGetOp2()); - - if (node->gtGetOp1()->OperIs(GT_LCL_VAR, GT_LCL_ADDR) && node->gtGetOp2()->IsCnsIntOrI()) - { - if (TopValue(1).IsAddress() && - m_compiler->lvaGetDesc(TopValue(1).LclNum())->GetLayout()->IsBlockLayout()) - { - ssize_t offset = node->AsIndexAddr()->gtElemOffset + - node->gtGetOp2()->AsIntCon()->IconValue() * node->AsIndexAddr()->gtElemSize; - - if (offset < static_cast(m_compiler->lvaLclSize(TopValue(1).LclNum()))) - { - if (FitsIn(offset) && - TopValue(2).AddOffset(TopValue(1), static_cast(offset))) - { - INDEBUG(TopValue(0).Consume()); - PopValue(); - PopValue(); - break; - } - } - else - { - GenTree* gtThrow = - m_compiler->gtNewMustThrowException(CORINFO_HELP_RNGCHKFAIL, node->TypeGet(), - node->AsIndexAddr()->gtStructElemClass); - m_compiler->lvaGetDesc(gtThrow->AsOp()->gtOp2->AsLclVarCommon())->incLvRefCnt(1, RCS_EARLY); - *use = gtThrow; - m_stmtModified = true; - } - } - } - - EscapeValue(TopValue(0), node); - PopValue(); - EscapeValue(TopValue(0), node); - PopValue(); - break; - } - case GT_ARR_LENGTH: - { - assert(TopValue(1).Node() == node); - assert(TopValue(0).Node() == node->gtGetOp1()); - - if (node->gtGetOp1()->OperIs(GT_LCL_VAR, GT_LCL_ADDR)) - { - if (TopValue(0).IsAddress() && - m_compiler->lvaGetDesc(TopValue(0).LclNum())->GetLayout()->IsBlockLayout()) - { - GenTree* gtLclFld = - m_compiler->gtNewLclFldNode(TopValue(0).LclNum(), TYP_INT, OFFSETOF__CORINFO_Array__length); - SequenceLocal(gtLclFld->AsLclVarCommon()); - *use = gtLclFld; - m_stmtModified = true; - INDEBUG(TopValue(0).Consume()); - PopValue(); - break; - } - } - - EscapeValue(TopValue(0), node); - PopValue(); - break; - } case GT_STORE_LCL_FLD: if (node->IsPartialLclFld(m_compiler)) { @@ -1174,6 +1106,73 @@ class LocalAddressVisitor final : public GenTreeVisitor break; } + case GT_INDEX_ADDR: + { + assert(TopValue(2).Node() == node); + assert(TopValue(1).Node() == node->gtGetOp1()); + assert(TopValue(0).Node() == node->gtGetOp2()); + + if (node->gtGetOp2()->IsCnsIntOrI() && TopValue(1).IsAddress()) + { + ssize_t offset = node->AsIndexAddr()->gtElemOffset + + node->gtGetOp2()->AsIntCon()->IconValue() * node->AsIndexAddr()->gtElemSize; + + if (offset < static_cast(m_compiler->lvaLclExactSize(TopValue(1).LclNum()))) + { + if (FitsIn(offset) && + TopValue(2).AddOffset(TopValue(1), static_cast(offset))) + { + INDEBUG(TopValue(0).Consume()); + PopValue(); + PopValue(); + break; + } + } + else + { + *use = m_compiler->gtNewOperNode(GT_COMMA, node->TypeGet(), + m_compiler->gtNewHelperCallNode(CORINFO_HELP_RNGCHKFAIL, + TYP_VOID), + m_compiler->gtNewIconNode(0, TYP_BYREF)); + m_stmtModified = true; + INDEBUG(TopValue(0).Consume()); + PopValue(); + INDEBUG(TopValue(0).Consume()); + PopValue(); + break; + } + } + + EscapeValue(TopValue(0), node); + PopValue(); + EscapeValue(TopValue(0), node); + PopValue(); + break; + } + + case GT_ARR_LENGTH: + { + assert(TopValue(1).Node() == node); + assert(TopValue(0).Node() == node->gtGetOp1()); + + if (TopValue(0).IsAddress()) + { + GenTree* gtLclFld = + m_compiler->gtNewLclFldNode(TopValue(0).LclNum(), TYP_INT, + TopValue(0).Offset() + OFFSETOF__CORINFO_Array__length); + SequenceLocal(gtLclFld->AsLclVarCommon()); + *use = gtLclFld; + m_stmtModified = true; + INDEBUG(TopValue(0).Consume()); + PopValue(); + break; + } + + EscapeValue(TopValue(0), node); + PopValue(); + break; + } + default: while (TopValue(0).Node() != node) { diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 8b747df0e0bc5..560cedbf4a6fe 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -400,14 +400,10 @@ bool ObjectAllocator::MorphAllocObjNodes() { allocType = OAT_NEWOBJ; } - else if (data->IsHelperCall()) + else if (data->IsCall() && data->AsCall()->IsHelperCall(comp, CORINFO_HELP_NEWARR_1_VC) && + data->AsCall()->gtArgs.GetUserArgByIndex(0)->GetNode()->IsCnsIntOrI()) { - GenTreeCall* call = data->AsCall(); - if (call->GetHelperNum() == CORINFO_HELP_NEWARR_1_VC && - call->gtArgs.GetArgByIndex(0)->GetNode()->IsCnsIntOrI()) - { - allocType = OAT_NEWARR; - } + allocType = OAT_NEWARR; } } @@ -445,13 +441,21 @@ bool ObjectAllocator::MorphAllocObjNodes() CallArg* arg = data->AsCall()->gtArgs.GetArgByIndex(0); GenTree* node = arg->GetNode(); - CORINFO_CLASS_HANDLE clsHnd = (CORINFO_CLASS_HANDLE)node->AsIntConCommon()->IntegralValue(); - GenTree* len = arg->GetNext()->GetNode(); - unsigned int blockSize = 0; + bool isExact = false; + bool isNonNull = false; + CORINFO_CLASS_HANDLE clsHnd = + comp->gtGetHelperCallClassHandle(data->AsCall(), &isExact, &isNonNull); + GenTree* len = arg->GetNext()->GetNode(); + unsigned int blockSize = 0; - // Don't attempt to do stack allocations inside basic blocks that may be in a loop. - // - if (!len->IsCnsIntOrI()) + comp->Metrics.NewArrayHelperCalls++; + + if (!isExact || !isNonNull) + { + onHeapReason = "[array type is either non-exact or null]"; + canStack = false; + } + else if (!len->IsCnsIntOrI()) { onHeapReason = "[non-constant size]"; canStack = false; @@ -619,6 +623,24 @@ GenTree* ObjectAllocator::MorphAllocObjNodeIntoHelperCall(GenTreeAllocObj* alloc return helperCall; } +//------------------------------------------------------------------------ +// MorphNewArrNodeIntoStackAlloc: Morph a GT_CALL CORINFO_HELP_NEWARR_1_VC +// node into stack allocation. +// +// Arguments: +// newArr - GT_CALL that will be replaced by helper call. +// clsHndclsHnd - class representing the type of the array +// length - length of the array +// blockSize - size of the layout +// block - a basic block where newArr is +// stmt - a statement where newArr is +// +// Return Value: +// local num for the new stack allocated local +// +// Notes: +// This function can insert additional statements before stmt. +// unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* newArr, CORINFO_CLASS_HANDLE clsHnd, unsigned int length, @@ -819,7 +841,6 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_NE: case GT_NULLCHECK: case GT_ARR_LENGTH: - case GT_INDEX_ADDR: canLclVarEscapeViaParentStack = false; break; @@ -836,6 +857,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_ADD: case GT_BOX: case GT_FIELD_ADDR: + case GT_INDEX_ADDR: // Check whether the local escapes via its grandparent. ++parentIndex; keepChecking = true; diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index e068f1a381540..3975995e5cf04 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -167,22 +167,18 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack( } else if (comp->info.compCompHnd->isSDArray(clsHnd)) { - CorInfoType type = comp->info.compCompHnd->getChildType(clsHnd, &clsHnd); - if (type != CORINFO_TYPE_UNDEF && clsHnd == nullptr) - { - // This is an array of primitive types - classSize = (unsigned int)OFFSETOF__CORINFO_Array__data + genTypeSize(JITtype2varType(type)) * length; - } - else if (comp->info.compCompHnd->isValueClass(clsHnd)) - { - classSize = - (unsigned int)OFFSETOF__CORINFO_Array__data + comp->info.compCompHnd->getClassSize(clsHnd) * length; - } - else + CORINFO_CLASS_HANDLE elemClsHnd = NO_CLASS_HANDLE; + CorInfoType corType = comp->info.compCompHnd->getChildType(clsHnd, &elemClsHnd); + var_types type = JITtype2varType(corType); + ClassLayout* elemLayout = type == TYP_STRUCT ? comp->typGetObjLayout(elemClsHnd) : nullptr; + if (varTypeIsGC(type) || ((elemLayout != nullptr) && elemLayout->HasGCPtr())) { *reason = "[array contains gc refs]"; return false; } + + unsigned elemSize = elemLayout != nullptr ? elemLayout->GetSize() : genTypeSize(type); + classSize = (unsigned int)OFFSETOF__CORINFO_Array__data + elemSize * length; } else { From a4588bb8ac81e0287636a668c7c99ecd5ccc7250 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 20:20:40 +0900 Subject: [PATCH 17/65] Fix INDEX_ADDR and add Sub --- src/coreclr/jit/objectalloc.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 560cedbf4a6fe..d795fdb1ae6ba 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -855,6 +855,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_COLON: case GT_QMARK: case GT_ADD: + case GT_SUB: case GT_BOX: case GT_FIELD_ADDR: case GT_INDEX_ADDR: @@ -939,7 +940,6 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p case GT_NE: case GT_NULLCHECK: case GT_ARR_LENGTH: - case GT_INDEX_ADDR: break; case GT_COMMA: @@ -951,7 +951,9 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p FALLTHROUGH; case GT_QMARK: case GT_ADD: + case GT_SUB: case GT_FIELD_ADDR: + case GT_INDEX_ADDR: if (parent->TypeGet() == TYP_REF) { parent->ChangeType(newType); From 32b9e2633c76681fee7ac89544d76cbf54b84b17 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 20:29:06 +0900 Subject: [PATCH 18/65] Support IsAddressLessThan and its friends --- src/coreclr/jit/objectalloc.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index d795fdb1ae6ba..d398d390dd8c8 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -839,6 +839,10 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_EQ: case GT_NE: + case GT_LT: + case GT_GT: + case GT_LE: + case GT_GE: case GT_NULLCHECK: case GT_ARR_LENGTH: canLclVarEscapeViaParentStack = false; @@ -938,6 +942,10 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p case GT_EQ: case GT_NE: + case GT_LT: + case GT_GT: + case GT_LE: + case GT_GE: case GT_NULLCHECK: case GT_ARR_LENGTH: break; From 39d1ad9a2ba71031550f92259c1626e0ec497404 Mon Sep 17 00:00:00 2001 From: Steven He Date: Tue, 16 Jul 2024 23:24:00 +0900 Subject: [PATCH 19/65] Fix assertions --- src/coreclr/jit/objectalloc.cpp | 22 +++++++++++++++------- src/coreclr/jit/objectalloc.h | 7 +++++++ src/coreclr/jit/promotiondecomposition.cpp | 5 +++++ 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index d398d390dd8c8..c6fe84fd48abd 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -336,17 +336,25 @@ void ObjectAllocator::ComputeStackObjectPointers(BitVecTraits* bitVecTraits) { // Check if we know what is assigned to this pointer. unsigned bitCount = BitVecOps::Count(bitVecTraits, m_ConnGraphAdjacencyMatrix[lclNum]); - assert(bitCount <= 1); - if (bitCount == 1) + if (bitCount > 0) { BitVecOps::Iter iter(bitVecTraits, m_ConnGraphAdjacencyMatrix[lclNum]); - unsigned rhsLclNum = 0; - iter.NextElem(&rhsLclNum); + unsigned rhsLclNum = 0; + bool definitelyStackPointing = true; - if (DoesLclVarPointToStack(rhsLclNum)) + while (iter.NextElem(&rhsLclNum)) { - // The only store to lclNum local is the definitely-stack-pointing - // rhsLclNum local so lclNum local is also definitely-stack-pointing. + if (!DoesLclVarPointToStack(rhsLclNum)) + { + definitelyStackPointing = false; + break; + } + } + + if (definitelyStackPointing) + { + // All stores to lclNum local are the definitely-stack-pointing + // rhsLclNum locals so lclNum local is also definitely-stack-pointing. MarkLclVarAsDefinitelyStackPointing(lclNum); } } diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 3975995e5cf04..abd4e8e4067d8 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -140,6 +140,7 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack( bool enableBoxedValueClasses = true; bool enableRefClasses = true; + bool enableArrays = true; *reason = "[ok]"; #ifdef DEBUG @@ -167,6 +168,12 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack( } else if (comp->info.compCompHnd->isSDArray(clsHnd)) { + if (!enableArrays) + { + *reason = "[disabled by config]"; + return false; + } + CORINFO_CLASS_HANDLE elemClsHnd = NO_CLASS_HANDLE; CorInfoType corType = comp->info.compCompHnd->getChildType(clsHnd, &elemClsHnd); var_types type = JITtype2varType(corType); diff --git a/src/coreclr/jit/promotiondecomposition.cpp b/src/coreclr/jit/promotiondecomposition.cpp index bf89852547fd7..8be998f74c16d 100644 --- a/src/coreclr/jit/promotiondecomposition.cpp +++ b/src/coreclr/jit/promotiondecomposition.cpp @@ -541,6 +541,11 @@ class DecompositionPlan if (addr != nullptr) { + if (addr->IsInvariant()) + { + indirFlags |= GTF_IND_INVARIANT; + } + for (int i = 0; i < m_entries.Height(); i++) { if (!CanSkipEntry(m_entries.BottomRef(i), dstDeaths, remainderStrategy)) From 9f408b2562f782de46c9d4d0b9be87f9747dfa89 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 17 Jul 2024 00:29:13 +0900 Subject: [PATCH 20/65] Use new overload --- src/coreclr/jit/objectalloc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index c6fe84fd48abd..8754d90932e08 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -408,7 +408,7 @@ bool ObjectAllocator::MorphAllocObjNodes() { allocType = OAT_NEWOBJ; } - else if (data->IsCall() && data->AsCall()->IsHelperCall(comp, CORINFO_HELP_NEWARR_1_VC) && + else if (data->IsHelperCall(comp, CORINFO_HELP_NEWARR_1_VC) && data->AsCall()->gtArgs.GetUserArgByIndex(0)->GetNode()->IsCnsIntOrI()) { allocType = OAT_NEWARR; From 418a62bb6ac72313cbe87b9519d4f5b87ffe6544 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 16 Jul 2024 18:11:54 +0200 Subject: [PATCH 21/65] JIT: Remove GTF_IND_INVARIANT and GTF_IND_NONFAULTING flags checking These flags are strictly optimizations. Having them required to be set for certain indirs based on context of the operand introduces IR invariants that are annoying to work with since it suddenly becomes necessary for all transformations to consider "did we just introduce this IR shape?". Instead, handle these patterns during morph as the optimization it is and remove the strict flags checking from `fgDebugCheckFlags`. --- src/coreclr/jit/fgdiagnostic.cpp | 30 ---------------------------- src/coreclr/jit/gentree.cpp | 34 +++++++++++++++++++++++++------- src/coreclr/jit/morph.cpp | 12 +++++++++-- 3 files changed, 37 insertions(+), 39 deletions(-) diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 45cdcfea0f46b..4818d85f8381e 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -3361,36 +3361,6 @@ void Compiler::fgDebugCheckFlags(GenTree* tree, BasicBlock* block) case GT_IND: // Do we have a constant integer address as op1 that is also a handle? - if (op1->IsIconHandle()) - { - if ((tree->gtFlags & GTF_IND_INVARIANT) != 0) - { - actualFlags |= GTF_IND_INVARIANT; - } - if ((tree->gtFlags & GTF_IND_NONFAULTING) != 0) - { - actualFlags |= GTF_IND_NONFAULTING; - } - - GenTreeFlags handleKind = op1->GetIconHandleFlag(); - - // Some of these aren't handles to invariant data... - if (GenTree::HandleKindDataIsInvariant(handleKind) && (handleKind != GTF_ICON_FTN_ADDR)) - { - expectedFlags |= GTF_IND_INVARIANT; - } - else - { - // For statics, we expect the GTF_GLOB_REF to be set. However, we currently - // fail to set it in a number of situations, and so this check is disabled. - // TODO: enable checking of GTF_GLOB_REF. - // expectedFlags |= GTF_GLOB_REF; - } - - // Currently we expect all indirections with constant addresses to be nonfaulting. - expectedFlags |= GTF_IND_NONFAULTING; - } - assert(((tree->gtFlags & GTF_IND_TGT_NOT_HEAP) == 0) || ((tree->gtFlags & GTF_IND_TGT_HEAP) == 0)); break; diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index f7da7729d2375..0b39412bc5334 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -10631,8 +10631,7 @@ void GenTree::SetIndirExceptionFlags(Compiler* comp) //------------------------------------------------------------------------------ // HandleKindDataIsInvariant: Returns true if the data referred to by a handle -// address is guaranteed to be invariant. Note that GTF_ICON_FTN_ADDR handles may -// or may not point to invariant data. +// address is guaranteed to be invariant. // // Arguments: // flags - GenTree flags for handle. @@ -10643,11 +10642,32 @@ bool GenTree::HandleKindDataIsInvariant(GenTreeFlags flags) GenTreeFlags handleKind = flags & GTF_ICON_HDL_MASK; assert(handleKind != GTF_EMPTY); - // All handle types are assumed invariant except those specifically listed here. - - return (handleKind != GTF_ICON_STATIC_HDL) && // Pointer to a mutable class Static variable - (handleKind != GTF_ICON_BBC_PTR) && // Pointer to a mutable basic block count value - (handleKind != GTF_ICON_GLOBAL_PTR); // Pointer to mutable data from the VM state + switch (handleKind) + { + case GTF_ICON_SCOPE_HDL: + case GTF_ICON_CLASS_HDL: + case GTF_ICON_METHOD_HDL: + case GTF_ICON_FIELD_HDL: + case GTF_ICON_STR_HDL: + case GTF_ICON_CONST_PTR: + case GTF_ICON_VARG_HDL: + case GTF_ICON_PINVKI_HDL: + case GTF_ICON_TOKEN_HDL: + case GTF_ICON_TLS_HDL: + case GTF_ICON_CIDMID_HDL: + case GTF_ICON_FIELD_SEQ: + case GTF_ICON_STATIC_ADDR_PTR: + case GTF_ICON_SECREL_OFFSET: + case GTF_ICON_TLSGD_OFFSET: + return true; + case GTF_ICON_FTN_ADDR: + case GTF_ICON_GLOBAL_PTR: + case GTF_ICON_STATIC_HDL: + case GTF_ICON_BBC_PTR: + case GTF_ICON_STATIC_BOX_PTR: + default: + return false; + } } #ifdef DEBUG diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 8e510cdb99fc1..1045d96258bd6 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -8679,9 +8679,17 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA case GT_IND: { - if (op1->IsIconHandle(GTF_ICON_OBJ_HDL)) + if (op1->IsIconHandle()) { - tree->gtFlags |= (GTF_IND_INVARIANT | GTF_IND_NONFAULTING | GTF_IND_NONNULL); + // All indirections with (handle) constant addresses are + // nonfaulting. + tree->gtFlags |= GTF_IND_NONFAULTING; + + // We know some handle types always point to invariant data. + if (GenTree::HandleKindDataIsInvariant(op1->GetIconHandleFlag())) + { + tree->gtFlags |= GTF_IND_INVARIANT; + } } GenTree* optimizedTree = fgMorphFinalizeIndir(tree->AsIndir()); From 4572408c20a95e905b1e3a1b8623d5127f6db57f Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 16 Jul 2024 18:16:50 +0200 Subject: [PATCH 22/65] Remove old comment --- src/coreclr/jit/fgdiagnostic.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 4818d85f8381e..9f1a72afce63e 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -3360,7 +3360,6 @@ void Compiler::fgDebugCheckFlags(GenTree* tree, BasicBlock* block) break; case GT_IND: - // Do we have a constant integer address as op1 that is also a handle? assert(((tree->gtFlags & GTF_IND_TGT_NOT_HEAP) == 0) || ((tree->gtFlags & GTF_IND_TGT_HEAP) == 0)); break; From 925576242fecf5b8d579d864394fe71fea5608f9 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 17 Jul 2024 00:57:38 +0900 Subject: [PATCH 23/65] Expose jitconfig --- src/coreclr/jit/jitconfigvalues.h | 1 + src/coreclr/jit/objectalloc.cpp | 2 +- src/coreclr/jit/objectalloc.h | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 50824303d2f78..d26b3ca87cd9a 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -650,6 +650,7 @@ CONFIG_STRING(JitObjectStackAllocationRange, W("JitObjectStackAllocationRange")) RELEASE_CONFIG_INTEGER(JitObjectStackAllocation, W("JitObjectStackAllocation"), 1) RELEASE_CONFIG_INTEGER(JitObjectStackAllocationRefClass, W("JitObjectStackAllocationRefClass"), 1) RELEASE_CONFIG_INTEGER(JitObjectStackAllocationBoxedValueClass, W("JitObjectStackAllocationBoxedValueClass"), 1) +RELEASE_CONFIG_INTEGER(JitObjectStackAllocationArray, W("JitObjectStackAllocationArray"), 1) RELEASE_CONFIG_INTEGER(JitEECallTimingInfo, W("JitEECallTimingInfo"), 0) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 8754d90932e08..1d653119784ea 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -442,7 +442,7 @@ bool ObjectAllocator::MorphAllocObjNodes() // We expect the following expression tree at this point // STMTx (IL 0x... ???) // * STORE_LCL_VAR ref - // \--* CALL helper ref + // \--* CALL help ref CORINFO_HELP_NEWARR_1_VC // +--* CNS_INT(h) long // \--* CNS_INT long //------------------------------------------------------------------------ diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index abd4e8e4067d8..4b1f77895d630 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -146,6 +146,7 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack( #ifdef DEBUG enableBoxedValueClasses = (JitConfig.JitObjectStackAllocationBoxedValueClass() != 0); enableRefClasses = (JitConfig.JitObjectStackAllocationRefClass() != 0); + enableArrays = (JitConfig.JitObjectStackAllocationArray() != 0); #endif unsigned int classSize = 0; From 1af84b9e6196837563f03c4abbaa20dfa1f77ce0 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 16 Jul 2024 18:53:19 +0200 Subject: [PATCH 24/65] Remove another assert --- src/coreclr/jit/gentree.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 0b39412bc5334..60ff8d61b65cd 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -7571,8 +7571,6 @@ GenTree* Compiler::gtNewIndOfIconHandleNode(var_types indType, size_t addr, GenT if (isInvariant) { - assert(GenTree::HandleKindDataIsInvariant(iconFlags)); - // This indirection also is invariant. indirFlags |= GTF_IND_INVARIANT; From 629c793fb9620803ba49acca0a242cf1a39fa3a7 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 16 Jul 2024 19:34:55 +0200 Subject: [PATCH 25/65] Count --- src/coreclr/jit/gentree.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 757ce23912d73..c3eb11fbffa68 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -513,18 +513,18 @@ enum GenTreeFlags : unsigned int GTF_ICON_FIELD_HDL = 0x04000000, // GT_CNS_INT -- constant is a field handle GTF_ICON_STATIC_HDL = 0x05000000, // GT_CNS_INT -- constant is a handle to static data GTF_ICON_STR_HDL = 0x06000000, // GT_CNS_INT -- constant is a pinned handle pointing to a string object - GTF_ICON_OBJ_HDL = 0x12000000, // GT_CNS_INT -- constant is an object handle (e.g. frozen string or Type object) - GTF_ICON_CONST_PTR = 0x07000000, // GT_CNS_INT -- constant is a pointer to immutable data, (e.g. IAT_PPVALUE) - GTF_ICON_GLOBAL_PTR = 0x08000000, // GT_CNS_INT -- constant is a pointer to mutable data (e.g. from the VM state) - GTF_ICON_VARG_HDL = 0x09000000, // GT_CNS_INT -- constant is a var arg cookie handle - GTF_ICON_PINVKI_HDL = 0x0A000000, // GT_CNS_INT -- constant is a pinvoke calli handle - GTF_ICON_TOKEN_HDL = 0x0B000000, // GT_CNS_INT -- constant is a token handle (other than class, method or field) - GTF_ICON_TLS_HDL = 0x0C000000, // GT_CNS_INT -- constant is a TLS ref with offset - GTF_ICON_FTN_ADDR = 0x0D000000, // GT_CNS_INT -- constant is a function address - GTF_ICON_CIDMID_HDL = 0x0E000000, // GT_CNS_INT -- constant is a class ID or a module ID - GTF_ICON_BBC_PTR = 0x0F000000, // GT_CNS_INT -- constant is a basic block count pointer - GTF_ICON_STATIC_BOX_PTR = 0x10000000, // GT_CNS_INT -- constant is an address of the box for a STATIC_IN_HEAP field - GTF_ICON_FIELD_SEQ = 0x11000000, // <--------> -- constant is a FieldSeq* (used only as VNHandle) + GTF_ICON_OBJ_HDL = 0x08000000, // GT_CNS_INT -- constant is an object handle (e.g. frozen string or Type object) + GTF_ICON_CONST_PTR = 0x08000000, // GT_CNS_INT -- constant is a pointer to immutable data, (e.g. IAT_PPVALUE) + GTF_ICON_GLOBAL_PTR = 0x09000000, // GT_CNS_INT -- constant is a pointer to mutable data (e.g. from the VM state) + GTF_ICON_VARG_HDL = 0x0A000000, // GT_CNS_INT -- constant is a var arg cookie handle + GTF_ICON_PINVKI_HDL = 0x0B000000, // GT_CNS_INT -- constant is a pinvoke calli handle + GTF_ICON_TOKEN_HDL = 0x0C000000, // GT_CNS_INT -- constant is a token handle (other than class, method or field) + GTF_ICON_TLS_HDL = 0x0D000000, // GT_CNS_INT -- constant is a TLS ref with offset + GTF_ICON_FTN_ADDR = 0x0E000000, // GT_CNS_INT -- constant is a function address + GTF_ICON_CIDMID_HDL = 0x0F000000, // GT_CNS_INT -- constant is a class ID or a module ID + GTF_ICON_BBC_PTR = 0x10000000, // GT_CNS_INT -- constant is a basic block count pointer + GTF_ICON_STATIC_BOX_PTR = 0x11000000, // GT_CNS_INT -- constant is an address of the box for a STATIC_IN_HEAP field + GTF_ICON_FIELD_SEQ = 0x12000000, // <--------> -- constant is a FieldSeq* (used only as VNHandle) GTF_ICON_STATIC_ADDR_PTR = 0x13000000, // GT_CNS_INT -- constant is a pointer to a static base address GTF_ICON_SECREL_OFFSET = 0x14000000, // GT_CNS_INT -- constant is an offset in a certain section. GTF_ICON_TLSGD_OFFSET = 0x15000000, // GT_CNS_INT -- constant is an argument to tls_get_addr. From b578203f3ab0e1dee81c8235c03612f3b2f9cac6 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 16 Jul 2024 19:35:26 +0200 Subject: [PATCH 26/65] Try 2 at counting --- src/coreclr/jit/gentree.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index c3eb11fbffa68..94e88caedc7c7 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -513,7 +513,7 @@ enum GenTreeFlags : unsigned int GTF_ICON_FIELD_HDL = 0x04000000, // GT_CNS_INT -- constant is a field handle GTF_ICON_STATIC_HDL = 0x05000000, // GT_CNS_INT -- constant is a handle to static data GTF_ICON_STR_HDL = 0x06000000, // GT_CNS_INT -- constant is a pinned handle pointing to a string object - GTF_ICON_OBJ_HDL = 0x08000000, // GT_CNS_INT -- constant is an object handle (e.g. frozen string or Type object) + GTF_ICON_OBJ_HDL = 0x07000000, // GT_CNS_INT -- constant is an object handle (e.g. frozen string or Type object) GTF_ICON_CONST_PTR = 0x08000000, // GT_CNS_INT -- constant is a pointer to immutable data, (e.g. IAT_PPVALUE) GTF_ICON_GLOBAL_PTR = 0x09000000, // GT_CNS_INT -- constant is a pointer to mutable data (e.g. from the VM state) GTF_ICON_VARG_HDL = 0x0A000000, // GT_CNS_INT -- constant is a var arg cookie handle From b4445f6e450f30f1f5557f7ceaf196dc365cd003 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 17 Jul 2024 02:56:07 +0900 Subject: [PATCH 27/65] Introduce BBF_HAS_NEWARR --- src/coreclr/jit/block.cpp | 1 + src/coreclr/jit/block.h | 3 ++- src/coreclr/jit/fgdiagnostic.cpp | 4 ++++ src/coreclr/jit/importer.cpp | 1 + src/coreclr/jit/objectalloc.cpp | 3 ++- 5 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index be2ba15e25469..e9ad2d072ec55 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -574,6 +574,7 @@ void BasicBlock::dspFlags() const {BBF_HAS_IDX_LEN, "idxlen"}, {BBF_HAS_MD_IDX_LEN, "mdidxlen"}, {BBF_HAS_NEWOBJ, "newobj"}, + {BBF_HAS_NEWARR, "newarr"}, {BBF_HAS_NULLCHECK, "nullcheck"}, {BBF_BACKWARD_JUMP, "bwd"}, {BBF_BACKWARD_JUMP_TARGET, "bwd-target"}, diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index e0989df456900..ea95994d580d0 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -462,7 +462,8 @@ enum BasicBlockFlags : uint64_t BBF_NO_CSE_IN = MAKE_BBFLAG(38), // Block should kill off any incoming CSE BBF_CAN_ADD_PRED = MAKE_BBFLAG(39), // Ok to add pred edge to this block, even when "safe" edge creation disabled BBF_HAS_VALUE_PROFILE = MAKE_BBFLAG(40), // Block has a node that needs a value probing - + + BBF_HAS_NEWARR = MAKE_BBFLAG(41), // BB contains 'new' of an array type. // The following are sets of flags. // Flags to update when two blocks are compacted diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 45cdcfea0f46b..951503ffef45e 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -973,6 +973,10 @@ bool Compiler::fgDumpFlowGraph(Phases phase, PhasePosition pos) { fprintf(fgxFile, "\n hot=\"true\""); } + if (block->HasFlag(BBF_HAS_NEWARR)) + { + fprintf(fgxFile, "\n callsNewArr=\"true\""); + } if (block->HasFlag(BBF_HAS_NEWOBJ)) { fprintf(fgxFile, "\n callsNew=\"true\""); diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index e2a9ec8d48384..87f2210bd40ef 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -9532,6 +9532,7 @@ void Compiler::impImportBlockCode(BasicBlock* block) op1->AsCall()->compileTimeHelperArgumentHandle = (CORINFO_GENERIC_HANDLE)resolvedToken.hClass; // Remember that this function contains 'new' of an SD array. + block->SetFlags(BBF_HAS_NEWARR); optMethodFlags |= OMF_HAS_NEWARRAY; // We assign the newly allocated object (by a GT_CALL to newarr node) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 1d653119784ea..4b2a72405ebdc 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -385,9 +385,10 @@ bool ObjectAllocator::MorphAllocObjNodes() for (BasicBlock* const block : comp->Blocks()) { const bool basicBlockHasNewObj = block->HasFlag(BBF_HAS_NEWOBJ); + const bool basicBlockHasNewArr = block->HasFlag(BBF_HAS_NEWARR); const bool basicBlockHasBackwardJump = block->HasFlag(BBF_BACKWARD_JUMP); #ifndef DEBUG - if (!basicBlockHasNewObj) + if (!basicBlockHasNewObj && !basicBlockHasNewArr) { continue; } From af9c40e109fbe9404d31ac6a138984e7f8f61a88 Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 17 Jul 2024 03:04:10 +0900 Subject: [PATCH 28/65] Early exit on debug as well --- src/coreclr/jit/objectalloc.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 4b2a72405ebdc..b84a9b7b809a5 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -387,12 +387,11 @@ bool ObjectAllocator::MorphAllocObjNodes() const bool basicBlockHasNewObj = block->HasFlag(BBF_HAS_NEWOBJ); const bool basicBlockHasNewArr = block->HasFlag(BBF_HAS_NEWARR); const bool basicBlockHasBackwardJump = block->HasFlag(BBF_BACKWARD_JUMP); -#ifndef DEBUG + if (!basicBlockHasNewObj && !basicBlockHasNewArr) { continue; } -#endif // DEBUG for (Statement* const stmt : block->Statements()) { From 8b54f5ad449af86cacdbd2acb0e2c8272df283fb Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 17 Jul 2024 03:11:23 +0900 Subject: [PATCH 29/65] Update computed flags --- src/coreclr/jit/block.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index ea95994d580d0..c70c4abdfe289 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -469,7 +469,7 @@ enum BasicBlockFlags : uint64_t // Flags to update when two blocks are compacted BBF_COMPACT_UPD = BBF_GC_SAFE_POINT | BBF_NEEDS_GCPOLL | BBF_HAS_JMP | BBF_HAS_IDX_LEN | BBF_HAS_MD_IDX_LEN | BBF_BACKWARD_JUMP | \ - BBF_HAS_NEWOBJ | BBF_HAS_NULLCHECK | BBF_HAS_MDARRAYREF | BBF_LOOP_HEAD, + BBF_HAS_NEWOBJ | BBF_HAS_NEWARR | BBF_HAS_NULLCHECK | BBF_HAS_MDARRAYREF | BBF_LOOP_HEAD, // Flags a block should not have had before it is split. @@ -487,7 +487,7 @@ enum BasicBlockFlags : uint64_t // For example, the bottom block might or might not have BBF_HAS_NULLCHECK, but we assume it has BBF_HAS_NULLCHECK. // TODO: Should BBF_RUN_RARELY be added to BBF_SPLIT_GAINED ? - BBF_SPLIT_GAINED = BBF_DONT_REMOVE | BBF_HAS_JMP | BBF_BACKWARD_JUMP | BBF_HAS_IDX_LEN | BBF_HAS_MD_IDX_LEN | BBF_PROF_WEIGHT | \ + BBF_SPLIT_GAINED = BBF_DONT_REMOVE | BBF_HAS_JMP | BBF_BACKWARD_JUMP | BBF_HAS_IDX_LEN | BBF_HAS_MD_IDX_LEN | BBF_PROF_WEIGHT | BBF_HAS_NEWARR | \ BBF_HAS_NEWOBJ | BBF_KEEP_BBJ_ALWAYS | BBF_CLONED_FINALLY_END | BBF_HAS_NULLCHECK | BBF_HAS_HISTOGRAM_PROFILE | BBF_HAS_VALUE_PROFILE | BBF_HAS_MDARRAYREF | BBF_NEEDS_GCPOLL, // Flags that must be propagated to a new block if code is copied from a block to a new block. These are flags that @@ -495,7 +495,7 @@ enum BasicBlockFlags : uint64_t // have actually copied one of these type of tree nodes, but if we only copy a portion of the block's statements, // we don't know (unless we actually pay close attention during the copy). - BBF_COPY_PROPAGATE = BBF_HAS_NEWOBJ | BBF_HAS_NULLCHECK | BBF_HAS_IDX_LEN | BBF_HAS_MD_IDX_LEN | BBF_HAS_MDARRAYREF, + BBF_COPY_PROPAGATE = BBF_HAS_NEWOBJ | BBF_HAS_NEWARR | BBF_HAS_NULLCHECK | BBF_HAS_IDX_LEN | BBF_HAS_MD_IDX_LEN | BBF_HAS_MDARRAYREF, }; FORCEINLINE From 6eca58d0c8017e841eade106ad2cd63fe8e0c23c Mon Sep 17 00:00:00 2001 From: Steven He Date: Wed, 17 Jul 2024 20:30:23 +0900 Subject: [PATCH 30/65] Partially revert 39d1ad9 --- src/coreclr/jit/promotiondecomposition.cpp | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/coreclr/jit/promotiondecomposition.cpp b/src/coreclr/jit/promotiondecomposition.cpp index 8be998f74c16d..bf89852547fd7 100644 --- a/src/coreclr/jit/promotiondecomposition.cpp +++ b/src/coreclr/jit/promotiondecomposition.cpp @@ -541,11 +541,6 @@ class DecompositionPlan if (addr != nullptr) { - if (addr->IsInvariant()) - { - indirFlags |= GTF_IND_INVARIANT; - } - for (int i = 0; i < m_entries.Height(); i++) { if (!CanSkipEntry(m_entries.BottomRef(i), dstDeaths, remainderStrategy)) From 49d85091f20ea7d90dfde8e7227b1281038a20f6 Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 18 Jul 2024 00:45:42 +0900 Subject: [PATCH 31/65] Reuse existing comma node --- src/coreclr/jit/gentree.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index e036d9cc1b58f..5f799e7d5010f 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -17139,7 +17139,15 @@ GenTree* Compiler::gtWrapWithSideEffects(GenTree* tree, // It should be possible to be smarter here and allow such cases by extracting the side effects // properly for this particular case. For now, caller is responsible for avoiding such cases. - tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), sideEffects, tree); + if (sideEffectsSource->OperIs(GT_COMMA) && sideEffectsSource->AsOp()->gtOp1 == sideEffects) + { + sideEffectsSource->AsOp()->gtOp2 = tree; + return sideEffectsSource; + } + else + { + tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), sideEffects, tree); + } } return tree; } From 4c6e35918661bda0e852db93bf88ea9f6f56281d Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 18 Jul 2024 02:00:54 +0900 Subject: [PATCH 32/65] Respect IsBoundsChecked --- src/coreclr/jit/lclmorph.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 406e49e500975..fe8e5427a7703 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1117,7 +1117,8 @@ class LocalAddressVisitor final : public GenTreeVisitor ssize_t offset = node->AsIndexAddr()->gtElemOffset + node->gtGetOp2()->AsIntCon()->IconValue() * node->AsIndexAddr()->gtElemSize; - if (offset < static_cast(m_compiler->lvaLclExactSize(TopValue(1).LclNum()))) + if (!node->AsIndexAddr()->IsBoundsChecked() || + offset < static_cast(m_compiler->lvaLclExactSize(TopValue(1).LclNum()))) { if (FitsIn(offset) && TopValue(2).AddOffset(TopValue(1), static_cast(offset))) From 4d8437914ccbfe468029bc4c7d437e948b061807 Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 18 Jul 2024 02:43:33 +0900 Subject: [PATCH 33/65] Check lowerbound too --- src/coreclr/jit/lclmorph.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index fe8e5427a7703..47e8b894573c5 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1118,7 +1118,8 @@ class LocalAddressVisitor final : public GenTreeVisitor node->gtGetOp2()->AsIntCon()->IconValue() * node->AsIndexAddr()->gtElemSize; if (!node->AsIndexAddr()->IsBoundsChecked() || - offset < static_cast(m_compiler->lvaLclExactSize(TopValue(1).LclNum()))) + (static_cast(node->AsIndexAddr()->gtElemOffset) <= offset && + offset < static_cast(m_compiler->lvaLclExactSize(TopValue(1).LclNum())))) { if (FitsIn(offset) && TopValue(2).AddOffset(TopValue(1), static_cast(offset))) From c0cad851f532f309218e166936b4162faed4d96c Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 18 Jul 2024 16:39:55 +0900 Subject: [PATCH 34/65] Fix assertion take 2 --- src/coreclr/jit/gentree.cpp | 16 +++++++++------- src/coreclr/jit/morph.cpp | 1 + 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 5f799e7d5010f..9c8f8ca5bc81e 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -17139,15 +17139,17 @@ GenTree* Compiler::gtWrapWithSideEffects(GenTree* tree, // It should be possible to be smarter here and allow such cases by extracting the side effects // properly for this particular case. For now, caller is responsible for avoiding such cases. - if (sideEffectsSource->OperIs(GT_COMMA) && sideEffectsSource->AsOp()->gtOp1 == sideEffects) - { - sideEffectsSource->AsOp()->gtOp2 = tree; - return sideEffectsSource; - } - else +#ifdef DEBUG + // The side effects may be a throw created by local morph, + // just remove the morphed flag to avoid unnecessary assertions. + if (fgIsThrow(sideEffects) && sideEffects->AsCall()->gtArgs.GetUserArgByIndex(0)->GetLateNode()->IsCnsIntOrI()) { - tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), sideEffects, tree); + sideEffects->AsCall()->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED; + sideEffects->AsCall()->gtArgs.GetUserArgByIndex(0)->GetLateNode()->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED; } +#endif + + tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), sideEffects, tree); } return tree; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1045d96258bd6..c47e42e84f626 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -8272,6 +8272,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA if (fgIsCommaThrow(tree)) { tree->AsOp()->gtOp1 = fgMorphTree(tree->AsOp()->gtOp1); + INDEBUG(tree->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); fgMorphTreeDone(tree); return tree; } From d28553af24a5a510a04bf2bdee9633df047b5ab4 Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 18 Jul 2024 18:17:23 +0900 Subject: [PATCH 35/65] Remove redundant jit-ee calls --- src/coreclr/jit/objectalloc.cpp | 7 +-- src/coreclr/jit/objectalloc.h | 80 ++++++++++++++++++++------------- 2 files changed, 52 insertions(+), 35 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index b84a9b7b809a5..31e8fa24574c4 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -468,8 +468,9 @@ bool ObjectAllocator::MorphAllocObjNodes() onHeapReason = "[non-constant size]"; canStack = false; } - else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, (unsigned int)len->AsIntCon()->IconValue(), - &blockSize, &onHeapReason)) + else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, allocType, + (unsigned int)len->AsIntCon()->IconValue(), &blockSize, + &onHeapReason)) { // reason set by the call canStack = false; @@ -511,7 +512,7 @@ bool ObjectAllocator::MorphAllocObjNodes() comp->Metrics.NewRefClassHelperCalls++; } - if (!CanAllocateLclVarOnStack(lclNum, clsHnd, 0, nullptr, &onHeapReason)) + if (!CanAllocateLclVarOnStack(lclNum, clsHnd, allocType, 0, nullptr, &onHeapReason)) { // reason set by the call canStack = false; diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 4b1f77895d630..9781205ae7583 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -55,6 +55,7 @@ class ObjectAllocator final : public Phase private: bool CanAllocateLclVarOnStack(unsigned int lclNum, CORINFO_CLASS_HANDLE clsHnd, + ObjectAllocationType allocType, unsigned int length, unsigned int* blockSize, const char** reason); @@ -126,15 +127,22 @@ inline void ObjectAllocator::EnableObjectStackAllocation() // allocated on the stack. // // Arguments: -// lclNum - Local variable number -// clsHnd - Class/struct handle of the variable class -// reason - [out, required] if result is false, reason why +// lclNum - Local variable number +// clsHnd - Class/struct handle of the variable class +// allocType - Type of allocation (newobj or newarr) +// length - Length of the array (for newarr) +// blockSize - [out, optional] exact size of the object +// reason - [out, required] if result is false, reason why // // Return Value: // Returns true iff local variable can be allocated on the stack. // -inline bool ObjectAllocator::CanAllocateLclVarOnStack( - unsigned int lclNum, CORINFO_CLASS_HANDLE clsHnd, unsigned int length, unsigned int* blockSize, const char** reason) +inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNum, + CORINFO_CLASS_HANDLE clsHnd, + ObjectAllocationType allocType, + unsigned int length, + unsigned int* blockSize, + const char** reason) { assert(m_AnalysisDone); @@ -151,23 +159,7 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack( unsigned int classSize = 0; - if (comp->info.compCompHnd->isValueClass(clsHnd)) - { - if (!enableBoxedValueClasses) - { - *reason = "[disabled by config]"; - return false; - } - - if (comp->info.compCompHnd->getTypeForBoxOnStack(clsHnd) == NO_CLASS_HANDLE) - { - *reason = "[no boxed type available]"; - return false; - } - - classSize = comp->info.compCompHnd->getClassSize(clsHnd); - } - else if (comp->info.compCompHnd->isSDArray(clsHnd)) + if (allocType == OAT_NEWARR) { if (!enableArrays) { @@ -188,21 +180,45 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack( unsigned elemSize = elemLayout != nullptr ? elemLayout->GetSize() : genTypeSize(type); classSize = (unsigned int)OFFSETOF__CORINFO_Array__data + elemSize * length; } - else + else if (allocType == OAT_NEWOBJ) { - if (!enableRefClasses) + if (comp->info.compCompHnd->isValueClass(clsHnd)) { - *reason = "[disabled by config]"; - return false; + if (!enableBoxedValueClasses) + { + *reason = "[disabled by config]"; + return false; + } + + if (comp->info.compCompHnd->getTypeForBoxOnStack(clsHnd) == NO_CLASS_HANDLE) + { + *reason = "[no boxed type available]"; + return false; + } + + classSize = comp->info.compCompHnd->getClassSize(clsHnd); } - - if (!comp->info.compCompHnd->canAllocateOnStack(clsHnd)) + else { - *reason = "[runtime disallows]"; - return false; + if (!enableRefClasses) + { + *reason = "[disabled by config]"; + return false; + } + + if (!comp->info.compCompHnd->canAllocateOnStack(clsHnd)) + { + *reason = "[runtime disallows]"; + return false; + } + + classSize = comp->info.compCompHnd->getHeapClassSize(clsHnd); } - - classSize = comp->info.compCompHnd->getHeapClassSize(clsHnd); + } + else + { + assert(!"Unexpected allocation type"); + return false; } if (classSize > s_StackAllocMaxSize) From c21c4f7b93fb54a6f21d48ef9a7ed12136e6bc79 Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 18 Jul 2024 20:52:36 +0900 Subject: [PATCH 36/65] Fix assertion again --- src/coreclr/jit/gentree.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 47d1cc5014aa3..d89b83519519e 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -17052,9 +17052,11 @@ GenTree* Compiler::gtWrapWithSideEffects(GenTree* tree, // properly for this particular case. For now, caller is responsible for avoiding such cases. #ifdef DEBUG - // The side effects may be a throw created by local morph, + // The side effects may be range check fail created by local morph, // just remove the morphed flag to avoid unnecessary assertions. - if (fgIsThrow(sideEffects) && sideEffects->AsCall()->gtArgs.GetUserArgByIndex(0)->GetLateNode()->IsCnsIntOrI()) + if (sideEffects->IsHelperCall(this, CORINFO_HELP_RNGCHKFAIL) && + sideEffects->AsCall()->gtArgs.CountUserArgs() == 1 && + sideEffects->AsCall()->gtArgs.GetUserArgByIndex(0)->GetLateNode()->IsCnsIntOrI()) { sideEffects->AsCall()->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED; sideEffects->AsCall()->gtArgs.GetUserArgByIndex(0)->GetLateNode()->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED; From 18ec558512303495b26cef10961b1ec7ed5d826e Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 18 Jul 2024 21:21:16 +0900 Subject: [PATCH 37/65] Check array length --- src/coreclr/jit/objectalloc.cpp | 5 ++--- src/coreclr/jit/objectalloc.h | 12 +++++++++--- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 31e8fa24574c4..2b1a604b7d8eb 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -468,9 +468,8 @@ bool ObjectAllocator::MorphAllocObjNodes() onHeapReason = "[non-constant size]"; canStack = false; } - else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, allocType, - (unsigned int)len->AsIntCon()->IconValue(), &blockSize, - &onHeapReason)) + else if (!CanAllocateLclVarOnStack(lclNum, clsHnd, allocType, len->AsIntCon()->IconValue(), + &blockSize, &onHeapReason)) { // reason set by the call canStack = false; diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 9781205ae7583..86e1b697be462 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -56,7 +56,7 @@ class ObjectAllocator final : public Phase bool CanAllocateLclVarOnStack(unsigned int lclNum, CORINFO_CLASS_HANDLE clsHnd, ObjectAllocationType allocType, - unsigned int length, + ssize_t length, unsigned int* blockSize, const char** reason); bool CanLclVarEscape(unsigned int lclNum); @@ -140,7 +140,7 @@ inline void ObjectAllocator::EnableObjectStackAllocation() inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNum, CORINFO_CLASS_HANDLE clsHnd, ObjectAllocationType allocType, - unsigned int length, + ssize_t length, unsigned int* blockSize, const char** reason) { @@ -167,6 +167,12 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNu return false; } + if (length < 0 || !FitsIn(length)) + { + *reason = "[invalid array length]"; + return false; + } + CORINFO_CLASS_HANDLE elemClsHnd = NO_CLASS_HANDLE; CorInfoType corType = comp->info.compCompHnd->getChildType(clsHnd, &elemClsHnd); var_types type = JITtype2varType(corType); @@ -178,7 +184,7 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNu } unsigned elemSize = elemLayout != nullptr ? elemLayout->GetSize() : genTypeSize(type); - classSize = (unsigned int)OFFSETOF__CORINFO_Array__data + elemSize * length; + classSize = (unsigned int)OFFSETOF__CORINFO_Array__data + elemSize * static_cast(length); } else if (allocType == OAT_NEWOBJ) { From eadb4ad5c7271bd6eb4f247fa4bb83da828efc27 Mon Sep 17 00:00:00 2001 From: Steven He Date: Thu, 18 Jul 2024 23:35:38 +0900 Subject: [PATCH 38/65] Fix assertion in another way --- src/coreclr/jit/gentree.cpp | 28 +++++++++++++--------------- src/coreclr/jit/morph.cpp | 1 - 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index d89b83519519e..5d9655af408c2 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -14385,10 +14385,15 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree) if (fgGlobalMorph) { +#ifdef DEBUG // We can sometimes produce a comma over the constant if the original op // had a side effect, so just ensure we set the flag (which will be already // set for the operands otherwise). - INDEBUG(op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + if (op->OperIs(GT_COMMA) && (op->AsOp()->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) != 0) + { + op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; + } +#endif } return op; } @@ -14612,10 +14617,15 @@ GenTree* Compiler::gtFoldExprSpecialFloating(GenTree* tree) if (fgGlobalMorph) { +#ifdef DEBUG // We can sometimes produce a comma over the constant if the original op // had a side effect, so just ensure we set the flag (which will be already // set for the operands otherwise). - INDEBUG(op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); + if (op->OperIs(GT_COMMA) && (op->AsOp()->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) != 0) + { + op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; + } +#endif } return op; } @@ -17051,19 +17061,7 @@ GenTree* Compiler::gtWrapWithSideEffects(GenTree* tree, // It should be possible to be smarter here and allow such cases by extracting the side effects // properly for this particular case. For now, caller is responsible for avoiding such cases. -#ifdef DEBUG - // The side effects may be range check fail created by local morph, - // just remove the morphed flag to avoid unnecessary assertions. - if (sideEffects->IsHelperCall(this, CORINFO_HELP_RNGCHKFAIL) && - sideEffects->AsCall()->gtArgs.CountUserArgs() == 1 && - sideEffects->AsCall()->gtArgs.GetUserArgByIndex(0)->GetLateNode()->IsCnsIntOrI()) - { - sideEffects->AsCall()->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED; - sideEffects->AsCall()->gtArgs.GetUserArgByIndex(0)->GetLateNode()->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED; - } -#endif - - tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), sideEffects, tree); + tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), gtCloneExpr(sideEffects), tree); } return tree; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index c47e42e84f626..1045d96258bd6 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -8272,7 +8272,6 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA if (fgIsCommaThrow(tree)) { tree->AsOp()->gtOp1 = fgMorphTree(tree->AsOp()->gtOp1); - INDEBUG(tree->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); fgMorphTreeDone(tree); return tree; } From 9d4021cdc0661960ad42be40bdea563e4c3117d3 Mon Sep 17 00:00:00 2001 From: Steven He Date: Fri, 19 Jul 2024 00:29:07 +0900 Subject: [PATCH 39/65] Unset the flag to avoid unnecessary assert --- src/coreclr/jit/gentree.cpp | 16 +++------------- src/coreclr/jit/morph.cpp | 9 +++++++++ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 5d9655af408c2..759dde9b584fb 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -14385,15 +14385,10 @@ GenTree* Compiler::gtFoldExprSpecial(GenTree* tree) if (fgGlobalMorph) { -#ifdef DEBUG // We can sometimes produce a comma over the constant if the original op // had a side effect, so just ensure we set the flag (which will be already // set for the operands otherwise). - if (op->OperIs(GT_COMMA) && (op->AsOp()->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) != 0) - { - op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; - } -#endif + INDEBUG(op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); } return op; } @@ -14617,15 +14612,10 @@ GenTree* Compiler::gtFoldExprSpecialFloating(GenTree* tree) if (fgGlobalMorph) { -#ifdef DEBUG // We can sometimes produce a comma over the constant if the original op // had a side effect, so just ensure we set the flag (which will be already // set for the operands otherwise). - if (op->OperIs(GT_COMMA) && (op->AsOp()->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) != 0) - { - op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED; - } -#endif + INDEBUG(op->gtDebugFlags |= GTF_DEBUG_NODE_MORPHED); } return op; } @@ -17061,7 +17051,7 @@ GenTree* Compiler::gtWrapWithSideEffects(GenTree* tree, // It should be possible to be smarter here and allow such cases by extracting the side effects // properly for this particular case. For now, caller is responsible for avoiding such cases. - tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), gtCloneExpr(sideEffects), tree); + tree = gtNewOperNode(GT_COMMA, tree->TypeGet(), sideEffects, tree); } return tree; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 1045d96258bd6..8baaa3e761432 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2377,6 +2377,7 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) { for (CallArg& arg : call->gtArgs.LateArgs()) { + INDEBUG(arg.GetLateNode()->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); arg.SetLateNode(fgMorphTree(arg.GetLateNode())); flagsSummary |= arg.GetLateNode()->gtFlags; } @@ -8272,6 +8273,7 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA if (fgIsCommaThrow(tree)) { tree->AsOp()->gtOp1 = fgMorphTree(tree->AsOp()->gtOp1); + INDEBUG(tree->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); fgMorphTreeDone(tree); return tree; } @@ -12147,8 +12149,15 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) if (fgGlobalMorph) { +#ifdef DEBUG + /* A RNGCHKFAIL may created by local morph for stack allocated arrays and causes remorph */ + if (tree->IsHelperCall(this, CORINFO_HELP_OVERFLOW)) + { + tree->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED; + } /* Ensure that we haven't morphed this node already */ assert(((tree->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) == 0) && "ERROR: Already morphed this node!"); +#endif /* Before morphing the tree, we try to propagate any active assertions */ if (optLocalAssertionProp) From 1fff53e3d270c5920b88032492621a350971418b Mon Sep 17 00:00:00 2001 From: Steven He Date: Fri, 19 Jul 2024 17:03:41 +0900 Subject: [PATCH 40/65] Add tests --- .../ObjectStackAllocationTests.cs | 121 +++++++++++++++--- 1 file changed, 103 insertions(+), 18 deletions(-) diff --git a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs index 33303daf7a07b..07114d7ebd9b7 100644 --- a/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs +++ b/src/tests/JIT/opt/ObjectStackAllocation/ObjectStackAllocationTests.cs @@ -156,6 +156,8 @@ public static int TestEntryPoint() // Stack allocation of boxed structs is now enabled CallTestAndVerifyAllocation(BoxSimpleStructAndAddFields, 12, expectedAllocationKind); + CallTestAndVerifyAllocation(AllocateArrayWithNonGCElements, 84, expectedAllocationKind); + // The remaining tests currently never allocate on the stack if (expectedAllocationKind == AllocationKind.Stack) { expectedAllocationKind = AllocationKind.Heap; @@ -167,6 +169,20 @@ public static int TestEntryPoint() // This test calls CORINFO_HELP_CHKCASTCLASS_SPECIAL CallTestAndVerifyAllocation(AllocateSimpleClassAndCast, 7, expectedAllocationKind); + CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsEscape, 42, expectedAllocationKind); + + // This test calls CORINFO_HELP_OVERFLOW + CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsOutOfRangeLeft, 0, expectedAllocationKind, true); + + // This test calls CORINFO_HELP_OVERFLOW + CallTestAndVerifyAllocation(AllocateArrayWithNonGCElementsOutOfRangeRight, 0, expectedAllocationKind, true); + + // This test calls CORINFO_HELP_ARTHEMIC_OVERFLOW + CallTestAndVerifyAllocation(AllocateNegativeLengthArrayWithNonGCElements, 0, expectedAllocationKind, true); + + // This test calls CORINFO_HELP_ARTHEMIC_OVERFLOW + CallTestAndVerifyAllocation(AllocateLongLengthArrayWithNonGCElements, 0, expectedAllocationKind, true); + return methodResult; } @@ -175,27 +191,38 @@ static bool GCStressEnabled() return Environment.GetEnvironmentVariable("DOTNET_GCStress") != null; } - static void CallTestAndVerifyAllocation(Test test, int expectedResult, AllocationKind expectedAllocationsKind) + static void CallTestAndVerifyAllocation(Test test, int expectedResult, AllocationKind expectedAllocationsKind, bool throws = false) { - long allocatedBytesBefore = GC.GetAllocatedBytesForCurrentThread(); - int testResult = test(); - long allocatedBytesAfter = GC.GetAllocatedBytesForCurrentThread(); string methodName = test.Method.Name; - - if (testResult != expectedResult) { - Console.WriteLine($"FAILURE ({methodName}): expected {expectedResult}, got {testResult}"); - methodResult = -1; - } - else if ((expectedAllocationsKind == AllocationKind.Stack) && (allocatedBytesBefore != allocatedBytesAfter)) { - Console.WriteLine($"FAILURE ({methodName}): unexpected allocation of {allocatedBytesAfter - allocatedBytesBefore} bytes"); - methodResult = -1; - } - else if ((expectedAllocationsKind == AllocationKind.Heap) && (allocatedBytesBefore == allocatedBytesAfter)) { - Console.WriteLine($"FAILURE ({methodName}): unexpected stack allocation"); - methodResult = -1; + try + { + long allocatedBytesBefore = GC.GetAllocatedBytesForCurrentThread(); + int testResult = test(); + long allocatedBytesAfter = GC.GetAllocatedBytesForCurrentThread(); + + if (testResult != expectedResult) { + Console.WriteLine($"FAILURE ({methodName}): expected {expectedResult}, got {testResult}"); + methodResult = -1; + } + else if ((expectedAllocationsKind == AllocationKind.Stack) && (allocatedBytesBefore != allocatedBytesAfter)) { + Console.WriteLine($"FAILURE ({methodName}): unexpected allocation of {allocatedBytesAfter - allocatedBytesBefore} bytes"); + methodResult = -1; + } + else if ((expectedAllocationsKind == AllocationKind.Heap) && (allocatedBytesBefore == allocatedBytesAfter)) { + Console.WriteLine($"FAILURE ({methodName}): unexpected stack allocation"); + methodResult = -1; + } + else { + Console.WriteLine($"SUCCESS ({methodName})"); + } } - else { - Console.WriteLine($"SUCCESS ({methodName})"); + catch { + if (throws) { + Console.WriteLine($"SUCCESS ({methodName})"); + } + else { + throw; + } } } @@ -339,6 +366,64 @@ static int AllocateClassWithGcFieldAndInt() return c.i; } + [MethodImpl(MethodImplOptions.NoInlining)] + static int AllocateArrayWithNonGCElements() + { + int[] array = new int[42]; + array[24] = 42; + GC.Collect(); + return array[24] + array.Length; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int AllocateArrayWithNonGCElementsEscape() + { + int[] array = new int[42]; + Use(ref array[24]); + GC.Collect(); + return array[24]; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int AllocateArrayWithNonGCElementsOutOfRangeRight() + { + int[] array = new int[42]; + array[43] = 42; + GC.Collect(); + return 1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int AllocateArrayWithNonGCElementsOutOfRangeLeft() + { + int[] array = new int[42]; + array[-1] = 42; + GC.Collect(); + return 1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int AllocateNegativeLengthArrayWithNonGCElements() + { + int[] array = new int["".Length - 2]; + GC.Collect(); + return 1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static int AllocateLongLengthArrayWithNonGCElements() + { + int[] array = new int[long.MaxValue]; + GC.Collect(); + return 1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static void Use(ref int v) + { + v = 42; + } + [MethodImpl(MethodImplOptions.NoInlining)] private static void ZeroAllocTest() { From d521a944015c707e18a9e2fbd56678e0b2689287 Mon Sep 17 00:00:00 2001 From: Steven He Date: Fri, 19 Jul 2024 19:08:09 +0900 Subject: [PATCH 41/65] sigh --- src/coreclr/jit/morph.cpp | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 8baaa3e761432..b8983eb3a8713 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -2377,7 +2377,6 @@ GenTreeCall* Compiler::fgMorphArgs(GenTreeCall* call) { for (CallArg& arg : call->gtArgs.LateArgs()) { - INDEBUG(arg.GetLateNode()->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); arg.SetLateNode(fgMorphTree(arg.GetLateNode())); flagsSummary |= arg.GetLateNode()->gtFlags; } @@ -8269,11 +8268,20 @@ GenTree* Compiler::fgMorphSmpOp(GenTree* tree, MorphAddrContext* mac, bool* optA return tree; } +#ifdef DEBUG + if ((tree->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) != 0) + { + return tree; + } +#endif + /* If we created a comma-throw tree then we need to morph op1 */ if (fgIsCommaThrow(tree)) { - tree->AsOp()->gtOp1 = fgMorphTree(tree->AsOp()->gtOp1); - INDEBUG(tree->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED); + INDEBUG(if ((tree->AsOp()->gtOp1->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) == 0)) + { + tree->AsOp()->gtOp1 = fgMorphTree(tree->AsOp()->gtOp1); + } fgMorphTreeDone(tree); return tree; } @@ -12149,15 +12157,8 @@ GenTree* Compiler::fgMorphTree(GenTree* tree, MorphAddrContext* mac) if (fgGlobalMorph) { -#ifdef DEBUG - /* A RNGCHKFAIL may created by local morph for stack allocated arrays and causes remorph */ - if (tree->IsHelperCall(this, CORINFO_HELP_OVERFLOW)) - { - tree->gtDebugFlags &= ~GTF_DEBUG_NODE_MORPHED; - } /* Ensure that we haven't morphed this node already */ assert(((tree->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) == 0) && "ERROR: Already morphed this node!"); -#endif /* Before morphing the tree, we try to propagate any active assertions */ if (optLocalAssertionProp) From 97ee2bec3d262a8618c92cdfbfb69ef750da2625 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 20 Jul 2024 01:38:53 +0900 Subject: [PATCH 42/65] Support R2R/NativeAOT --- src/coreclr/jit/objectalloc.cpp | 59 ++++++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 9 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 2b1a604b7d8eb..12ce40685bd76 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -408,10 +408,24 @@ bool ObjectAllocator::MorphAllocObjNodes() { allocType = OAT_NEWOBJ; } - else if (data->IsHelperCall(comp, CORINFO_HELP_NEWARR_1_VC) && - data->AsCall()->gtArgs.GetUserArgByIndex(0)->GetNode()->IsCnsIntOrI()) + else if (data->IsHelperCall()) { - allocType = OAT_NEWARR; + switch (data->AsCall()->GetHelperNum()) + { + case CORINFO_HELP_NEWARR_1_VC: + case CORINFO_HELP_NEWARR_1_OBJ: + case CORINFO_HELP_NEWARR_1_DIRECT: + case CORINFO_HELP_NEWARR_1_ALIGN8: +#ifdef FEATURE_READYTORUN + case CORINFO_HELP_READYTORUN_NEWARR_1: +#endif + { + if (data->AsCall()->gtArgs.GetArgByIndex(0)->GetNode()->IsCnsIntOrI()) + { + allocType = OAT_NEWARR; + } + } + } } } @@ -437,25 +451,42 @@ bool ObjectAllocator::MorphAllocObjNodes() { if (allocType == OAT_NEWARR) { - assert(data->AsCall()->GetHelperNum() == CORINFO_HELP_NEWARR_1_VC); + assert(basicBlockHasNewArr); //------------------------------------------------------------------------ // We expect the following expression tree at this point + // For non-ReadyToRun: // STMTx (IL 0x... ???) // * STORE_LCL_VAR ref - // \--* CALL help ref CORINFO_HELP_NEWARR_1_VC + // \--* CALL help ref // +--* CNS_INT(h) long // \--* CNS_INT long + // For ReadyToRun: + // STMTx (IL 0x... ???) + // * STORE_LCL_VAR ref + // \--* CALL help ref + // \--* CNS_INT long //------------------------------------------------------------------------ CallArg* arg = data->AsCall()->gtArgs.GetArgByIndex(0); - GenTree* node = arg->GetNode(); bool isExact = false; bool isNonNull = false; CORINFO_CLASS_HANDLE clsHnd = comp->gtGetHelperCallClassHandle(data->AsCall(), &isExact, &isNonNull); - GenTree* len = arg->GetNext()->GetNode(); - unsigned int blockSize = 0; + GenTree* len = nullptr; +#ifdef FEATURE_READYTORUN + if (comp->opts.IsReadyToRun() && data->IsHelperCall(comp, CORINFO_HELP_READYTORUN_NEWARR_1)) + { + len = arg->GetNode(); + } + else +#endif + { + len = arg->GetNext()->GetNode(); + } + + assert(len != nullptr); + unsigned int blockSize = 0; comp->Metrics.NewArrayHelperCalls++; if (!isExact || !isNonNull) @@ -659,17 +690,27 @@ unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* assert(newArr != nullptr); assert(m_AnalysisDone); assert(clsHnd != NO_CLASS_HANDLE); + assert(newArr->IsHelperCall()); + assert(newArr->GetHelperNum() != CORINFO_HELP_NEWARR_1_MAYBEFROZEN); +#ifdef FEATURE_READYTORUN + assert(newArr->GetHelperNum() == CORINFO_HELP_READYTORUN_NEWARR_1 || !comp->opts.IsReadyToRun()); +#endif const bool shortLifetime = false; + const bool alignTo8 = newArr->GetHelperNum() == CORINFO_HELP_NEWARR_1_ALIGN8; const unsigned int lclNum = comp->lvaGrabTemp(shortLifetime DEBUGARG("stack allocated array temp")); + if (alignTo8) + { + blockSize = AlignUp(blockSize, TARGET_POINTER_SIZE); + } + comp->lvaSetStruct(lclNum, comp->typGetBlkLayout(blockSize), /* unsafeValueClsCheck */ false); // Initialize the object memory if necessary. bool bbInALoop = block->HasFlag(BBF_BACKWARD_JUMP); bool bbIsReturn = block->KindIs(BBJ_RETURN); LclVarDsc* const lclDsc = comp->lvaGetDesc(lclNum); - lclDsc->lvStackAllocatedBox = false; if (comp->fgVarNeedsExplicitZeroInit(lclNum, bbInALoop, bbIsReturn)) { //------------------------------------------------------------------------ From 5bcb78606802d1714755715ffb392e1c52c54d56 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 20 Jul 2024 02:21:31 +0900 Subject: [PATCH 43/65] Fix building --- src/coreclr/jit/objectalloc.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 12ce40685bd76..90b19ed07e19d 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -424,6 +424,11 @@ bool ObjectAllocator::MorphAllocObjNodes() { allocType = OAT_NEWARR; } + break; + } + default: + { + break; } } } From a01562e0737a0d4cdd6ccbd882806ca3c76cb4fb Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 20 Jul 2024 02:24:27 +0900 Subject: [PATCH 44/65] cleanup --- src/coreclr/jit/objectalloc.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 90b19ed07e19d..5518455741d2f 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -416,16 +416,25 @@ bool ObjectAllocator::MorphAllocObjNodes() case CORINFO_HELP_NEWARR_1_OBJ: case CORINFO_HELP_NEWARR_1_DIRECT: case CORINFO_HELP_NEWARR_1_ALIGN8: + { + if (data->AsCall()->gtArgs.CountArgs() == 2 && + data->AsCall()->gtArgs.GetArgByIndex(1)->GetNode()->IsCnsIntOrI()) + { + allocType = OAT_NEWARR; + } + break; + } #ifdef FEATURE_READYTORUN case CORINFO_HELP_READYTORUN_NEWARR_1: -#endif { - if (data->AsCall()->gtArgs.GetArgByIndex(0)->GetNode()->IsCnsIntOrI()) + if (data->AsCall()->gtArgs.CountArgs() == 1 && + data->AsCall()->gtArgs.GetArgByIndex(0)->GetNode()->IsCnsIntOrI()) { allocType = OAT_NEWARR; } break; } +#endif default: { break; @@ -472,7 +481,6 @@ bool ObjectAllocator::MorphAllocObjNodes() // \--* CNS_INT long //------------------------------------------------------------------------ - CallArg* arg = data->AsCall()->gtArgs.GetArgByIndex(0); bool isExact = false; bool isNonNull = false; CORINFO_CLASS_HANDLE clsHnd = @@ -481,12 +489,12 @@ bool ObjectAllocator::MorphAllocObjNodes() #ifdef FEATURE_READYTORUN if (comp->opts.IsReadyToRun() && data->IsHelperCall(comp, CORINFO_HELP_READYTORUN_NEWARR_1)) { - len = arg->GetNode(); + len = data->AsCall()->gtArgs.GetArgByIndex(0)->GetNode(); } else #endif { - len = arg->GetNext()->GetNode(); + len = data->AsCall()->gtArgs.GetArgByIndex(1)->GetNode(); } assert(len != nullptr); From e728d4ff945fad1f9bd16becd9d41b02bd3de4f5 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 20 Jul 2024 03:29:41 +0900 Subject: [PATCH 45/65] remove invalid assert --- src/coreclr/jit/objectalloc.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 5518455741d2f..0636f912f4197 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -705,9 +705,6 @@ unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* assert(clsHnd != NO_CLASS_HANDLE); assert(newArr->IsHelperCall()); assert(newArr->GetHelperNum() != CORINFO_HELP_NEWARR_1_MAYBEFROZEN); -#ifdef FEATURE_READYTORUN - assert(newArr->GetHelperNum() == CORINFO_HELP_READYTORUN_NEWARR_1 || !comp->opts.IsReadyToRun()); -#endif const bool shortLifetime = false; const bool alignTo8 = newArr->GetHelperNum() == CORINFO_HELP_NEWARR_1_ALIGN8; From d73c5c5723a5ff2b2415c8472576b50f62b4497d Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 20 Jul 2024 03:37:41 +0900 Subject: [PATCH 46/65] double align on 32bit platform --- src/coreclr/jit/objectalloc.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 0636f912f4197..7c49d00ce3cae 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -741,6 +741,10 @@ unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* comp->compSuppressedZeroInit = true; } +#ifndef TARGET_64BIT + lclDsc->lvStructDoubleAlign = alignTo8; +#endif + // Initialize the vtable slot. // //------------------------------------------------------------------------ From c9fea239be739d24a247020e609fcd97cb49c84f Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 20 Jul 2024 03:39:40 +0900 Subject: [PATCH 47/65] Use correct alignment for align8 --- src/coreclr/jit/objectalloc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 7c49d00ce3cae..421ce964bbd81 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -712,7 +712,7 @@ unsigned int ObjectAllocator::MorphNewArrNodeIntoStackAlloc(GenTreeCall* if (alignTo8) { - blockSize = AlignUp(blockSize, TARGET_POINTER_SIZE); + blockSize = AlignUp(blockSize, 8); } comp->lvaSetStruct(lclNum, comp->typGetBlkLayout(blockSize), /* unsafeValueClsCheck */ false); From 772bee6104d56407356ddc81fe7860283c1ae3fa Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 20 Jul 2024 19:25:59 +0900 Subject: [PATCH 48/65] Fix intrinsic expansion --- src/coreclr/jit/importercalls.cpp | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 0d2045d553c82..0576c25fa5eef 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -2579,12 +2579,28 @@ GenTree* Compiler::impInitializeArrayIntrinsic(CORINFO_SIG_INFO* sig) } // - // We start by looking at the last statement, making sure it's a store, and - // that the target of the store is the array passed to InitializeArray. + // We start by looking at the last statement, making sure it's a store. // GenTree* arrayLocalStore = impLastStmt->GetRootNode(); - if (!arrayLocalStore->OperIs(GT_STORE_LCL_VAR) || !arrayLocalNode->OperIs(GT_LCL_VAR) || - (arrayLocalStore->AsLclVar()->GetLclNum() != arrayLocalNode->AsLclVar()->GetLclNum())) + if (arrayLocalStore->OperIs(GT_STORE_LCL_VAR) && arrayLocalNode->OperIs(GT_LCL_VAR)) + { + // Make sure the target of the store is the array passed to InitializeArray. + if (arrayLocalStore->AsLclVar()->GetLclNum() != arrayLocalNode->AsLclVar()->GetLclNum()) + { + // The array can be spilled to a temp for stack allocation. + // Try getting the actual store node from the previous statement. + if (arrayLocalStore->AsLclVar()->Data()->OperIs(GT_LCL_VAR) && impLastStmt->GetPrevStmt() != nullptr) + { + arrayLocalStore = impLastStmt->GetPrevStmt()->GetRootNode(); + if (!arrayLocalStore->OperIs(GT_STORE_LCL_VAR) || + arrayLocalStore->AsLclVar()->GetLclNum() != arrayLocalNode->AsLclVar()->GetLclNum()) + { + return nullptr; + } + } + } + } + else { return nullptr; } From 3dbe8ac2f5b1f5f558a98cc9489590bb54ff6d6e Mon Sep 17 00:00:00 2001 From: Steven He Date: Fri, 19 Jul 2024 03:08:23 +0900 Subject: [PATCH 49/65] Initial prototype --- src/coreclr/jit/objectalloc.cpp | 85 ++++++++++++++++++++++++++++----- 1 file changed, 73 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 421ce964bbd81..485392d930fab 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -203,7 +203,7 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() { lclEscapes = false; } - else if (tree->OperIs(GT_LCL_VAR) && tree->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL)) + else if (tree->OperIs(GT_LCL_VAR, GT_LCL_ADDR) && tree->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL, TYP_STRUCT)) { assert(tree == m_ancestors.Top()); @@ -230,7 +230,7 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() { var_types type = comp->lvaTable[lclNum].TypeGet(); - if (type == TYP_REF || genActualType(type) == TYP_I_IMPL || type == TYP_BYREF) + if (type == TYP_REF || genActualType(type) == TYP_I_IMPL || type == TYP_BYREF || type == TYP_STRUCT) { m_ConnGraphAdjacencyMatrix[lclNum] = BitVecOps::MakeEmpty(&m_bitVecTraits); @@ -926,6 +926,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_BOX: case GT_FIELD_ADDR: case GT_INDEX_ADDR: + case GT_LCL_ADDR: // Check whether the local escapes via its grandparent. ++parentIndex; keepChecking = true; @@ -936,7 +937,58 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_BLK: if (tree != parent->AsIndir()->Addr()) { - // TODO-ObjectStackAllocation: track stores to fields. + switch (parent->AsIndir()->Addr()->OperGet()) + { + case GT_FIELD_ADDR: + { + GenTree* fieldObj = parent->AsIndir()->Addr()->AsFieldAddr()->GetFldObj(); + if (fieldObj != nullptr && fieldObj->IsAnyLocal()) + { + // Add an edge to the connection graph. + const unsigned int dstLclNum = fieldObj->AsLclVarCommon()->GetLclNum(); + const unsigned int srcLclNum = lclNum; + if (dstLclNum == 0 && !comp->info.compIsStatic) + { + break; + } + + if (fieldObj->IsLocal()) + { + if (comp->lvaGetDesc(dstLclNum)->TypeGet() != TYP_REF) + { + break; + } + } + else if (comp->lvaGetDesc(dstLclNum)->TypeGet() != TYP_STRUCT) + { + break; + } + + AddConnGraphEdge(dstLclNum, srcLclNum); + ++parentIndex; + keepChecking = true; + break; + } + break; + } + default: + { + if (parent->AsIndir()->Addr()->IsAnyLocal()) + { + // Add an edge to the connection graph. + const unsigned int dstLclNum = parent->AsIndir()->Addr()->AsLclVarCommon()->GetLclNum(); + const unsigned int srcLclNum = lclNum; + + if (comp->lvaGetDesc(dstLclNum)->TypeGet() != TYP_REF) + { + break; + } + + AddConnGraphEdge(dstLclNum, srcLclNum); + canLclVarEscapeViaParentStack = false; + } + } + } break; } FALLTHROUGH; @@ -1025,6 +1077,7 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p case GT_SUB: case GT_FIELD_ADDR: case GT_INDEX_ADDR: + case GT_LCL_ADDR: if (parent->TypeGet() == TYP_REF) { parent->ChangeType(newType); @@ -1062,17 +1115,25 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack* p case GT_STOREIND: case GT_STORE_BLK: case GT_BLK: - assert(tree == parent->AsIndir()->Addr()); - - // The new target could be *not* on the heap. - parent->gtFlags &= ~GTF_IND_TGT_HEAP; + if (tree == parent->AsIndir()->Addr()) + { + // The new target could be *not* on the heap. + parent->gtFlags &= ~GTF_IND_TGT_HEAP; - if (newType != TYP_BYREF) + if (newType != TYP_BYREF) + { + // This indicates that a write barrier is not needed when writing + // to this field/indirection since the address is not pointing to the heap. + // It's either null or points to inside a stack-allocated object. + parent->gtFlags |= GTF_IND_TGT_NOT_HEAP; + } + } + else { - // This indicates that a write barrier is not needed when writing - // to this field/indirection since the address is not pointing to the heap. - // It's either null or points to inside a stack-allocated object. - parent->gtFlags |= GTF_IND_TGT_NOT_HEAP; + parent->ChangeType(newType); + + ++parentIndex; + keepChecking = true; } break; From 90220786d27dc72bf50a2675239b885315db2f38 Mon Sep 17 00:00:00 2001 From: Steven He Date: Fri, 19 Jul 2024 19:26:01 +0900 Subject: [PATCH 50/65] handle delegates --- src/coreclr/jit/objectalloc.cpp | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 485392d930fab..87672d0e98f1f 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -999,12 +999,23 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_CALL: { - GenTreeCall* asCall = parent->AsCall(); + GenTreeCall* const call = parent->AsCall(); - if (asCall->IsHelperCall()) + if (call->IsHelperCall()) { canLclVarEscapeViaParentStack = - !Compiler::s_helperCallProperties.IsNoEscape(comp->eeGetHelperNum(asCall->gtCallMethHnd)); + !Compiler::s_helperCallProperties.IsNoEscape(comp->eeGetHelperNum(call->gtCallMethHnd)); + } + else if (call->gtCallType == CT_USER_FUNC) + { + // Delegate invoke won't escape the delegate which is passed as "this" + // And gets expanded inline later. + // + if ((call->gtCallMoreFlags & GTF_CALL_M_DELEGATE_INV) != 0) + { + GenTree* const thisArg = call->gtArgs.GetThisArg()->GetNode(); + canLclVarEscapeViaParentStack = thisArg != tree; + } } break; } From ada0890da8bb0b5c43e4a9157cfc95b53d50cf4a Mon Sep 17 00:00:00 2001 From: Steven He Date: Fri, 19 Jul 2024 19:57:16 +0900 Subject: [PATCH 51/65] handle escape through IND --- src/coreclr/jit/objectalloc.cpp | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 87672d0e98f1f..dfaf6bf03f8e8 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -993,8 +993,16 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent } FALLTHROUGH; case GT_IND: - // Address of the field/ind is not taken so the local doesn't escape. - canLclVarEscapeViaParentStack = false; + if (parent->OperIs(GT_IND) && parent->TypeIs(TYP_REF)) + { + ++parentIndex; + keepChecking = true; + } + else + { + // Address of the field/ind is not taken so the local doesn't escape. + canLclVarEscapeViaParentStack = false; + } break; case GT_CALL: From 9f40866f23445bf95c68684a8b9f9389e001c386 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 20 Jul 2024 19:30:31 +0900 Subject: [PATCH 52/65] Disable stackalloc for arrays and delegates --- src/coreclr/jit/objectalloc.cpp | 22 +++++++++++----------- src/coreclr/jit/objectalloc.h | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index dfaf6bf03f8e8..a2907bacc3caf 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -1014,17 +1014,17 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent canLclVarEscapeViaParentStack = !Compiler::s_helperCallProperties.IsNoEscape(comp->eeGetHelperNum(call->gtCallMethHnd)); } - else if (call->gtCallType == CT_USER_FUNC) - { - // Delegate invoke won't escape the delegate which is passed as "this" - // And gets expanded inline later. - // - if ((call->gtCallMoreFlags & GTF_CALL_M_DELEGATE_INV) != 0) - { - GenTree* const thisArg = call->gtArgs.GetThisArg()->GetNode(); - canLclVarEscapeViaParentStack = thisArg != tree; - } - } + //else if (call->gtCallType == CT_USER_FUNC) + //{ + // // Delegate invoke won't escape the delegate which is passed as "this" + // // And gets expanded inline later. + // // + // if ((call->gtCallMoreFlags & GTF_CALL_M_DELEGATE_INV) != 0) + // { + // GenTree* const thisArg = call->gtArgs.GetThisArg()->GetNode(); + // canLclVarEscapeViaParentStack = thisArg != tree; + // } + //} break; } diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 86e1b697be462..b8f4f48b8179b 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -148,7 +148,7 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNu bool enableBoxedValueClasses = true; bool enableRefClasses = true; - bool enableArrays = true; + bool enableArrays = false; *reason = "[ok]"; #ifdef DEBUG From bf58c6d81bacfda7c71c7719d2b312837dc3017c Mon Sep 17 00:00:00 2001 From: Steven He Date: Sat, 20 Jul 2024 20:57:45 +0900 Subject: [PATCH 53/65] Track conditional field assignment --- src/coreclr/jit/objectalloc.cpp | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index a2907bacc3caf..2507998d83029 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -896,7 +896,20 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent const unsigned int srcLclNum = lclNum; AddConnGraphEdge(dstLclNum, srcLclNum); - canLclVarEscapeViaParentStack = false; + + if (!parent->AsLclVar()->TypeIs(TYP_REF)) + { + canLclVarEscapeViaParentStack = false; + } + else + { + // A local may be assigned to a field of another local. + // Add an inverse edge to the connection graph too. + // For example, this.Foo = foo ?? new Foo(); + AddConnGraphEdge(srcLclNum, dstLclNum); + ++parentIndex; + keepChecking = true; + } } break; @@ -1011,6 +1024,10 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent if (call->IsHelperCall()) { + if (call->IsHelperCall(comp, CORINFO_HELP_VIRTUAL_FUNC_PTR)) + { + break; + } canLclVarEscapeViaParentStack = !Compiler::s_helperCallProperties.IsNoEscape(comp->eeGetHelperNum(call->gtCallMethHnd)); } From 1faa8d9b1e1f9773e87c981125bfd08093e724b2 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 00:30:26 +0900 Subject: [PATCH 54/65] Refine field-wise analysis --- src/coreclr/jit/objectalloc.cpp | 68 ++++++++++++++++----------------- src/coreclr/jit/objectalloc.h | 4 +- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 2507998d83029..e771409989cc4 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -228,22 +228,12 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() for (unsigned int lclNum = 0; lclNum < comp->lvaCount; ++lclNum) { - var_types type = comp->lvaTable[lclNum].TypeGet(); + m_ConnGraphAdjacencyMatrix[lclNum] = BitVecOps::MakeEmpty(&m_bitVecTraits); - if (type == TYP_REF || genActualType(type) == TYP_I_IMPL || type == TYP_BYREF || type == TYP_STRUCT) + if (comp->lvaTable[lclNum].IsAddressExposed()) { - m_ConnGraphAdjacencyMatrix[lclNum] = BitVecOps::MakeEmpty(&m_bitVecTraits); - - if (comp->lvaTable[lclNum].IsAddressExposed()) - { - JITDUMP(" V%02u is address exposed\n", lclNum); - MarkLclVarAsEscaping(lclNum); - } - } - else - { - // Variable that may not point to objects will not participate in our analysis. - m_ConnGraphAdjacencyMatrix[lclNum] = BitVecOps::UninitVal(); + JITDUMP(" V%02u is address exposed\n", lclNum); + MarkLclVarAsEscaping(lclNum); } } @@ -272,7 +262,21 @@ void ObjectAllocator::ComputeEscapingNodes(BitVecTraits* bitVecTraits, BitVec& e unsigned int lclNum; + BitVecOps::Iter lclStoreIterator(bitVecTraits, m_IndirectRefStoredPointers); + while (lclStoreIterator.NextElem(&lclNum)) + { + BitSetShortLongRep lclStoreToProcess = BitVecOps::MakeCopy(bitVecTraits, m_ConnGraphAdjacencyMatrix[lclNum]); + BitVecOps::Iter targetLclIter(bitVecTraits, lclStoreToProcess); + unsigned int targetLclNum; + while (targetLclIter.NextElem(&targetLclNum)) + { + JITDUMP("V%02u may point to objects that V%02u points to\n", lclNum, targetLclNum); + BitVecOps::AddElemD(bitVecTraits, m_ConnGraphAdjacencyMatrix[targetLclNum], lclNum); + } + } + bool doOneMoreIteration = true; + while (doOneMoreIteration) { BitVecOps::Iter iterator(bitVecTraits, escapingNodesToProcess); @@ -903,10 +907,6 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent } else { - // A local may be assigned to a field of another local. - // Add an inverse edge to the connection graph too. - // For example, this.Foo = foo ?? new Foo(); - AddConnGraphEdge(srcLclNum, dstLclNum); ++parentIndex; keepChecking = true; } @@ -957,7 +957,6 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent GenTree* fieldObj = parent->AsIndir()->Addr()->AsFieldAddr()->GetFldObj(); if (fieldObj != nullptr && fieldObj->IsAnyLocal()) { - // Add an edge to the connection graph. const unsigned int dstLclNum = fieldObj->AsLclVarCommon()->GetLclNum(); const unsigned int srcLclNum = lclNum; if (dstLclNum == 0 && !comp->info.compIsStatic) @@ -977,7 +976,9 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent break; } + // Add an edge to the connection graph. AddConnGraphEdge(dstLclNum, srcLclNum); + BitVecOps::AddElemD(&m_bitVecTraits, m_IndirectRefStoredPointers, dstLclNum); ++parentIndex; keepChecking = true; break; @@ -988,7 +989,6 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent { if (parent->AsIndir()->Addr()->IsAnyLocal()) { - // Add an edge to the connection graph. const unsigned int dstLclNum = parent->AsIndir()->Addr()->AsLclVarCommon()->GetLclNum(); const unsigned int srcLclNum = lclNum; @@ -997,7 +997,9 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent break; } + // Add an edge to the connection graph. AddConnGraphEdge(dstLclNum, srcLclNum); + BitVecOps::AddElemD(&m_bitVecTraits, m_IndirectRefStoredPointers, dstLclNum); canLclVarEscapeViaParentStack = false; } } @@ -1024,24 +1026,20 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent if (call->IsHelperCall()) { - if (call->IsHelperCall(comp, CORINFO_HELP_VIRTUAL_FUNC_PTR)) - { - break; - } canLclVarEscapeViaParentStack = !Compiler::s_helperCallProperties.IsNoEscape(comp->eeGetHelperNum(call->gtCallMethHnd)); } - //else if (call->gtCallType == CT_USER_FUNC) - //{ - // // Delegate invoke won't escape the delegate which is passed as "this" - // // And gets expanded inline later. - // // - // if ((call->gtCallMoreFlags & GTF_CALL_M_DELEGATE_INV) != 0) - // { - // GenTree* const thisArg = call->gtArgs.GetThisArg()->GetNode(); - // canLclVarEscapeViaParentStack = thisArg != tree; - // } - //} + else if (call->gtCallType == CT_USER_FUNC) + { + // Delegate invoke won't escape the delegate which is passed as "this" + // And gets expanded inline later. + // + if ((call->gtCallMoreFlags & GTF_CALL_M_DELEGATE_INV) != 0) + { + GenTree* const thisArg = call->gtArgs.GetThisArg()->GetNode(); + canLclVarEscapeViaParentStack = thisArg != tree; + } + } break; } diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index b8f4f48b8179b..4caa4c198b41d 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -35,6 +35,7 @@ class ObjectAllocator final : public Phase bool m_AnalysisDone; BitVecTraits m_bitVecTraits; BitVec m_EscapingPointers; + BitVec m_IndirectRefStoredPointers; // We keep the set of possibly-stack-pointing pointers as a superset of the set of // definitely-stack-pointing pointers. All definitely-stack-pointing pointers are in both sets. BitVec m_PossiblyStackPointingPointers; @@ -100,6 +101,7 @@ inline ObjectAllocator::ObjectAllocator(Compiler* comp) m_EscapingPointers = BitVecOps::UninitVal(); m_PossiblyStackPointingPointers = BitVecOps::UninitVal(); m_DefinitelyStackPointingPointers = BitVecOps::UninitVal(); + m_IndirectRefStoredPointers = BitVecOps::MakeEmpty(&m_bitVecTraits); m_ConnGraphAdjacencyMatrix = nullptr; } @@ -148,7 +150,7 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNu bool enableBoxedValueClasses = true; bool enableRefClasses = true; - bool enableArrays = false; + bool enableArrays = true; *reason = "[ok]"; #ifdef DEBUG From 3b24b7a5c93045c6596ea0e49eb7a770de76bcc3 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 00:39:45 +0900 Subject: [PATCH 55/65] Disable arrays and delegates --- src/coreclr/jit/objectalloc.cpp | 22 +++++++++++----------- src/coreclr/jit/objectalloc.h | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index e771409989cc4..b355bcc9cdeac 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -1029,17 +1029,17 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent canLclVarEscapeViaParentStack = !Compiler::s_helperCallProperties.IsNoEscape(comp->eeGetHelperNum(call->gtCallMethHnd)); } - else if (call->gtCallType == CT_USER_FUNC) - { - // Delegate invoke won't escape the delegate which is passed as "this" - // And gets expanded inline later. - // - if ((call->gtCallMoreFlags & GTF_CALL_M_DELEGATE_INV) != 0) - { - GenTree* const thisArg = call->gtArgs.GetThisArg()->GetNode(); - canLclVarEscapeViaParentStack = thisArg != tree; - } - } + // else if (call->gtCallType == CT_USER_FUNC) + //{ + // // Delegate invoke won't escape the delegate which is passed as "this" + // // And gets expanded inline later. + // // + // if ((call->gtCallMoreFlags & GTF_CALL_M_DELEGATE_INV) != 0) + // { + // GenTree* const thisArg = call->gtArgs.GetThisArg()->GetNode(); + // canLclVarEscapeViaParentStack = thisArg != tree; + // } + // } break; } diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 4caa4c198b41d..6902413a5f499 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -150,7 +150,7 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNu bool enableBoxedValueClasses = true; bool enableRefClasses = true; - bool enableArrays = true; + bool enableArrays = false; *reason = "[ok]"; #ifdef DEBUG From b3dcc12ba0f76022802ca78f5c49e2aa68b83840 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 00:58:55 +0900 Subject: [PATCH 56/65] Remove invalid assert --- src/coreclr/jit/objectalloc.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index b355bcc9cdeac..50ed970ef28cf 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -1228,9 +1228,6 @@ void ObjectAllocator::RewriteUses() if ((lclNum < BitVecTraits::GetSize(&m_allocator->m_bitVecTraits)) && m_allocator->MayLclVarPointToStack(lclNum)) { - // Analysis does not handle indirect access to pointer locals. - assert(tree->OperIsScalarLocal()); - var_types newType; if (m_allocator->m_HeapLocalToStackLocalMap.TryGetValue(lclNum, &newLclNum)) { From 94966d12f801dee4996a352ac127d49ff40cddbf Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 04:03:59 +0900 Subject: [PATCH 57/65] Minor enhancement --- src/coreclr/jit/objectalloc.cpp | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 50ed970ef28cf..66d089e94d049 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -950,11 +950,17 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_BLK: if (tree != parent->AsIndir()->Addr()) { - switch (parent->AsIndir()->Addr()->OperGet()) + GenTree* target = parent->AsIndir()->Addr(); + while (target->OperIs(GT_ADD, GT_SUB)) + { + target = target->AsOp()->gtGetOp1(); + } + + switch (target->OperGet()) { case GT_FIELD_ADDR: { - GenTree* fieldObj = parent->AsIndir()->Addr()->AsFieldAddr()->GetFldObj(); + GenTree* fieldObj = target->AsFieldAddr()->GetFldObj(); if (fieldObj != nullptr && fieldObj->IsAnyLocal()) { const unsigned int dstLclNum = fieldObj->AsLclVarCommon()->GetLclNum(); @@ -987,9 +993,9 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent } default: { - if (parent->AsIndir()->Addr()->IsAnyLocal()) + if (target->IsAnyLocal()) { - const unsigned int dstLclNum = parent->AsIndir()->Addr()->AsLclVarCommon()->GetLclNum(); + const unsigned int dstLclNum = target->AsLclVarCommon()->GetLclNum(); const unsigned int srcLclNum = lclNum; if (comp->lvaGetDesc(dstLclNum)->TypeGet() != TYP_REF) From 8f0ef527fef680d8c47049f76480ae7fbc529841 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 04:10:18 +0900 Subject: [PATCH 58/65] Check type --- src/coreclr/jit/objectalloc.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 66d089e94d049..fa24e3eec3c46 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -951,7 +951,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent if (tree != parent->AsIndir()->Addr()) { GenTree* target = parent->AsIndir()->Addr(); - while (target->OperIs(GT_ADD, GT_SUB)) + while (target->OperIs(GT_ADD, GT_SUB) && target->TypeIs(TYP_BYREF)) { target = target->AsOp()->gtGetOp1(); } From d3b8f578bbd738297b138ede6c550ff044a00ec1 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 04:31:39 +0900 Subject: [PATCH 59/65] rollback --- src/coreclr/jit/objectalloc.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index fa24e3eec3c46..01453bbf27206 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -949,13 +949,8 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_STORE_BLK: case GT_BLK: if (tree != parent->AsIndir()->Addr()) - { - GenTree* target = parent->AsIndir()->Addr(); - while (target->OperIs(GT_ADD, GT_SUB) && target->TypeIs(TYP_BYREF)) { - target = target->AsOp()->gtGetOp1(); - } - + GenTree* target = parent->AsIndir()->Addr(); switch (target->OperGet()) { case GT_FIELD_ADDR: From 49e15bca8fac4a74bc4065b0482e49ff6580c05f Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 15:18:45 +0900 Subject: [PATCH 60/65] Refine analysis --- src/coreclr/jit/objectalloc.cpp | 31 +++++++++---------------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 01453bbf27206..62313b087ac8d 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -54,12 +54,12 @@ PhaseStatus ObjectAllocator::DoPhase() if (enabled) { - JITDUMP("enabled, analyzing...\n"); + JITDUMP("Enabled, analyzing...\n"); DoAnalysis(); } else { - JITDUMP("disabled%s, punting\n", IsObjectStackAllocationEnabled() ? disableReason : ""); + JITDUMP("Disabled%s, punting\n", IsObjectStackAllocationEnabled() ? disableReason : ""); m_IsObjectStackAllocationEnabled = false; } @@ -887,7 +887,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent GenTree* parent = parentStack->Top(parentIndex); keepChecking = false; - JITDUMP("... L%02u ... checking [%06u]\n", lclNum, comp->dspTreeID(parent)); + JITDUMP("... V%02u ... checking [%06u]\n", lclNum, comp->dspTreeID(parent)); switch (parent->OperGet()) { @@ -947,7 +947,6 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_STOREIND: case GT_STORE_BLK: - case GT_BLK: if (tree != parent->AsIndir()->Addr()) { GenTree* target = parent->AsIndir()->Addr(); @@ -965,21 +964,11 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent break; } - if (fieldObj->IsLocal()) - { - if (comp->lvaGetDesc(dstLclNum)->TypeGet() != TYP_REF) - { - break; - } - } - else if (comp->lvaGetDesc(dstLclNum)->TypeGet() != TYP_STRUCT) - { - break; - } - // Add an edge to the connection graph. AddConnGraphEdge(dstLclNum, srcLclNum); BitVecOps::AddElemD(&m_bitVecTraits, m_IndirectRefStoredPointers, dstLclNum); + JITDUMP(" V%02u is indirect accessed via field at [%06u]\n", dstLclNum, + comp->dspTreeID(target)); ++parentIndex; keepChecking = true; break; @@ -993,14 +982,11 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent const unsigned int dstLclNum = target->AsLclVarCommon()->GetLclNum(); const unsigned int srcLclNum = lclNum; - if (comp->lvaGetDesc(dstLclNum)->TypeGet() != TYP_REF) - { - break; - } - // Add an edge to the connection graph. AddConnGraphEdge(dstLclNum, srcLclNum); BitVecOps::AddElemD(&m_bitVecTraits, m_IndirectRefStoredPointers, dstLclNum); + JITDUMP(" V%02u is indirect accessed at [%06u]\n", dstLclNum, + comp->dspTreeID(target)); canLclVarEscapeViaParentStack = false; } } @@ -1008,8 +994,9 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent break; } FALLTHROUGH; + case GT_BLK: case GT_IND: - if (parent->OperIs(GT_IND) && parent->TypeIs(TYP_REF)) + if (parent->OperIs(GT_IND, GT_BLK) && parent->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL, TYP_STRUCT)) { ++parentIndex; keepChecking = true; From 878faa6fea3d14cf3e8a661e4c9ff37e07b1c76a Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 16:36:04 +0900 Subject: [PATCH 61/65] Refine analysis --- src/coreclr/jit/objectalloc.cpp | 38 +++++++++++++++++---------------- src/coreclr/jit/objectalloc.h | 2 +- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 62313b087ac8d..58de29ec82539 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -901,16 +901,13 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent AddConnGraphEdge(dstLclNum, srcLclNum); - if (!parent->AsLclVar()->TypeIs(TYP_REF)) + if (parent->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL, TYP_STRUCT)) { - canLclVarEscapeViaParentStack = false; + BitVecOps::AddElemD(&m_bitVecTraits, m_IndirectRefStoredPointers, dstLclNum); } - else - { - ++parentIndex; - keepChecking = true; + + canLclVarEscapeViaParentStack = false; } - } break; case GT_EQ: @@ -1017,17 +1014,19 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent canLclVarEscapeViaParentStack = !Compiler::s_helperCallProperties.IsNoEscape(comp->eeGetHelperNum(call->gtCallMethHnd)); } - // else if (call->gtCallType == CT_USER_FUNC) - //{ - // // Delegate invoke won't escape the delegate which is passed as "this" - // // And gets expanded inline later. - // // - // if ((call->gtCallMoreFlags & GTF_CALL_M_DELEGATE_INV) != 0) - // { - // GenTree* const thisArg = call->gtArgs.GetThisArg()->GetNode(); - // canLclVarEscapeViaParentStack = thisArg != tree; - // } - // } + else if (call->gtCallType == CT_USER_FUNC) + { + // Delegate invoke won't escape the delegate which is passed as "this" + // And gets expanded inline later. + // + if ((call->gtCallMoreFlags & GTF_CALL_M_DELEGATE_INV) != 0) + { + GenTree* const thisArg = call->gtArgs.GetThisArg()->GetNode(); + canLclVarEscapeViaParentStack = thisArg != tree; + ++parentIndex; + keepChecking = true; + } + } break; } @@ -1216,6 +1215,9 @@ void ObjectAllocator::RewriteUses() if ((lclNum < BitVecTraits::GetSize(&m_allocator->m_bitVecTraits)) && m_allocator->MayLclVarPointToStack(lclNum)) { + // Analysis does not handle indirect access to pointer locals. + assert(tree->OperIsScalarLocal()); + var_types newType; if (m_allocator->m_HeapLocalToStackLocalMap.TryGetValue(lclNum, &newLclNum)) { diff --git a/src/coreclr/jit/objectalloc.h b/src/coreclr/jit/objectalloc.h index 6902413a5f499..4caa4c198b41d 100644 --- a/src/coreclr/jit/objectalloc.h +++ b/src/coreclr/jit/objectalloc.h @@ -150,7 +150,7 @@ inline bool ObjectAllocator::CanAllocateLclVarOnStack(unsigned int lclNu bool enableBoxedValueClasses = true; bool enableRefClasses = true; - bool enableArrays = false; + bool enableArrays = true; *reason = "[ok]"; #ifdef DEBUG From 6931affbe1466fe2f2e2bb51b5b5b9eea7083e7e Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 16:49:52 +0900 Subject: [PATCH 62/65] Fix format --- src/coreclr/jit/objectalloc.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 58de29ec82539..21bc91ff1ab17 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -906,9 +906,9 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent BitVecOps::AddElemD(&m_bitVecTraits, m_IndirectRefStoredPointers, dstLclNum); } - canLclVarEscapeViaParentStack = false; - } - break; + canLclVarEscapeViaParentStack = false; + break; + } case GT_EQ: case GT_NE: @@ -945,7 +945,7 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_STOREIND: case GT_STORE_BLK: if (tree != parent->AsIndir()->Addr()) - { + { GenTree* target = parent->AsIndir()->Addr(); switch (target->OperGet()) { From 38ba8087652fba656baae3859c07fc17f41b1034 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 17:23:17 +0900 Subject: [PATCH 63/65] Fully tracking stores --- src/coreclr/jit/objectalloc.cpp | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 21bc91ff1ab17..9dc302768f379 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -190,13 +190,6 @@ void ObjectAllocator::MarkEscapingVarsAndBuildConnGraph() GenTree* const tree = *use; unsigned const lclNum = tree->AsLclVarCommon()->GetLclNum(); - // If this local already escapes, no need to look further. - // - if (m_allocator->CanLclVarEscape(lclNum)) - { - return Compiler::fgWalkResult::WALK_CONTINUE; - } - bool lclEscapes = true; if (tree->OperIsLocalStore()) From 08e8fb26666da2e7f9602d7185ca58435414f164 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 17:28:41 +0900 Subject: [PATCH 64/65] Full tracking leftovers --- src/coreclr/jit/objectalloc.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 9dc302768f379..018ad13cc1fd5 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -893,12 +893,6 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent const unsigned int srcLclNum = lclNum; AddConnGraphEdge(dstLclNum, srcLclNum); - - if (parent->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL, TYP_STRUCT)) - { - BitVecOps::AddElemD(&m_bitVecTraits, m_IndirectRefStoredPointers, dstLclNum); - } - canLclVarEscapeViaParentStack = false; break; } From 66fc46b7d2ee6f92456d60b1ae65426d780e4ed7 Mon Sep 17 00:00:00 2001 From: Steven He Date: Sun, 21 Jul 2024 21:24:36 +0900 Subject: [PATCH 65/65] More fixes --- src/coreclr/jit/objectalloc.cpp | 56 ++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/src/coreclr/jit/objectalloc.cpp b/src/coreclr/jit/objectalloc.cpp index 018ad13cc1fd5..ee16546f528e0 100644 --- a/src/coreclr/jit/objectalloc.cpp +++ b/src/coreclr/jit/objectalloc.cpp @@ -258,9 +258,8 @@ void ObjectAllocator::ComputeEscapingNodes(BitVecTraits* bitVecTraits, BitVec& e BitVecOps::Iter lclStoreIterator(bitVecTraits, m_IndirectRefStoredPointers); while (lclStoreIterator.NextElem(&lclNum)) { - BitSetShortLongRep lclStoreToProcess = BitVecOps::MakeCopy(bitVecTraits, m_ConnGraphAdjacencyMatrix[lclNum]); - BitVecOps::Iter targetLclIter(bitVecTraits, lclStoreToProcess); - unsigned int targetLclNum; + BitVecOps::Iter targetLclIter(bitVecTraits, m_ConnGraphAdjacencyMatrix[lclNum]); + unsigned int targetLclNum; while (targetLclIter.NextElem(&targetLclNum)) { JITDUMP("V%02u may point to objects that V%02u points to\n", lclNum, targetLclNum); @@ -893,6 +892,12 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent const unsigned int srcLclNum = lclNum; AddConnGraphEdge(dstLclNum, srcLclNum); + + if (parent->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL, TYP_STRUCT)) + { + AddConnGraphEdge(srcLclNum, dstLclNum); + } + canLclVarEscapeViaParentStack = false; break; } @@ -924,6 +929,9 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent case GT_FIELD_ADDR: case GT_INDEX_ADDR: case GT_LCL_ADDR: + case GT_CAST: + case GT_BLK: + case GT_IND: // Check whether the local escapes via its grandparent. ++parentIndex; keepChecking = true; @@ -943,7 +951,10 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent { const unsigned int dstLclNum = fieldObj->AsLclVarCommon()->GetLclNum(); const unsigned int srcLclNum = lclNum; - if (dstLclNum == 0 && !comp->info.compIsStatic) + LclVarDsc* lclVarDsc = comp->lvaGetDesc(dstLclNum); + + // Escapes through 'this' pointer + if (!comp->info.compIsStatic && dstLclNum == comp->info.compThisArg) { break; } @@ -965,10 +976,24 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent { const unsigned int dstLclNum = target->AsLclVarCommon()->GetLclNum(); const unsigned int srcLclNum = lclNum; + LclVarDsc* lclVarDsc = comp->lvaGetDesc(dstLclNum); + + BitVecOps::AddElemD(&m_bitVecTraits, m_IndirectRefStoredPointers, dstLclNum); + + // Escapes through 'this' pointer + if (dstLclNum == 0 && !comp->info.compIsStatic) + { + break; + } + + // Escapes through a byref parameter + if (lclVarDsc->lvIsParam && lclVarDsc->TypeGet() == TYP_BYREF) + { + break; + } // Add an edge to the connection graph. AddConnGraphEdge(dstLclNum, srcLclNum); - BitVecOps::AddElemD(&m_bitVecTraits, m_IndirectRefStoredPointers, dstLclNum); JITDUMP(" V%02u is indirect accessed at [%06u]\n", dstLclNum, comp->dspTreeID(target)); canLclVarEscapeViaParentStack = false; @@ -977,19 +1002,9 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack* parent } break; } - FALLTHROUGH; - case GT_BLK: - case GT_IND: - if (parent->OperIs(GT_IND, GT_BLK) && parent->TypeIs(TYP_REF, TYP_BYREF, TYP_I_IMPL, TYP_STRUCT)) - { - ++parentIndex; - keepChecking = true; - } - else - { - // Address of the field/ind is not taken so the local doesn't escape. - canLclVarEscapeViaParentStack = false; - } + + // Address of the field/ind is not taken so the local doesn't escape. + canLclVarEscapeViaParentStack = false; break; case GT_CALL: @@ -1202,9 +1217,6 @@ void ObjectAllocator::RewriteUses() if ((lclNum < BitVecTraits::GetSize(&m_allocator->m_bitVecTraits)) && m_allocator->MayLclVarPointToStack(lclNum)) { - // Analysis does not handle indirect access to pointer locals. - assert(tree->OperIsScalarLocal()); - var_types newType; if (m_allocator->m_HeapLocalToStackLocalMap.TryGetValue(lclNum, &newLclNum)) { @@ -1216,7 +1228,7 @@ void ObjectAllocator::RewriteUses() else { newType = m_allocator->DoesLclVarPointToStack(lclNum) ? TYP_I_IMPL : TYP_BYREF; - if (tree->TypeGet() == TYP_REF) + if (tree->TypeIs(TYP_REF, TYP_I_IMPL)) { tree->ChangeType(newType); }