Skip to content

Commit

Permalink
Jit: Conditional Escape Analysis and Cloning (#111473)
Browse files Browse the repository at this point in the history
Enhance escape analysis to determine if an object escapes only under
failed GDV tests. If so, clone to create a path of code so that
the object doesn't escape, and then stack allocate the object.

More details in the included document.

Contributes to #108913

Co-authored-by: Aman Khalid <[email protected]>
  • Loading branch information
AndyAyersMS and amanasifkhalid authored Feb 4, 2025
1 parent e567edb commit f6c74b8
Show file tree
Hide file tree
Showing 16 changed files with 3,048 additions and 139 deletions.
804 changes: 804 additions & 0 deletions docs/design/coreclr/jit/DeabstractionAndConditionalEscapeAnalysis.md

Large diffs are not rendered by default.

27 changes: 25 additions & 2 deletions src/coreclr/jit/compiler.h
Original file line number Diff line number Diff line change
Expand Up @@ -667,6 +667,9 @@ class LclVarDsc

unsigned char lvRedefinedInEmbeddedStatement : 1; // Local has redefinitions inside embedded statements that
// disqualify it from local copy prop.

unsigned char lvIsEnumerator : 1; // Local is assigned exact class where : IEnumerable<T> via GDV

private:
unsigned char lvIsNeverNegative : 1; // The local is known to be never negative

Expand Down Expand Up @@ -3711,6 +3714,8 @@ class Compiler
return gtNewStmt(exprClone, stmt->GetDebugInfo());
}

Statement* gtLatestStatement(Statement* stmt1, Statement* stmt2);

// Internal helper for cloning a call
GenTreeCall* gtCloneExprCallHelper(GenTreeCall* call);

Expand Down Expand Up @@ -4606,6 +4611,26 @@ class Compiler

unsigned impStkSize; // Size of the full stack

// Enumerator de-abstraction support
//
typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, unsigned> NodeToUnsignedMap;

// Map is only set on the root instance.
//
NodeToUnsignedMap* impEnumeratorGdvLocalMap = nullptr;
bool hasImpEnumeratorGdvLocalMap() { return impInlineRoot()->impEnumeratorGdvLocalMap != nullptr; }
NodeToUnsignedMap* getImpEnumeratorGdvLocalMap()
{
Compiler* compiler = impInlineRoot();
if (compiler->impEnumeratorGdvLocalMap == nullptr)
{
CompAllocator alloc(compiler->getAllocator(CMK_Generic));
compiler->impEnumeratorGdvLocalMap = new (alloc) NodeToUnsignedMap(alloc);
}

return compiler->impEnumeratorGdvLocalMap;
}

#define SMALL_STACK_SIZE 16 // number of elements in impSmallStack

struct SavedStack // used to save/restore stack contents.
Expand Down Expand Up @@ -11677,8 +11702,6 @@ class Compiler
return compRoot->m_fieldSeqStore;
}

typedef JitHashTable<GenTree*, JitPtrKeyFuncs<GenTree>, unsigned> NodeToUnsignedMap;

NodeToUnsignedMap* m_memorySsaMap[MemoryKindCount];

// In some cases, we want to assign intermediate SSA #'s to memory states, and know what nodes create those memory
Expand Down
9 changes: 8 additions & 1 deletion src/coreclr/jit/fgdiagnostic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2860,7 +2860,14 @@ bool BBPredsChecker::CheckEHFinallyRet(BasicBlock* blockPred, BasicBlock* block)
}
}

assert(found && "BBJ_EHFINALLYRET predecessor of block that doesn't follow a BBJ_CALLFINALLY!");
if (!found)
{
JITDUMP(FMT_BB " is successor of finallyret " FMT_BB " but prev block is not a callfinally to " FMT_BB
" (search range was [" FMT_BB "..." FMT_BB "]\n",
block->bbNum, blockPred->bbNum, finBeg->bbNum, firstBlock->bbNum, lastBlock->bbNum);
assert(!"BBJ_EHFINALLYRET predecessor of block that doesn't follow a BBJ_CALLFINALLY!");
}

return found;
}

Expand Down
7 changes: 3 additions & 4 deletions src/coreclr/jit/fgehopt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2552,6 +2552,7 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
auto addBlockToClone = [=, &blocks, &visited, &numberOfBlocksToClone](BasicBlock* block, const char* msg) {
if (!BitVecOps::TryAddElemD(traits, visited, block->bbID))
{
JITDUMP("[already seen] %s block " FMT_BB "\n", msg, block->bbNum);
return false;
}

Expand Down Expand Up @@ -2781,6 +2782,8 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
// all the EH indicies at or above insertBeforeIndex will shift,
// and the EH table may reallocate.
//
// This addition may also fail, if the table would become too large...
//
EHblkDsc* const clonedOutermostEbd =
fgTryAddEHTableEntries(insertBeforeIndex, regionCount, /* deferAdding */ deferCloning);

Expand Down Expand Up @@ -3000,8 +3003,6 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
cloneTryIndex += indexShift;
}

EHblkDsc* const originalEbd = ehGetDsc(originalTryIndex);
EHblkDsc* const clonedEbd = ehGetDsc(cloneTryIndex);
newBlock->setTryIndex(cloneTryIndex);
updateBlockReferences(cloneTryIndex);
}
Expand All @@ -3016,8 +3017,6 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info,
cloneHndIndex += indexShift;
}

EHblkDsc* const originalEbd = ehGetDsc(originalHndIndex);
EHblkDsc* const clonedEbd = ehGetDsc(cloneHndIndex);
newBlock->setHndIndex(cloneHndIndex);
updateBlockReferences(cloneHndIndex);

Expand Down
39 changes: 39 additions & 0 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -32679,3 +32679,42 @@ void GenTree::SetMorphed(Compiler* compiler, bool doChildren /* = false */)
}
}
#endif

//------------------------------------------------------------------------
// gtLatestStmt: determine which of two statements happens later
//
// Arguments:
// stmt1 - first statement to consider
// stmt2 - second statement to consider
//
// Returns:
// either stmt1 or stmt2, whichever happens later in the block
//
Statement* Compiler::gtLatestStatement(Statement* stmt1, Statement* stmt2)
{
if (stmt1 == stmt2)
{
return stmt1;
}

Statement* cursor1 = stmt1->GetNextStmt();
Statement* cursor2 = stmt2->GetNextStmt();

while (true)
{
if ((cursor1 == stmt2) || (cursor2 == nullptr))
{
return stmt2;
}

if ((cursor2 == stmt1) || (cursor1 == nullptr))
{
return stmt1;
}

cursor1 = cursor1->GetNextStmt();
cursor2 = cursor2->GetNextStmt();
}

assert(!"could not determine latest stmt");
}
52 changes: 52 additions & 0 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3445,6 +3445,21 @@ void Compiler::impImportAndPushBox(CORINFO_RESOLVED_TOKEN* pResolvedToken)
{
GenTreeCall* const call = exprToBox->AsRetExpr()->gtInlineCandidate->AsCall();

// If the call was flagged for possible enumerator cloning, flag the allocation as well.
//
if (compIsForInlining() && hasImpEnumeratorGdvLocalMap())
{
NodeToUnsignedMap* const map = getImpEnumeratorGdvLocalMap();
unsigned enumeratorLcl = BAD_VAR_NUM;
GenTreeCall* const call = impInlineInfo->iciCall;
if (map->Lookup(call, &enumeratorLcl))
{
JITDUMP("Flagging [%06u] for enumerator cloning via V%02u\n", dspTreeID(op1), enumeratorLcl);
map->Remove(call);
map->Set(op1, enumeratorLcl);
}
}

if (call->ShouldHaveRetBufArg())
{
JITDUMP("Must insert newobj stmts for box before call [%06u]\n", dspTreeID(call));
Expand Down Expand Up @@ -6740,6 +6755,26 @@ void Compiler::impImportBlockCode(BasicBlock* block)
{
lvaUpdateClass(lclNum, op1, tiRetVal.GetClassHandleForObjRef());
}

// If we see a local being assigned the result of a GDV-inlineable
// IEnumerable<T>.GetEnumerator, keep track of both the local and the call.
//
if (op1->OperIs(GT_RET_EXPR))
{
JITDUMP(".... checking for GDV of IEnumerable<T>...\n");

GenTreeCall* const call = op1->AsRetExpr()->gtInlineCandidate;
NamedIntrinsic const ni = lookupNamedIntrinsic(call->gtCallMethHnd);

if (ni == NI_System_Collections_Generic_IEnumerable_GetEnumerator)
{
JITDUMP("V%02u value is GDV of IEnumerable<T>.GetEnumerator\n", lclNum);
lvaTable[lclNum].lvIsEnumerator = true;
JITDUMP("Flagging [%06u] for enumerator cloning via V%02u\n", dspTreeID(call), lclNum);
getImpEnumeratorGdvLocalMap()->Set(call, lclNum);
Metrics.EnumeratorGDV++;
}
}
}

/* Filter out simple stores to itself */
Expand Down Expand Up @@ -8838,6 +8873,23 @@ void Compiler::impImportBlockCode(BasicBlock* block)
op1->gtFlags |= GTF_ALLOCOBJ_EMPTY_STATIC;
}

// If the method being imported is an inlinee, and the original call was flagged
// for possible enumerator cloning, flag the allocation as well.
//
if (compIsForInlining() && hasImpEnumeratorGdvLocalMap())
{
NodeToUnsignedMap* const map = getImpEnumeratorGdvLocalMap();
unsigned enumeratorLcl = BAD_VAR_NUM;
GenTreeCall* const call = impInlineInfo->iciCall;
if (map->Lookup(call, &enumeratorLcl))
{
JITDUMP("Flagging [%06u] for enumerator cloning via V%02u\n", dspTreeID(op1),
enumeratorLcl);
map->Remove(call);
map->Set(op1, enumeratorLcl);
}
}

// Remember that this basic block contains 'new' of an object
block->SetFlags(BBF_HAS_NEWOBJ);
optMethodFlags |= OMF_HAS_NEWOBJ;
Expand Down
7 changes: 7 additions & 0 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10570,6 +10570,13 @@ NamedIntrinsic Compiler::lookupNamedIntrinsic(CORINFO_METHOD_HANDLE method)
result = NI_System_Collections_Generic_EqualityComparer_get_Default;
}
}
else if (strcmp(className, "IEnumerable`1") == 0)
{
if (strcmp(methodName, "GetEnumerator") == 0)
{
result = NI_System_Collections_Generic_IEnumerable_GetEnumerator;
}
}
}
else if (strcmp(namespaceName, "Numerics") == 0)
{
Expand Down
16 changes: 16 additions & 0 deletions src/coreclr/jit/indirectcalltransformer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,22 @@ class IndirectCallTransformer
GenTreeCall* call = compiler->gtCloneCandidateCall(origCall);
call->gtArgs.GetThisArg()->SetEarlyNode(compiler->gtNewLclvNode(thisTemp, TYP_REF));

// If the original call was flagged as one that might inspire enumerator de-abstraction
// cloning, move the flag to the devirtualized call.
//
if (compiler->hasImpEnumeratorGdvLocalMap())
{
Compiler::NodeToUnsignedMap* const map = compiler->getImpEnumeratorGdvLocalMap();
unsigned enumeratorLcl = BAD_VAR_NUM;
if (map->Lookup(origCall, &enumeratorLcl))
{
JITDUMP("Flagging [%06u] for enumerator cloning via V%02u\n", compiler->dspTreeID(call),
enumeratorLcl);
map->Remove(origCall);
map->Set(call, enumeratorLcl);
}
}

INDEBUG(call->SetIsGuarded());

JITDUMP("Direct call [%06u] in block " FMT_BB "\n", compiler->dspTreeID(call), block->bbNum);
Expand Down
41 changes: 1 addition & 40 deletions src/coreclr/jit/inductionvariableopts.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1392,7 +1392,6 @@ class StrengthReductionContext
BasicBlock* FindPostUseUpdateInsertionPoint(ArrayStack<CursorInfo>* cursors,
BasicBlock* backEdgeDominator,
Statement** afterStmt);
Statement* LatestStatement(Statement* stmt1, Statement* stmt2);
bool InsertionPointPostDominatesUses(BasicBlock* insertionPoint, ArrayStack<CursorInfo>* cursors);

bool StressProfitability()
Expand Down Expand Up @@ -2615,7 +2614,7 @@ BasicBlock* StrengthReductionContext::FindPostUseUpdateInsertionPoint(ArrayStack
}
else
{
latestStmt = LatestStatement(latestStmt, cursor.Stmt);
latestStmt = m_comp->gtLatestStatement(latestStmt, cursor.Stmt);
}
}

Expand All @@ -2632,44 +2631,6 @@ BasicBlock* StrengthReductionContext::FindPostUseUpdateInsertionPoint(ArrayStack
return nullptr;
}

//------------------------------------------------------------------------
// LatestStatement: Given two statements in the same basic block, return the
// latter of the two.
//
// Parameters:
// stmt1 - First statement
// stmt2 - Second statement
//
// Returns:
// Latter of the statements.
//
Statement* StrengthReductionContext::LatestStatement(Statement* stmt1, Statement* stmt2)
{
if (stmt1 == stmt2)
{
return stmt1;
}

Statement* cursor1 = stmt1->GetNextStmt();
Statement* cursor2 = stmt2->GetNextStmt();

while (true)
{
if ((cursor1 == stmt2) || (cursor2 == nullptr))
{
return stmt2;
}

if ((cursor2 == stmt1) || (cursor1 == nullptr))
{
return stmt1;
}

cursor1 = cursor1->GetNextStmt();
cursor2 = cursor2->GetNextStmt();
}
}

//------------------------------------------------------------------------
// InsertionPointPostDominatesUses: Check if a basic block post-dominates all
// locations specified by the cursors.
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/jit/jitconfigvalues.h
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,8 @@ CONFIG_STRING(JitObjectStackAllocationRange, "JitObjectStackAllocationRange")
RELEASE_CONFIG_INTEGER(JitObjectStackAllocation, "JitObjectStackAllocation", 1)
RELEASE_CONFIG_INTEGER(JitObjectStackAllocationRefClass, "JitObjectStackAllocationRefClass", 1)
RELEASE_CONFIG_INTEGER(JitObjectStackAllocationBoxedValueClass, "JitObjectStackAllocationBoxedValueClass", 1)
RELEASE_CONFIG_INTEGER(JitObjectStackAllocationConditionalEscape, "JitObjectStackAllocationConditionalEscape", 1)
CONFIG_STRING(JitObjectStackAllocationConditionalEscapeRange, "JitObjectStackAllocationConditionalEscapeRange")
RELEASE_CONFIG_INTEGER(JitObjectStackAllocationArray, "JitObjectStackAllocationArray", 1)
RELEASE_CONFIG_INTEGER(JitObjectStackAllocationSize, "JitObjectStackAllocationSize", 528)

Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/jitmetadatalist.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ JITMETADATAMETRIC(ClassGDV, int, 0)
JITMETADATAMETRIC(MethodGDV, int, 0)
JITMETADATAMETRIC(MultiGuessGDV, int, 0)
JITMETADATAMETRIC(ChainedGDV, int, 0)
JITMETADATAMETRIC(EnumeratorGDV, int, 0)
JITMETADATAMETRIC(InlinerBranchFold, int, 0)
JITMETADATAMETRIC(InlineAttempt, int, 0)
JITMETADATAMETRIC(InlineCount, int, 0)
Expand Down Expand Up @@ -92,6 +93,8 @@ JITMETADATAMETRIC(LocalAssertionCount, int, 0)
JITMETADATAMETRIC(LocalAssertionOverflow, int, 0)
JITMETADATAMETRIC(MorphTrackedLocals, int, 0)
JITMETADATAMETRIC(MorphLocals, int, 0)
JITMETADATAMETRIC(EnumeratorGDVProvisionalNoEscape, int, 0)
JITMETADATAMETRIC(EnumeratorGDVCanCloneToEnsureNoEscape, int, 0)

#undef JITMETADATA
#undef JITMETADATAINFO
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/namedintrinsiclist.h
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ enum NamedIntrinsic : unsigned short
//
NI_System_SZArrayHelper_GetEnumerator,
NI_System_Array_T_GetEnumerator,
NI_System_Collections_Generic_IEnumerable_GetEnumerator,
};

#endif // _NAMEDINTRINSICLIST_H_
Loading

0 comments on commit f6c74b8

Please sign in to comment.