Skip to content

JIT: Clear out return value references from continuations #129157

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

Open
jakobbotsch wants to merge 3 commits into
base: main
Choose a base branch
from
+71 −1
Open

JIT: Clear out return value references from continuations #129157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c2e7016
JIT: Clear out return value references from continuations
jakobbotsch Jun 9, 2026
ac32080
Feedback
jakobbotsch Jun 9, 2026
6574b37
Run jit-format
jakobbotsch Jun 9, 2026
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
71 changes: 70 additions & 1 deletion src/coreclr/jit/async.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3050,7 +3050,8 @@ BasicBlock* AsyncTransformation::RethrowExceptionOnResumption(BasicBlock*
//------------------------------------------------------------------------
// AsyncTransformation::CopyReturnValueOnResumption:
// Create IR that copies the return value from the continuation object to the
// right local.
// right local. When continuations may be reused, also clears out any GC
// references in the return value from the continuation afterwards.
//
// Parameters:
// call - The async call.
Expand Down Expand Up @@ -3149,6 +3150,74 @@ void AsyncTransformation::CopyReturnValueOnResumption(GenTreeCall*

LIR::AsRange(storeResultBB).InsertAtEnd(LIR::SeqTree(m_compiler, storeResult));
}

if (ReuseContinuations())
{
ClearReturnValueOnResumption(retInfo, resultOffset, storeResultBB);
}
}

//------------------------------------------------------------------------
// AsyncTransformation::ClearReturnValueOnResumption:
// Create IR that clears out any GC references in the return value from the
// continuation object. This is used after the return value has been copied
// out to ensure that a reused continuation does not keep those references
// alive.
//
// Parameters:
// retInfo - Information about the return value in the continuation.
// resultOffset - Offset of the return value from the start of the continuation object.
// storeResultBB - Basic block to append IR to.
//
void AsyncTransformation::ClearReturnValueOnResumption(const ReturnInfo* retInfo,
unsigned resultOffset,
BasicBlock* storeResultBB)
{
auto clearGCRef = [=](unsigned offset, var_types type) {
GenTree* base = m_compiler->gtNewLclvNode(m_compiler->lvaAsyncContinuationArg, TYP_REF);
GenTree* zero = m_compiler->gtNewZeroConNode(type);
GenTree* clear = StoreAtOffset(base, offset, zero, type);
LIR::AsRange(storeResultBB).InsertAtEnd(LIR::SeqTree(m_compiler, clear));
};

if (retInfo->Type.ReturnType == TYP_STRUCT)
{
ClassLayout* retLayout = retInfo->Type.ReturnLayout;
unsigned gcPtrCount = retLayout->GetGCPtrCount();
if (gcPtrCount == 0)
{
return;
}

// If there are few GC references, and at most half of the struct is
// made up of GC references, then clear the individual GC pointers
// instead of zeroing out the whole struct.
if ((gcPtrCount <= 4) && ((gcPtrCount * 2) <= retLayout->GetSlotCount()))
Comment on lines +3192 to +3195

@jakobbotsch jakobbotsch Jun 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@EgorBo Any suggestion on the heuristic to use here? Wondering if something in the backend does similar reasoning.
Could also always just be a struct clear.

@EgorBo EgorBo Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@jakobbotsch I assume we don't care about non-gc slots? it's actually a very interesting question that might improve perf everywhere. Example: https://godbolt.org/z/xMTcqn8EW today in SkipLocalsInit context (which is default everywhere in BCL) we don't try to optimize it like you do here 🤔 so I assume a fix for that might improve perf in many places.

I wonder if we should introduce a "GT_UNINIT" (or GT_POISON?) node as RHL of GT_STORE_BLK and then lower will come up with the best strategy

@jakobbotsch jakobbotsch Jun 9, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Right, we only care about clearing GC slots to ensure they don't stay rooted.

Example: https://godbolt.org/z/xMTcqn8EW today in SkipLocalsInit context (which is default everywhere in BCL) we don't try to optimize it like you do here 🤔

We do have some logic it seems:

runtime/src/coreclr/jit/codegencommon.cpp

Lines 4097 to 4112 in 4b6b5d0

if (varDsc->TypeIs(TYP_STRUCT) && !m_compiler->info.compInitMem &&
(varDsc->lvExactSize() >= TARGET_POINTER_SIZE))
{
// We only initialize the GC variables in the TYP_STRUCT
const unsigned slots = (unsigned)m_compiler->lvaLclStackHomeSize(varNum) / REGSIZE_BYTES;
ClassLayout* layout = varDsc->GetLayout();
for (unsigned i = 0; i < slots; i++)
{
if (layout->IsGCPtr(i))
{
GetEmitter()->emitIns_S_R(ins_Store(TYP_I_IMPL), EA_PTRSIZE,
genGetZeroReg(initReg, pInitRegZeroed), varNum, i * REGSIZE_BYTES);
}
}
}

{
for (unsigned i = 0; i < retLayout->GetSlotCount(); i++)
{
if (retLayout->IsGCPtr(i))
{
clearGCRef(resultOffset + (i * TARGET_POINTER_SIZE), retLayout->GetGCPtrType(i));
}
}
}
else
{
GenTree* base = m_compiler->gtNewLclvNode(m_compiler->lvaAsyncContinuationArg, TYP_REF);
GenTree* offset = m_compiler->gtNewIconNode((ssize_t)resultOffset, TYP_I_IMPL);
GenTree* addr = m_compiler->gtNewOperNode(GT_ADD, TYP_BYREF, base, offset);
GenTreeFlags indirFlags =
GTF_IND_NONFAULTING | (retInfo->HeapAlignment() < retInfo->Alignment ? GTF_IND_UNALIGNED : GTF_EMPTY);
GenTree* zero = m_compiler->gtNewIconNode(0);
GenTree* store = m_compiler->gtNewStoreValueNode(retLayout, addr, zero, indirFlags);

@EgorBo EgorBo Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you only zero the area betwenn the very first to very last gc slots?

LIR::AsRange(storeResultBB).InsertAtEnd(LIR::SeqTree(m_compiler, store));
}
}
else if (retInfo->Type.ReturnType == TYP_REF)
{
clearGCRef(resultOffset, TYP_REF);
}
}

//------------------------------------------------------------------------
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/async.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ class AsyncTransformation
const CallDefinitionInfo& callDefInfo,
const ContinuationLayout& layout,
BasicBlock* storeResultBB);
void ClearReturnValueOnResumption(const ReturnInfo* retInfo, unsigned resultOffset, BasicBlock* storeResultBB);

GenTreeIndir* LoadFromOffset(GenTree* base,
unsigned offset,
Expand Down
Loading