Skip to content

JIT: stop tracking TYP_I_IMPL locals in escape analysis #114130

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 3, 2025
Merged

JIT: stop tracking TYP_I_IMPL locals in escape analysis #114130

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
baff212
JIT: stop tracking TYP_I_IMPL locals in escape analysis
AndyAyersMS Apr 1, 2025
6f46294
fix GT_SUB type propagation
AndyAyersMS Apr 2, 2025
0ff879e
add comments/asserts to sub retyping
AndyAyersMS Apr 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 51 additions & 6 deletions src/coreclr/jit/objectalloc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ ObjectAllocator::ObjectAllocator(Compiler* comp)
//
bool ObjectAllocator::IsTrackedType(var_types type)
{
const bool isTrackableScalar = (type == TYP_REF) || (genActualType(type) == TYP_I_IMPL) || (type == TYP_BYREF);
const bool isTrackableScalar = (type == TYP_REF) || (type == TYP_BYREF);
return isTrackableScalar;
}

Expand Down Expand Up @@ -1579,12 +1579,19 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack<GenTree*>* parent

switch (parent->OperGet())
{
// Update the connection graph if we are storing to a local.
// For all other stores we mark the local as escaping.
case GT_STORE_LCL_VAR:
{
// Add an edge to the connection graph.
const unsigned int dstLclNum = parent->AsLclVar()->GetLclNum();

// If we're not tracking stores to this local, the value
// does not escape.
if (!IsTrackedLocal(dstLclNum))
{
canLclVarEscapeViaParentStack = false;
break;
}
Comment on lines +1586 to +1592
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that true? What if the object is pinned and a TYP_I_IMPL pointer to it is passed somewhere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pinning only lasts for the current stack frame, and the object being pinned must already exist, so the pinning lifetime will be shorter than the allocating stack frame lifetime.

I suppose the object (or part of the object) could be exposed cross-thread this way, so even though it could be stack allocated it would not be thread private.


// Add an edge to the connection graph.
const unsigned int srcLclNum = lclNum;

AddConnGraphEdge(dstLclNum, srcLclNum);
Expand Down Expand Up @@ -1626,13 +1633,25 @@ bool ObjectAllocator::CanLclVarEscapeViaParentStack(ArrayStack<GenTree*>* parent
case GT_COLON:
case GT_QMARK:
case GT_ADD:
case GT_SUB:
case GT_FIELD_ADDR:
// Check whether the local escapes via its grandparent.
++parentIndex;
keepChecking = true;
break;

case GT_SUB:
// Sub of two GC refs is no longer a GC ref.
if (!parent->TypeIs(TYP_BYREF, TYP_REF))
{
canLclVarEscapeViaParentStack = false;
break;
}

// Check whether the local escapes higher up
++parentIndex;
keepChecking = true;
break;

case GT_BOX:
isCopy = wasCopy;
++parentIndex;
Expand Down Expand Up @@ -1787,7 +1806,6 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack<GenTree*>* p
FALLTHROUGH;
case GT_QMARK:
case GT_ADD:
case GT_SUB:
case GT_FIELD_ADDR:
case GT_INDEX_ADDR:
case GT_BOX:
Expand All @@ -1799,6 +1817,33 @@ void ObjectAllocator::UpdateAncestorTypes(GenTree* tree, ArrayStack<GenTree*>* p
keepChecking = true;
break;

case GT_SUB:
{
// Parent type can be TYP_I_IMPL, TYP_BYREF.
// But not TYP_REF.
//
var_types parentType = parent->TypeGet();
assert(parentType != TYP_REF);

// New type can be TYP_I_IMPL, TYP_BYREF.
// But TYP_BYREF only if parent is also
//
if (parentType != newType)
{
// We must be retyping TYP_BYREF to TYP_I_IMPL.
//
assert(newType == TYP_I_IMPL);
assert(parentType == TYP_BYREF);
parent->ChangeType(newType);

// Propgate that upwards.
//
++parentIndex;
keepChecking = true;
}
break;
}

case GT_COLON:
{
GenTree* const lhs = parent->AsOp()->gtGetOp1();
Expand Down
Loading