From e7f430a58aa055b7a491bc08acc621b4870532d8 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 15 Jul 2025 15:56:48 +0200 Subject: [PATCH 01/28] WIP --- .../CompilerServices/AsyncHelpers.CoreCLR.cs | 27 +- src/coreclr/clrdefinitions.cmake | 2 +- src/coreclr/inc/corinfo.h | 2 + src/coreclr/jit/async.cpp | 236 ++++++++++-------- src/coreclr/jit/compiler.h | 12 +- src/coreclr/jit/gentree.cpp | 26 +- src/coreclr/jit/gentree.h | 3 + src/coreclr/jit/gschecks.cpp | 2 +- src/coreclr/jit/lclmorph.cpp | 13 +- src/coreclr/jit/lclvars.cpp | 14 +- src/coreclr/jit/morph.cpp | 8 + src/coreclr/jit/promotion.cpp | 2 +- src/coreclr/jit/valuenum.cpp | 2 +- src/tests/async/Directory.Build.targets | 13 - .../synchronization-context.csproj | 1 + 15 files changed, 213 insertions(+), 150 deletions(-) delete mode 100644 src/tests/async/Directory.Build.targets diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs index 9f379245a309b2..fc2b3cb84f3364 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs @@ -591,13 +591,32 @@ private static ValueTask FinalizeValueTaskReturningThunk(Continuation continuati } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static void RestoreExecutionContext(ExecutionContext? previousExecutionCtx) + private static void RestoreExecutionContext(ExecutionContext? previousExecCtx) { Thread thread = Thread.CurrentThreadAssumedInitialized; - ExecutionContext? currentExecutionCtx = thread._executionContext; - if (previousExecutionCtx != currentExecutionCtx) + ExecutionContext? currentExecCtx = thread._executionContext; + if (previousExecCtx != currentExecCtx) { - ExecutionContext.RestoreChangedContextToThread(thread, previousExecutionCtx, currentExecutionCtx); + ExecutionContext.RestoreChangedContextToThread(thread, previousExecCtx, currentExecCtx); + } + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void CaptureContexts(out Thread currentThread, out ExecutionContext? execCtx, out SynchronizationContext? syncCtx) + { + Thread thread = Thread.CurrentThreadAssumedInitialized; + currentThread = thread; + execCtx = thread._executionContext; + syncCtx = thread._synchronizationContext; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void RestoreExecutionContextWithThread(Thread thread, ExecutionContext? previousExecCtx) + { + ExecutionContext? currentExecCtx = thread._executionContext; + if (previousExecCtx != currentExecCtx) + { + ExecutionContext.RestoreChangedContextToThread(thread, previousExecCtx, currentExecCtx); } } diff --git a/src/coreclr/clrdefinitions.cmake b/src/coreclr/clrdefinitions.cmake index 18b6c457977877..010a1ad1ed74d9 100644 --- a/src/coreclr/clrdefinitions.cmake +++ b/src/coreclr/clrdefinitions.cmake @@ -152,7 +152,7 @@ if(FEATURE_OBJCMARSHAL) add_compile_definitions(FEATURE_OBJCMARSHAL) endif() -# add_compile_definitions(FEATURE_RUNTIME_ASYNC) +add_compile_definitions(FEATURE_RUNTIME_ASYNC) add_compile_definitions($<${FEATURE_JAVAMARSHAL}:FEATURE_JAVAMARSHAL>) diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 313d67a11ec6ee..ba7129a82c8fc2 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -1747,6 +1747,8 @@ struct CORINFO_ASYNC_INFO // Method handle for AsyncHelpers.RestoreExecutionContext CORINFO_METHOD_HANDLE restoreExecutionContextMethHnd; CORINFO_METHOD_HANDLE captureContinuationContextMethHnd; + CORINFO_METHOD_HANDLE captureContextsMethHnd; + CORINFO_METHOD_HANDLE restoreExecutionContextWithThreadMethHnd; }; // Flags passed from JIT to runtime. diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index 7a26ab4b004856..a21763fc264cc7 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -67,115 +67,133 @@ PhaseStatus Compiler::SaveAsyncContexts() PhaseStatus result = PhaseStatus::MODIFIED_NOTHING; - BasicBlock* curBB = fgFirstBB; - while (curBB != nullptr) - { - BasicBlock* nextBB = curBB->Next(); - - for (Statement* stmt : curBB->Statements()) - { - GenTree* tree = stmt->GetRootNode(); - if (tree->OperIs(GT_STORE_LCL_VAR)) - { - tree = tree->AsLclVarCommon()->Data(); - } - - if (!tree->IsCall() || !tree->AsCall()->IsAsyncAndAlwaysSavesAndRestoresExecutionContext()) - { - ValidateNoAsyncSavesNecessaryInStatement(stmt); - continue; - } - - GenTreeCall* call = tree->AsCall(); - - unsigned lclNum = lvaGrabTemp(false DEBUGARG("ExecutionContext for SaveAndRestore async call")); - - JITDUMP("Saving ExecutionContext in V%02u around [%06u]\n", lclNum, call->gtTreeID); - - CORINFO_ASYNC_INFO* asyncInfo = eeGetAsyncInfo(); - - GenTreeCall* capture = gtNewCallNode(CT_USER_FUNC, asyncInfo->captureExecutionContextMethHnd, TYP_REF); - CORINFO_CALL_INFO callInfo = {}; - callInfo.hMethod = capture->gtCallMethHnd; - callInfo.methodFlags = info.compCompHnd->getMethodAttribs(callInfo.hMethod); - impMarkInlineCandidate(capture, MAKE_METHODCONTEXT(callInfo.hMethod), false, &callInfo, compInlineContext); - - if (capture->IsInlineCandidate()) - { - Statement* captureStmt = fgNewStmtFromTree(capture); - - GenTreeRetExpr* retExpr = gtNewInlineCandidateReturnExpr(capture, TYP_REF); - - capture->GetSingleInlineCandidateInfo()->retExpr = retExpr; - GenTree* storeCapture = gtNewTempStore(lclNum, retExpr); - Statement* storeCaptureStmt = fgNewStmtFromTree(storeCapture); - - fgInsertStmtBefore(curBB, stmt, captureStmt); - fgInsertStmtBefore(curBB, stmt, storeCaptureStmt); - - JITDUMP("Inserted capture:\n"); - DISPSTMT(captureStmt); - DISPSTMT(storeCaptureStmt); - } - else - { - GenTree* storeCapture = gtNewTempStore(lclNum, capture); - Statement* storeCaptureStmt = fgNewStmtFromTree(storeCapture); - - fgInsertStmtBefore(curBB, stmt, storeCaptureStmt); - - JITDUMP("Inserted capture:\n"); - DISPSTMT(storeCaptureStmt); - } - - BasicBlock* restoreBB = curBB; - Statement* restoreAfterStmt = stmt; - - if (call->IsInlineCandidate() && (call->gtReturnType != TYP_VOID)) - { - restoreAfterStmt = stmt->GetNextStmt(); - assert(restoreAfterStmt->GetRootNode()->OperIs(GT_RET_EXPR) || - (restoreAfterStmt->GetRootNode()->OperIs(GT_STORE_LCL_VAR) && - restoreAfterStmt->GetRootNode()->AsLclVarCommon()->Data()->OperIs(GT_RET_EXPR))); - } - - if (curBB->hasTryIndex()) - { -#ifdef FEATURE_EH_WINDOWS_X86 - IMPL_LIMITATION("Cannot handle insertion of try-finally without funclets"); -#else - // Await is inside a try, need to insert try-finally around it. - restoreBB = InsertTryFinallyForContextRestore(curBB, stmt, restoreAfterStmt); - restoreAfterStmt = nullptr; -#endif - } - - GenTreeCall* restore = gtNewCallNode(CT_USER_FUNC, asyncInfo->restoreExecutionContextMethHnd, TYP_VOID); - restore->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclVarNode(lclNum))); - - callInfo = {}; - callInfo.hMethod = restore->gtCallMethHnd; - callInfo.methodFlags = info.compCompHnd->getMethodAttribs(callInfo.hMethod); - impMarkInlineCandidate(restore, MAKE_METHODCONTEXT(callInfo.hMethod), false, &callInfo, compInlineContext); - - Statement* restoreStmt = fgNewStmtFromTree(restore); - if (restoreAfterStmt == nullptr) - { - fgInsertStmtNearEnd(restoreBB, restoreStmt); - } - else - { - fgInsertStmtAfter(restoreBB, restoreAfterStmt, restoreStmt); - } - - JITDUMP("Inserted restore:\n"); - DISPSTMT(restoreStmt); - - result = PhaseStatus::MODIFIED_EVERYTHING; - } - - curBB = nextBB; - } +// BasicBlock* curBB = fgFirstBB; +// while (curBB != nullptr) +// { +// BasicBlock* nextBB = curBB->Next(); +// +// for (Statement* stmt : curBB->Statements()) +// { +// GenTree* tree = stmt->GetRootNode(); +// if (tree->OperIs(GT_STORE_LCL_VAR)) +// { +// tree = tree->AsLclVarCommon()->Data(); +// } +// +// if (!tree->IsCall()) +// { +// ValidateNoAsyncSavesNecessaryInStatement(stmt); +// continue; +// } +// +// GenTreeCall* call = tree->AsCall(); +// const AsyncCallInfo& asyncCallInfo = call->GetAsyncInfo(); +// +// // Currently we always expect that ExecutionContenxt and +// // SynchronizationContext to correlate about their save/restore +// // behavior. +// assert((asyncCallInfo.ExecutionContextHandling == ExecutionContextHandling::SaveAndRestore) == asyncCallInfo.SaveAndRestoreSynchronizationContextField); +// +// if (asyncCallInfo.ExecutionContextHandling != ExecutionContextHandling::SaveAndRestore) +// { +// continue; +// } +// +// unsigned callSuspendedIndicatorLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Suspended indicator for [%06u]", dspTreeID(call)))); +// lvaGetDesc(callSuspendedIndicatorLclNum)->lvType = TYP_INT; +// +// call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclAddrNode(callSuspendedIndicatorLclNum, 0))); +// +// unsigned threadLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Thread for [%06u]", dspTreeID(call)))); +// unsigned execCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("ExecutionContext for [%06u]", dspTreeID(call)))); +// unsigned syncCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("SynchronizationContext for [%06u]", dspTreeID(call)))); +// +// JITDUMP("Saving contexts around [%06u], thread = V%02u, ExecutionContext = V%02u, SynchronizationContext = V%02u\n", call->gtTreeID, threadLclNum, execCtxLclNum, syncCtxLclNum); +// +// CORINFO_ASYNC_INFO* asyncInfo = eeGetAsyncInfo(); +// +// GenTreeCall* capture = gtNewCallNode(CT_USER_FUNC, asyncInfo->captureExecutionContextMethHnd, TYP_REF); +// CORINFO_CALL_INFO callInfo = {}; +// callInfo.hMethod = capture->gtCallMethHnd; +// callInfo.methodFlags = info.compCompHnd->getMethodAttribs(callInfo.hMethod); +// impMarkInlineCandidate(capture, MAKE_METHODCONTEXT(callInfo.hMethod), false, &callInfo, compInlineContext); +// +// if (capture->IsInlineCandidate()) +// { +// Statement* captureStmt = fgNewStmtFromTree(capture); +// +// GenTreeRetExpr* retExpr = gtNewInlineCandidateReturnExpr(capture, TYP_REF); +// +// capture->GetSingleInlineCandidateInfo()->retExpr = retExpr; +// GenTree* storeCapture = gtNewTempStore(lclNum, retExpr); +// Statement* storeCaptureStmt = fgNewStmtFromTree(storeCapture); +// +// fgInsertStmtBefore(curBB, stmt, captureStmt); +// fgInsertStmtBefore(curBB, stmt, storeCaptureStmt); +// +// JITDUMP("Inserted capture:\n"); +// DISPSTMT(captureStmt); +// DISPSTMT(storeCaptureStmt); +// } +// else +// { +// GenTree* storeCapture = gtNewTempStore(lclNum, capture); +// Statement* storeCaptureStmt = fgNewStmtFromTree(storeCapture); +// +// fgInsertStmtBefore(curBB, stmt, storeCaptureStmt); +// +// JITDUMP("Inserted capture:\n"); +// DISPSTMT(storeCaptureStmt); +// } +// +// BasicBlock* restoreBB = curBB; +// Statement* restoreAfterStmt = stmt; +// +// if (call->IsInlineCandidate() && (call->gtReturnType != TYP_VOID)) +// { +// restoreAfterStmt = stmt->GetNextStmt(); +// assert(restoreAfterStmt->GetRootNode()->OperIs(GT_RET_EXPR) || +// (restoreAfterStmt->GetRootNode()->OperIs(GT_STORE_LCL_VAR) && +// restoreAfterStmt->GetRootNode()->AsLclVarCommon()->Data()->OperIs(GT_RET_EXPR))); +// } +// +// if (curBB->hasTryIndex()) +// { +//#ifdef FEATURE_EH_WINDOWS_X86 +// IMPL_LIMITATION("Cannot handle insertion of try-finally without funclets"); +//#else +// // Await is inside a try, need to insert try-finally around it. +// restoreBB = InsertTryFinallyForContextRestore(curBB, stmt, restoreAfterStmt); +// restoreAfterStmt = nullptr; +//#endif +// } +// +// GenTreeCall* restore = gtNewCallNode(CT_USER_FUNC, asyncInfo->restoreExecutionContextMethHnd, TYP_VOID); +// restore->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclVarNode(lclNum))); +// +// callInfo = {}; +// callInfo.hMethod = restore->gtCallMethHnd; +// callInfo.methodFlags = info.compCompHnd->getMethodAttribs(callInfo.hMethod); +// impMarkInlineCandidate(restore, MAKE_METHODCONTEXT(callInfo.hMethod), false, &callInfo, compInlineContext); +// +// Statement* restoreStmt = fgNewStmtFromTree(restore); +// if (restoreAfterStmt == nullptr) +// { +// fgInsertStmtNearEnd(restoreBB, restoreStmt); +// } +// else +// { +// fgInsertStmtAfter(restoreBB, restoreAfterStmt, restoreStmt); +// } +// +// JITDUMP("Inserted restore:\n"); +// DISPSTMT(restoreStmt); +// +// result = PhaseStatus::MODIFIED_EVERYTHING; +// } +// +// curBB = nextBB; +// } return result; } diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 8ab406b2a52bc1..8a7abe748603ab 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -601,8 +601,7 @@ class LclVarDsc unsigned char lvIsMultiRegDest : 1; // true if this is a multireg LclVar struct that is stored from a multireg node #ifdef DEBUG - unsigned char lvHiddenBufferStructArg : 1; // True when this struct (or its field) are passed as hidden buffer - // pointer. + unsigned char lvDefinedViaAddress : 1; // True when this local may have LCL_ADDRs representing definitions #endif #ifdef FEATURE_HFA_FIELDS_PRESENT @@ -753,14 +752,14 @@ class LclVarDsc } #ifdef DEBUG - void SetHiddenBufferStructArg(char value) + void SetDefinedViaAddress(char value) { - lvHiddenBufferStructArg = value; + lvDefinedViaAddress = value; } - bool IsHiddenBufferStructArg() const + bool IsDefinedViaAddress() const { - return lvHiddenBufferStructArg; + return lvDefinedViaAddress; } #endif @@ -3741,6 +3740,7 @@ class Compiler bool gtIsTypeof(GenTree* tree, CORINFO_CLASS_HANDLE* handle = nullptr); GenTreeLclVarCommon* gtCallGetDefinedRetBufLclAddr(GenTreeCall* call); + GenTreeLclVarCommon* gtCallGetDefinedAsyncSuspensionIndicatorLclAddr(GenTreeCall* call); //------------------------------------------------------------------------- // Functions to display the trees diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 489e5d08c70808..4ce914073f46ef 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -11574,9 +11574,9 @@ void Compiler::gtDispNode(GenTree* tree, IndentStack* indentStack, _In_ _In_opt_ { printf("(AX)"); // Variable has address exposed. } - if (varDsc->IsHiddenBufferStructArg()) + if (varDsc->IsDefinedViaAddress()) { - printf("(RB)"); // Variable is hidden return buffer + printf("(DA)"); // Variable is defined via address } if (varDsc->lvUnusedStruct) { @@ -19657,7 +19657,27 @@ GenTreeLclVarCommon* Compiler::gtCallGetDefinedRetBufLclAddr(GenTreeCall* call) // This may be called very late to check validity of LIR. node = node->gtSkipReloadOrCopy(); - assert(node->OperIs(GT_LCL_ADDR) && lvaGetDesc(node->AsLclVarCommon())->IsHiddenBufferStructArg()); + assert(node->OperIs(GT_LCL_ADDR) && lvaGetDesc(node->AsLclVarCommon())->IsDefinedViaAddress()); + + return node->AsLclVarCommon(); +} + +GenTreeLclVarCommon* Compiler::gtCallGetDefinedAsyncSuspensionIndicatorLclAddr(GenTreeCall* call) +{ + if (!call->IsAsync()) + { + return nullptr; + } + + CallArg* asyncSuspensionIndicatorArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); + if (asyncSuspensionIndicatorArg == nullptr) + { + return nullptr; + } + + GenTree* node = asyncSuspensionIndicatorArg->GetNode(); + + assert(node->OperIs(GT_LCL_ADDR) && lvaGetDesc(node->AsLclVarCommon())->IsDefinedViaAddress()); return node->AsLclVarCommon(); } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 33690a76ef9169..3f1aef913d2770 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4343,6 +4343,8 @@ struct AsyncCallInfo { ExecutionContextHandling ExecutionContextHandling = ExecutionContextHandling::None; ContinuationContextHandling ContinuationContextHandling = ContinuationContextHandling::None; + bool SaveAndRestoreSynchronizationContextField = false; + unsigned SynchronizationContextLclNum = BAD_VAR_NUM; }; // Return type descriptor of a GT_CALL node. @@ -4615,6 +4617,7 @@ enum class WellKnownArg : unsigned X86TailCallSpecialArg, StackArrayLocal, RuntimeMethodHandle, + AsyncSuspendedIndicator, }; #ifdef DEBUG diff --git a/src/coreclr/jit/gschecks.cpp b/src/coreclr/jit/gschecks.cpp index 75d5a365a5871f..4d7a54fcdcc2ff 100644 --- a/src/coreclr/jit/gschecks.cpp +++ b/src/coreclr/jit/gschecks.cpp @@ -410,7 +410,7 @@ void Compiler::gsParamsToShadows() shadowVarDsc->lvDoNotEnregister = varDsc->lvDoNotEnregister; #ifdef DEBUG shadowVarDsc->SetDoNotEnregReason(varDsc->GetDoNotEnregReason()); - shadowVarDsc->SetHiddenBufferStructArg(varDsc->IsHiddenBufferStructArg()); + shadowVarDsc->SetDefinedViaAddress(varDsc->IsDefinedViaAddress()); #endif if (varTypeIsStruct(type)) diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 7366dfe9bce1fd..1b3dcc5bd7d535 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1464,7 +1464,7 @@ class LocalAddressVisitor final : public GenTreeVisitor GenTreeFlags defFlag = GTF_EMPTY; GenTreeCall* callUser = (user != nullptr) && user->IsCall() ? user->AsCall() : nullptr; - bool hasHiddenStructArg = false; + bool escapeAddr = true; if (m_compiler->opts.compJitOptimizeStructHiddenBuffer && (callUser != nullptr) && m_compiler->IsValidLclAddr(lclNum, val.Offset())) { @@ -1484,7 +1484,7 @@ class LocalAddressVisitor final : public GenTreeVisitor (val.Node() == callUser->gtArgs.GetRetBufferArg()->GetNode())) { m_compiler->lvaSetHiddenBufferStructArg(lclNum); - hasHiddenStructArg = true; + escapeAddr = false; callUser->gtCallMoreFlags |= GTF_CALL_M_RETBUFFARG_LCLOPT; defFlag = GTF_VAR_DEF; @@ -1496,7 +1496,12 @@ class LocalAddressVisitor final : public GenTreeVisitor } } - if (!hasHiddenStructArg) + if ((callUser != nullptr) && m_compiler->IsValidLclAddr(lclNum, val.Offset())) + { + + } + + if (escapeAddr) { unsigned exposedLclNum = varDsc->lvIsStructField ? varDsc->lvParentLcl : lclNum; @@ -1618,7 +1623,7 @@ class LocalAddressVisitor final : public GenTreeVisitor assert(addr->TypeIs(TYP_BYREF, TYP_I_IMPL)); assert(m_compiler->lvaVarAddrExposed(lclNum) || ((m_lclAddrAssertions != nullptr) && m_lclAddrAssertions->IsMarkedForExposure(lclNum)) || - m_compiler->lvaGetDesc(lclNum)->IsHiddenBufferStructArg()); + m_compiler->lvaGetDesc(lclNum)->IsDefinedViaAddress()); if (m_compiler->IsValidLclAddr(lclNum, offset)) { diff --git a/src/coreclr/jit/lclvars.cpp b/src/coreclr/jit/lclvars.cpp index 07e584dbe441eb..f96943f2069bbe 100644 --- a/src/coreclr/jit/lclvars.cpp +++ b/src/coreclr/jit/lclvars.cpp @@ -2089,7 +2089,7 @@ void Compiler::lvaSetHiddenBufferStructArg(unsigned varNum) LclVarDsc* varDsc = lvaGetDesc(varNum); #ifdef DEBUG - varDsc->SetHiddenBufferStructArg(true); + varDsc->SetDefinedViaAddress(true); #endif if (varDsc->lvPromoted) @@ -2100,7 +2100,7 @@ void Compiler::lvaSetHiddenBufferStructArg(unsigned varNum) { noway_assert(lvaTable[i].lvIsStructField); #ifdef DEBUG - lvaTable[i].SetHiddenBufferStructArg(true); + lvaTable[i].SetDefinedViaAddress(true); #endif lvaSetVarDoNotEnregister(i DEBUGARG(DoNotEnregisterReason::HiddenBufferStructArg)); @@ -3435,7 +3435,7 @@ void Compiler::lvaMarkLclRefs(GenTree* tree, BasicBlock* block, Statement* stmt, if (tree->OperIs(GT_LCL_ADDR)) { LclVarDsc* varDsc = lvaGetDesc(tree->AsLclVarCommon()); - assert(varDsc->IsAddressExposed() || varDsc->IsHiddenBufferStructArg()); + assert(varDsc->IsAddressExposed() || varDsc->IsDefinedViaAddress()); varDsc->incRefCnts(weight, this); return; } @@ -6365,9 +6365,9 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r { printf("X"); } - if (varDsc->IsHiddenBufferStructArg()) + if (varDsc->IsDefinedViaAddress()) { - printf("H"); + printf("DA"); } if (varTypeIsStruct(varDsc)) { @@ -6428,9 +6428,9 @@ void Compiler::lvaDumpEntry(unsigned lclNum, FrameLayoutState curState, size_t r { printf(" addr-exposed"); } - if (varDsc->IsHiddenBufferStructArg()) + if (varDsc->IsDefinedViaAddress()) { - printf(" hidden-struct-arg"); + printf(" defined-via-address"); } if (varDsc->lvHasLdAddrOp) { diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 688787a6761674..ae5213652ccac0 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -1919,6 +1919,14 @@ void CallArgs::DetermineABIInfo(Compiler* comp, GenTreeCall* call) for (CallArg& arg : Args()) { + if (arg.GetWellKnownArg() == WellKnownArg::AsyncSuspendedIndicator) + { + // Represents a definition of a local indicating whether the call + // suspended or not. Removed by async transformation. + arg.AbiInfo = ABIPassingInformation(comp, 0); + continue; + } + const var_types argSigType = arg.GetSignatureType(); const CORINFO_CLASS_HANDLE argSigClass = arg.GetSignatureClassHandle(); ClassLayout* argLayout = argSigClass == NO_CLASS_HANDLE ? nullptr : comp->typGetObjLayout(argSigClass); diff --git a/src/coreclr/jit/promotion.cpp b/src/coreclr/jit/promotion.cpp index b4e2e284d051e4..8a7d53f27e172b 100644 --- a/src/coreclr/jit/promotion.cpp +++ b/src/coreclr/jit/promotion.cpp @@ -1126,7 +1126,7 @@ class LocalsUseVisitor : public GenTreeVisitor if (lcl->OperIs(GT_LCL_ADDR)) { - assert(user->OperIs(GT_CALL) && dsc->IsHiddenBufferStructArg() && + assert(user->OperIs(GT_CALL) && dsc->IsDefinedViaAddress() && (user->AsCall()->gtArgs.GetRetBufferArg()->GetNode() == lcl)); accessType = TYP_STRUCT; diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 19a34dba7f6bf6..e904e0edc21792 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -12355,7 +12355,7 @@ void Compiler::fgValueNumberTree(GenTree* tree) unsigned lclOffs = tree->AsLclFld()->GetLclOffs(); tree->gtVNPair.SetBoth(vnStore->VNForFunc(TYP_BYREF, VNF_PtrToLoc, vnStore->VNForIntCon(lclNum), vnStore->VNForIntPtrCon(lclOffs))); - assert(lvaGetDesc(lclNum)->IsAddressExposed() || lvaGetDesc(lclNum)->IsHiddenBufferStructArg()); + assert(lvaGetDesc(lclNum)->IsAddressExposed() || lvaGetDesc(lclNum)->IsDefinedViaAddress()); } break; diff --git a/src/tests/async/Directory.Build.targets b/src/tests/async/Directory.Build.targets deleted file mode 100644 index 5a4d413a9e1f62..00000000000000 --- a/src/tests/async/Directory.Build.targets +++ /dev/null @@ -1,13 +0,0 @@ - - - - true - - - - - true - - - - diff --git a/src/tests/async/synchronization-context/synchronization-context.csproj b/src/tests/async/synchronization-context/synchronization-context.csproj index 1ae294349c376f..1c9f61351ff305 100644 --- a/src/tests/async/synchronization-context/synchronization-context.csproj +++ b/src/tests/async/synchronization-context/synchronization-context.csproj @@ -1,6 +1,7 @@ True + portable From 3b85c952d8481b4f1e26185c8024f376f2b92083 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 15 Jul 2025 15:57:24 +0200 Subject: [PATCH 02/28] JIT: Preparatory refactoring to support multiple defs per single node Replace `GenTree::DefinesLocal` by a `GenTree::VisitLocalDefs` to prepare for being able to allow a single node to have multiple definitions. Update uses to use the new function. --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/compiler.hpp | 64 ++++++++++++ src/coreclr/jit/copyprop.cpp | 35 ++++--- src/coreclr/jit/fgdiagnostic.cpp | 83 +++++++-------- src/coreclr/jit/flowgraph.cpp | 10 +- src/coreclr/jit/gentree.cpp | 99 ------------------ src/coreclr/jit/gentree.h | 37 +++++-- src/coreclr/jit/liveness.cpp | 44 ++++---- src/coreclr/jit/lower.cpp | 13 +-- src/coreclr/jit/morph.cpp | 25 +++-- src/coreclr/jit/ssabuilder.cpp | 157 ++++++++++++++++------------ src/coreclr/jit/ssabuilder.h | 1 + src/coreclr/jit/treelifeupdater.cpp | 16 ++- src/coreclr/jit/valuenum.cpp | 19 ++-- 14 files changed, 313 insertions(+), 292 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 8ab406b2a52bc1..80670b20835cac 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7362,7 +7362,7 @@ class Compiler LclNumToLiveDefsMap* curSsaName); void optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* curSsaName); bool optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaName); - void optCopyPropPushDef(GenTree* defNode, GenTreeLclVarCommon* lclNode, LclNumToLiveDefsMap* curSsaName); + void optCopyPropPushDef(GenTreeLclVarCommon* lclNode, LclNumToLiveDefsMap* curSsaName); int optCopyProp_LclVarScore(const LclVarDsc* lclVarDsc, const LclVarDsc* copyVarDsc, bool preferOp2); PhaseStatus optVnCopyProp(); INDEBUG(void optDumpCopyPropStack(LclNumToLiveDefsMap* curSsaName)); diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 0f9b1de014ad49..172c65d306b7c2 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4703,6 +4703,70 @@ GenTree::VisitResult GenTree::VisitOperands(TVisitor visitor) } } +//------------------------------------------------------------------------ +// VisitLocalDefs: Visit locals being defined by this node. +// +// Arguments: +// comp - the compiler instance +// visitor - Functor of type GenTree::VisitResult(LocalDef) +// +// Return Value: +// VisitResult::Abort if the functor aborted; otherwise VisitResult::Continue. +// +// Notes: +// This function is contractually bound to recognize a superset of stores +// that "LocalAddressVisitor" recognizes and transforms, as it is used to +// detect which trees can define tracked locals. +// +template +GenTree::VisitResult GenTree::VisitLocalDefs(Compiler* comp, TVisitor visitor) +{ + if (OperIs(GT_STORE_LCL_VAR)) + { + unsigned size = comp->lvaLclExactSize(AsLclVarCommon()->GetLclNum()); + return visitor(LocalDef(AsLclVarCommon(), /* isEntire */ true, 0, size)); + } + if (OperIs(GT_STORE_LCL_FLD)) + { + GenTreeLclFld* fld = AsLclFld(); + return visitor(LocalDef(fld, !fld->IsPartialLclFld(comp), fld->GetLclOffs(), fld->GetSize())); + } + if (OperIs(GT_CALL)) + { + GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(AsCall()); + if (lclAddr != nullptr) + { + unsigned storeSize = comp->typGetObjLayout(AsCall()->gtRetClsHnd)->GetSize(); + + bool isEntire = storeSize == comp->lvaLclExactSize(lclAddr->GetLclNum()); + + if (visitor(LocalDef(lclAddr, isEntire, lclAddr->GetLclOffs(), storeSize)) == VisitResult::Abort) + { + return VisitResult::Abort; + } + } + } + + return VisitResult::Continue; +} + +//------------------------------------------------------------------------ +// HasAnyLocalDefs: +// Check if a tree is considered as defining any locals. +// +// Arguments: +// comp - the compiler instance +// +// Return Value: +// True if it is. +// +inline bool GenTree::HasAnyLocalDefs(Compiler* comp) +{ + return VisitLocalDefs(comp, [](const LocalDef& def) { + return GenTree::VisitResult::Abort; + }) == GenTree::VisitResult::Abort; +} + /***************************************************************************** * operator new * diff --git a/src/coreclr/jit/copyprop.cpp b/src/coreclr/jit/copyprop.cpp index 93ff8bd2676127..9e50047aa02094 100644 --- a/src/coreclr/jit/copyprop.cpp +++ b/src/coreclr/jit/copyprop.cpp @@ -46,24 +46,31 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* { for (GenTree* const tree : stmt->TreeList()) { - GenTreeLclVarCommon* lclDefNode = nullptr; - if (tree->OperIsSsaDef() && tree->DefinesLocal(this, &lclDefNode)) + if (!tree->OperIsSsaDef()) { - if (lclDefNode->HasCompositeSsaName()) + continue; + } + + auto visitDef = [=](const LocalDef& def) { + if (def.Def->HasCompositeSsaName()) { - LclVarDsc* varDsc = lvaGetDesc(lclDefNode); + LclVarDsc* varDsc = lvaGetDesc(def.Def); assert(varDsc->lvPromoted); for (unsigned index = 0; index < varDsc->lvFieldCnt; index++) { - popDef(varDsc->lvFieldLclStart + index, lclDefNode->GetSsaNum(this, index)); + popDef(varDsc->lvFieldLclStart + index, def.Def->GetSsaNum(this, index)); } } else { - popDef(lclDefNode->GetLclNum(), lclDefNode->GetSsaNum()); + popDef(def.Def->GetLclNum(), def.Def->GetSsaNum()); } - } + + return GenTree::VisitResult::Continue; + }; + + tree->VisitLocalDefs(this, visitDef); } } } @@ -304,11 +311,10 @@ bool Compiler::optCopyProp( // optCopyPropPushDef: Push the new live SSA def on the stack for "lclNode". // // Arguments: -// defNode - The definition node for this def (store/GT_CALL) (will be "nullptr" for "use" defs) // lclNode - The local tree representing "the def" // curSsaName - The map of local numbers to stacks of their defs // -void Compiler::optCopyPropPushDef(GenTree* defNode, GenTreeLclVarCommon* lclNode, LclNumToLiveDefsMap* curSsaName) +void Compiler::optCopyPropPushDef(GenTreeLclVarCommon* lclNode, LclNumToLiveDefsMap* curSsaName) { unsigned lclNum = lclNode->GetLclNum(); @@ -401,9 +407,14 @@ bool Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaNa treeLifeUpdater.UpdateLife(tree); GenTreeLclVarCommon* lclDefNode = nullptr; - if (tree->OperIsSsaDef() && tree->DefinesLocal(this, &lclDefNode)) + if (tree->OperIsSsaDef()) { - optCopyPropPushDef(tree, lclDefNode, curSsaName); + auto visitDef = [=](const LocalDef& def) { + optCopyPropPushDef(def.Def, curSsaName); + return GenTree::VisitResult::Continue; + }; + + tree->VisitLocalDefs(this, visitDef); } else if (tree->OperIs(GT_LCL_VAR, GT_LCL_FLD) && tree->AsLclVarCommon()->HasSsaName()) { @@ -413,7 +424,7 @@ bool Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaNa // live definition. Since they are always live, we'll do it only once. if ((lvaGetDesc(lclNum)->lvIsParam || (lclNum == info.compThisArg)) && !curSsaName->Lookup(lclNum)) { - optCopyPropPushDef(nullptr, tree->AsLclVarCommon(), curSsaName); + optCopyPropPushDef(tree->AsLclVarCommon(), curSsaName); } // TODO-Review: EH successor/predecessor iteration seems broken. diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 50145f04b32b81..29f3b356c0ca2e 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -4326,61 +4326,58 @@ class SsaCheckVisitor : public GenTreeVisitor void ProcessDefs(GenTree* tree) { - GenTreeLclVarCommon* lclNode; - bool isFullDef = false; - ssize_t offset = 0; - unsigned storeSize = 0; - bool definesLocal = tree->DefinesLocal(m_compiler, &lclNode, &isFullDef, &offset, &storeSize); - - if (!definesLocal) - { - return; - } - - const bool isUse = (lclNode->gtFlags & GTF_VAR_USEASG) != 0; - unsigned const lclNum = lclNode->GetLclNum(); - LclVarDsc* const varDsc = m_compiler->lvaGetDesc(lclNum); + auto visitDef = [=](const LocalDef& def) { + const bool isUse = (def.Def->gtFlags & GTF_VAR_USEASG) != 0; + unsigned const lclNum = def.Def->GetLclNum(); + LclVarDsc* const varDsc = m_compiler->lvaGetDesc(lclNum); - assert(!(isFullDef && isUse)); + assert(!(def.IsEntire && isUse)); - if (lclNode->HasCompositeSsaName()) - { - for (unsigned index = 0; index < varDsc->lvFieldCnt; index++) + if (def.Def->HasCompositeSsaName()) { - unsigned const fieldLclNum = varDsc->lvFieldLclStart + index; - LclVarDsc* const fieldVarDsc = m_compiler->lvaGetDesc(fieldLclNum); - unsigned const fieldSsaNum = lclNode->GetSsaNum(m_compiler, index); - - ssize_t fieldStoreOffset; - unsigned fieldStoreSize; - if (m_compiler->gtStoreDefinesField(fieldVarDsc, offset, storeSize, &fieldStoreOffset, &fieldStoreSize)) + for (unsigned index = 0; index < varDsc->lvFieldCnt; index++) { - ProcessDef(lclNode, fieldLclNum, fieldSsaNum); - - if (!ValueNumStore::LoadStoreIsEntire(genTypeSize(fieldVarDsc), fieldStoreOffset, fieldStoreSize)) + unsigned const fieldLclNum = varDsc->lvFieldLclStart + index; + LclVarDsc* const fieldVarDsc = m_compiler->lvaGetDesc(fieldLclNum); + unsigned const fieldSsaNum = def.Def->GetSsaNum(m_compiler, index); + + ssize_t fieldStoreOffset; + unsigned fieldStoreSize; + if (m_compiler->gtStoreDefinesField(fieldVarDsc, def.Offset, def.Size, &fieldStoreOffset, + &fieldStoreSize)) { - assert(isUse); - unsigned const fieldUseSsaNum = fieldVarDsc->GetPerSsaData(fieldSsaNum)->GetUseDefSsaNum(); - ProcessUse(lclNode, fieldLclNum, fieldUseSsaNum); + ProcessDef(def.Def, fieldLclNum, fieldSsaNum); + + if (!ValueNumStore::LoadStoreIsEntire(genTypeSize(fieldVarDsc), fieldStoreOffset, + fieldStoreSize)) + { + assert(isUse); + unsigned const fieldUseSsaNum = fieldVarDsc->GetPerSsaData(fieldSsaNum)->GetUseDefSsaNum(); + ProcessUse(def.Def, fieldLclNum, fieldUseSsaNum); + } } } } - } - else - { - unsigned const ssaNum = lclNode->GetSsaNum(); - ProcessDef(lclNode, lclNum, ssaNum); - - if (isUse) + else { - unsigned useSsaNum = SsaConfig::RESERVED_SSA_NUM; - if (ssaNum != SsaConfig::RESERVED_SSA_NUM) + unsigned const ssaNum = def.Def->GetSsaNum(); + ProcessDef(def.Def, lclNum, ssaNum); + + if (isUse) { - useSsaNum = varDsc->GetPerSsaData(ssaNum)->GetUseDefSsaNum(); + unsigned useSsaNum = SsaConfig::RESERVED_SSA_NUM; + if (ssaNum != SsaConfig::RESERVED_SSA_NUM) + { + useSsaNum = varDsc->GetPerSsaData(ssaNum)->GetUseDefSsaNum(); + } + ProcessUse(def.Def, lclNum, useSsaNum); } - ProcessUse(lclNode, lclNum, useSsaNum); } - } + + return GenTree::VisitResult::Continue; + }; + + tree->VisitLocalDefs(m_compiler, visitDef); } void ProcessUse(GenTreeLclVarCommon* tree, unsigned lclNum, unsigned ssaNum) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 4db0f7b1ccbce1..92a84b0995b647 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -5391,11 +5391,13 @@ bool FlowGraphNaturalLoop::VisitDefs(TFunc func) return Compiler::WALK_SKIP_SUBTREES; } - GenTreeLclVarCommon* lclDef; - if (tree->DefinesLocal(m_compiler, &lclDef)) + auto visitDef = [=](const LocalDef& def) { + return m_func(def.Def) ? GenTree::VisitResult::Continue : GenTree::VisitResult::Abort; + }; + + if (tree->VisitLocalDefs(m_compiler, visitDef) == GenTree::VisitResult::Abort) { - if (!m_func(lclDef)) - return Compiler::WALK_ABORT; + return Compiler::WALK_ABORT; } return Compiler::WALK_CONTINUE; diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 489e5d08c70808..d78d867e99c04a 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -17895,105 +17895,6 @@ bool GenTree::IsPartialLclFld(Compiler* comp) (comp->lvaGetDesc(AsLclFld())->lvExactSize() != AsLclFld()->GetSize()); } -//------------------------------------------------------------------------ -// DefinesLocal: Does "this" define a local? -// -// Arguments: -// comp - the compiler instance -// pLclVarTree - [out] parameter for the local representing the definition -// pIsEntire - optional [out] parameter for whether the store represents -// a "full" definition (overwrites the entire variable) -// pOffset - optional [out] parameter for the offset, relative to the -// local, at which the store is performed -// pSize - optional [out] parameter for the amount of bytes affected -// by the store -// -// Return Value: -// Whether "this" represents a store to a possibly tracked local variable -// before rationalization. -// -// Notes: -// This function is contractually bound to recognize a superset of stores -// that "LocalAddressVisitor" recognizes and transforms, as it is used to -// detect which trees can define tracked locals. -// -bool GenTree::DefinesLocal( - Compiler* comp, GenTreeLclVarCommon** pLclVarTree, bool* pIsEntire, ssize_t* pOffset, unsigned* pSize) -{ - assert((pOffset == nullptr) || (*pOffset == 0)); - - if (OperIs(GT_STORE_LCL_VAR)) - { - *pLclVarTree = AsLclVarCommon(); - if (pIsEntire != nullptr) - { - *pIsEntire = true; - } - if (pOffset != nullptr) - { - *pOffset = 0; - } - if (pSize != nullptr) - { - *pSize = comp->lvaLclExactSize(AsLclVarCommon()->GetLclNum()); - } - - return true; - } - if (OperIs(GT_STORE_LCL_FLD)) - { - *pLclVarTree = AsLclVarCommon(); - if (pIsEntire != nullptr) - { - *pIsEntire = !AsLclFld()->IsPartialLclFld(comp); - } - if (pOffset != nullptr) - { - *pOffset = AsLclFld()->GetLclOffs(); - } - if (pSize != nullptr) - { - *pSize = AsLclFld()->GetSize(); - } - - return true; - } - if (OperIs(GT_CALL)) - { - GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(AsCall()); - if (lclAddr == nullptr) - { - return false; - } - - *pLclVarTree = lclAddr; - - if ((pIsEntire != nullptr) || (pSize != nullptr)) - { - unsigned storeSize = comp->typGetObjLayout(AsCall()->gtRetClsHnd)->GetSize(); - - if (pIsEntire != nullptr) - { - *pIsEntire = storeSize == comp->lvaLclExactSize(lclAddr->GetLclNum()); - } - - if (pSize != nullptr) - { - *pSize = storeSize; - } - } - - if (pOffset != nullptr) - { - *pOffset = lclAddr->GetLclOffs(); - } - - return true; - } - - return false; -} - //------------------------------------------------------------------------ // IsImplicitByrefParameterValuePreMorph: // Determine if this tree represents the value of an implicit byref diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 33690a76ef9169..8ea0d2651cfc40 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -648,6 +648,22 @@ inline GenTreeDebugFlags& operator &=(GenTreeDebugFlags& a, GenTreeDebugFlags b) // clang-format on +struct LocalDef +{ + GenTreeLclVarCommon* Def; + bool IsEntire; + ssize_t Offset; + unsigned Size; + + LocalDef(GenTreeLclVarCommon* def, bool isEntire, ssize_t offset, unsigned size) + : Def(def) + , IsEntire(isEntire) + , Offset(offset) + , Size(size) + { + } +}; + #ifndef HOST_64BIT #include #endif @@ -696,6 +712,12 @@ struct GenTree #include "gtstructs.h" + enum class VisitResult + { + Abort = false, + Continue = true + }; + genTreeOps gtOper; // enum subtype BYTE var_types gtType; // enum subtype BYTE @@ -2023,11 +2045,10 @@ struct GenTree // is not the same size as the type of the GT_LCL_VAR. bool IsPartialLclFld(Compiler* comp); - bool DefinesLocal(Compiler* comp, - GenTreeLclVarCommon** pLclVarTree, - bool* pIsEntire = nullptr, - ssize_t* pOffset = nullptr, - unsigned* pSize = nullptr); + template + VisitResult VisitLocalDefs(Compiler* comp, TVisitor visitor); + + bool HasAnyLocalDefs(Compiler* comp); GenTreeLclVarCommon* IsImplicitByrefParameterValuePreMorph(Compiler* compiler); GenTreeLclVar* IsImplicitByrefParameterValuePostMorph(Compiler* compiler, GenTree** addr, target_ssize_t* offset); @@ -2350,12 +2371,6 @@ struct GenTree // Returns a range that will produce the operands of this node in execution order. IteratorPair Operands(); - enum class VisitResult - { - Abort = false, - Continue = true - }; - // Visits each operand of this node. The operand must be either a lambda, function, or functor with the signature // `GenTree::VisitResult VisitorFunction(GenTree* operand)`. Here is a simple example: // diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index d51746731b6c8e..ae6294af115eae 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -342,11 +342,11 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) } } - GenTreeLclVarCommon* definedLcl = gtCallGetDefinedRetBufLclAddr(call); - if (definedLcl != nullptr) - { - fgMarkUseDef(definedLcl); - } + auto visitDef = [=](const LocalDef& def) { + fgMarkUseDef(def.Def); + return GenTree::VisitResult::Continue; + }; + call->VisitLocalDefs(this, visitDef); break; } @@ -796,13 +796,11 @@ void Compiler::fgLiveVarAnalysis() // call - The call node in question. // // Returns: -// local defined by the call, if any (eg retbuf) +// partially defined local by the call, if any (eg retbuf) // GenTreeLclVarCommon* Compiler::fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_TP keepAliveVars, GenTreeCall* call) { assert(call != nullptr); - GenTreeLclVarCommon* definedLcl = nullptr; - // If this is a tail-call via helper, and we have any unmanaged p/invoke calls in // the method, then we're going to run the p/invoke epilog // So we mark the FrameRoot as used by this instruction. @@ -861,13 +859,22 @@ GenTreeLclVarCommon* Compiler::fgComputeLifeCall(VARSET_TP& life, VARSET_VALARG_ } } - definedLcl = gtCallGetDefinedRetBufLclAddr(call); - if (definedLcl != nullptr) - { - fgComputeLifeLocal(life, keepAliveVars, definedLcl); - } + GenTreeLclVarCommon* partialDef = nullptr; + + auto visitDef = [&](const LocalDef& def) { + if (!def.IsEntire) + { + assert(partialDef == nullptr); + partialDef = def.Def; + } + + fgComputeLifeLocal(life, keepAliveVars, def.Def); + return GenTree::VisitResult::Continue; + }; + + call->VisitLocalDefs(this, visitDef); - return definedLcl; + return partialDef; } //------------------------------------------------------------------------ @@ -1207,11 +1214,12 @@ void Compiler::fgComputeLife(VARSET_TP& life, if (tree->IsCall()) { - GenTreeLclVarCommon* const definedLcl = fgComputeLifeCall(life, keepAliveVars, tree->AsCall()); - if (definedLcl != nullptr) + GenTreeLclVarCommon* const partialDef = fgComputeLifeCall(life, keepAliveVars, tree->AsCall()); + if (partialDef != nullptr) { - isUse = (definedLcl->gtFlags & GTF_VAR_USEASG) != 0; - varDsc = lvaGetDesc(definedLcl); + assert((partialDef->gtFlags & GTF_VAR_USEASG) != 0); + isUse = true; + varDsc = lvaGetDesc(partialDef); } } else if (tree->OperIsNonPhiLocal()) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 3b8b03c975c45f..410cd34039fdaf 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -8636,15 +8636,16 @@ void Lowering::FindInducedParameterRegisterLocals() { hasRegisterKill |= node->IsCall(); - GenTreeLclVarCommon* storeLcl; - if (node->DefinesLocal(comp, &storeLcl)) - { - storedToLocals.Emplace(storeLcl->GetLclNum(), true); - continue; - } + auto visitDefs = [&](const LocalDef& def) { + storedToLocals.Emplace(def.Def->GetLclNum(), true); + return GenTree::VisitResult::Continue; + }; + + node->VisitLocalDefs(comp, visitDefs); if (node->OperIs(GT_LCL_ADDR)) { + // Model these as stored to, since we cannot reason about them in the same way. storedToLocals.Emplace(node->AsLclVarCommon()->GetLclNum(), true); continue; } diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 688787a6761674..4802268169946d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -6824,22 +6824,22 @@ GenTree* Compiler::fgMorphLeaf(GenTree* tree) void Compiler::fgAssignSetVarDef(GenTree* tree) { - GenTreeLclVarCommon* lclVarCmnTree; - bool isEntire = false; - if (tree->DefinesLocal(this, &lclVarCmnTree, &isEntire)) - { - if (isEntire) + auto visitDef = [=](const LocalDef& def) { + if (def.IsEntire) { - lclVarCmnTree->gtFlags |= GTF_VAR_DEF; + def.Def->gtFlags |= GTF_VAR_DEF; } else { // We consider partial definitions to be modeled as uses followed by definitions. // This captures the idea that precedings defs are not necessarily made redundant // by this definition. - lclVarCmnTree->gtFlags |= (GTF_VAR_DEF | GTF_VAR_USEASG); + def.Def->gtFlags |= (GTF_VAR_DEF | GTF_VAR_USEASG); } - } + return GenTree::VisitResult::Continue; + }; + + tree->VisitLocalDefs(this, visitDef); } #ifdef FEATURE_SIMD @@ -12636,9 +12636,14 @@ void Compiler::fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone DEBUGARG // Kill active assertions // GenTreeLclVarCommon* lclVarTree = nullptr; - if ((optAssertionCount > 0) && tree->DefinesLocal(this, &lclVarTree)) + if ((optAssertionCount > 0)) { - fgKillDependentAssertions(lclVarTree->GetLclNum() DEBUGARG(tree)); + auto visitDef = [=](const LocalDef& def) { + fgKillDependentAssertions(def.Def->GetLclNum() DEBUGARG(tree)); + return GenTree::VisitResult::Continue; + }; + + tree->VisitLocalDefs(this, visitDef); } // Generate assertions diff --git a/src/coreclr/jit/ssabuilder.cpp b/src/coreclr/jit/ssabuilder.cpp index e7aec078041b29..3cdd3441ac19c1 100644 --- a/src/coreclr/jit/ssabuilder.cpp +++ b/src/coreclr/jit/ssabuilder.cpp @@ -421,25 +421,21 @@ void SsaBuilder::RenameDef(GenTree* defNode, BasicBlock* block) { assert(defNode->OperIsStore() || defNode->OperIs(GT_CALL)); - GenTreeLclVarCommon* lclNode; - bool isFullDef = false; - ssize_t offset = 0; - unsigned storeSize = 0; - bool isLocal = defNode->DefinesLocal(m_pCompiler, &lclNode, &isFullDef, &offset, &storeSize); - - if (isLocal) - { + bool anyDefs = false; + auto visitDef = [&](const LocalDef& def) { + anyDefs = true; // This should have been marked as definition. - assert(((lclNode->gtFlags & GTF_VAR_DEF) != 0) && (((lclNode->gtFlags & GTF_VAR_USEASG) != 0) == !isFullDef)); + assert(((def.Def->gtFlags & GTF_VAR_DEF) != 0) && + (((def.Def->gtFlags & GTF_VAR_USEASG) != 0) == !def.IsEntire)); - unsigned lclNum = lclNode->GetLclNum(); + unsigned lclNum = def.Def->GetLclNum(); LclVarDsc* varDsc = m_pCompiler->lvaGetDesc(lclNum); if (m_pCompiler->lvaInSsa(lclNum)) { - lclNode->SetSsaNum(RenamePushDef(defNode, block, lclNum, isFullDef)); + def.Def->SetSsaNum(RenamePushDef(defNode, block, lclNum, def.IsEntire)); assert(!varDsc->IsAddressExposed()); // Cannot define SSA memory. - return; + return GenTree::VisitResult::Continue; } if (varDsc->lvPromoted) @@ -455,11 +451,11 @@ void SsaBuilder::RenameDef(GenTree* defNode, BasicBlock* block) unsigned ssaNum = SsaConfig::RESERVED_SSA_NUM; // Fast-path the common case of an "entire" store. - if (isFullDef) + if (def.IsEntire) { ssaNum = RenamePushDef(defNode, block, fieldLclNum, /* defIsFull */ true); } - else if (m_pCompiler->gtStoreDefinesField(fieldVarDsc, offset, storeSize, &fieldStoreOffset, + else if (m_pCompiler->gtStoreDefinesField(fieldVarDsc, def.Offset, def.Size, &fieldStoreOffset, &fieldStoreSize)) { ssaNum = RenamePushDef(defNode, block, fieldLclNum, @@ -469,71 +465,34 @@ void SsaBuilder::RenameDef(GenTree* defNode, BasicBlock* block) if (ssaNum != SsaConfig::RESERVED_SSA_NUM) { - lclNode->SetSsaNum(m_pCompiler, index, ssaNum); + def.Def->SetSsaNum(m_pCompiler, index, ssaNum); } } } } - } - else if (defNode->OperIs(GT_CALL)) - { - // If the current def is a call we either know the call is pure or else has arbitrary memory definitions, - // the memory effect of the call is captured by the live out state from the block and doesn't need special - // handling here. If we ever change liveness to more carefully model call effects (from interprecedural - // information) we might need to revisit this. - return; - } - // Figure out if "defNode" may make a new GC heap state (if we care for this block). - if (((block->bbMemoryHavoc & memoryKindSet(GcHeap)) == 0) && m_pCompiler->ehBlockHasExnFlowDsc(block)) - { - bool isAddrExposedLocal = isLocal && m_pCompiler->lvaVarAddrExposed(lclNode->GetLclNum()); - bool hasByrefHavoc = ((block->bbMemoryHavoc & memoryKindSet(ByrefExposed)) != 0); - if (!isLocal || (isAddrExposedLocal && !hasByrefHavoc)) + if (varDsc->IsAddressExposed()) { - // It *may* define byref memory in a non-havoc way. Make a new SSA # -- associate with this node. - unsigned ssaNum = m_pCompiler->lvMemoryPerSsaData.AllocSsaNum(m_allocator); - if (!hasByrefHavoc) - { - m_renameStack.PushMemory(ByrefExposed, block, ssaNum); - m_pCompiler->GetMemorySsaMap(ByrefExposed)->Set(defNode, ssaNum); -#ifdef DEBUG - if (m_pCompiler->verboseSsa) - { - printf("Node "); - Compiler::printTreeID(defNode); - printf(" (in try block) may define memory; ssa # = %d.\n", ssaNum); - } -#endif // DEBUG + RenamePushMemoryDef(def.Def, block); + } - // Now add this SSA # to all phis of the reachable catch blocks. - AddMemoryDefToEHSuccessorPhis(ByrefExposed, block, ssaNum); - } + return GenTree::VisitResult::Continue; + }; - if (!isLocal) - { - // Add a new def for GcHeap as well - if (m_pCompiler->byrefStatesMatchGcHeapStates) - { - // GcHeap and ByrefExposed share the same stacks, SsaMap, and phis - assert(!hasByrefHavoc); - assert(*m_pCompiler->GetMemorySsaMap(GcHeap)->LookupPointer(defNode) == ssaNum); - assert(block->bbMemorySsaPhiFunc[GcHeap] == block->bbMemorySsaPhiFunc[ByrefExposed]); - } - else - { - if (!hasByrefHavoc) - { - // Allocate a distinct defnum for the GC Heap - ssaNum = m_pCompiler->lvMemoryPerSsaData.AllocSsaNum(m_allocator); - } + defNode->VisitLocalDefs(m_pCompiler, visitDef); - m_renameStack.PushMemory(GcHeap, block, ssaNum); - m_pCompiler->GetMemorySsaMap(GcHeap)->Set(defNode, ssaNum); - AddMemoryDefToEHSuccessorPhis(GcHeap, block, ssaNum); - } - } + if (!anyDefs) + { + if (defNode->OperIs(GT_CALL)) + { + // If the current def is a call we either know the call is pure or else has arbitrary memory definitions, + // the memory effect of the call is captured by the live out state from the block and doesn't need special + // handling here. If we ever change liveness to more carefully model call effects (from interprecedural + // information) we might need to revisit this. + return; } + + RenamePushMemoryDef(defNode, block); } } @@ -583,6 +542,66 @@ unsigned SsaBuilder::RenamePushDef(GenTree* defNode, BasicBlock* block, unsigned return ssaNum; } +void SsaBuilder::RenamePushMemoryDef(GenTree* defNode, BasicBlock* block) +{ + // Figure out if "defNode" may make a new GC heap state (if we care for this block). + if (((block->bbMemoryHavoc & memoryKindSet(GcHeap)) != 0) || !m_pCompiler->ehBlockHasExnFlowDsc(block)) + { + return; + } + + bool hasByrefHavoc = ((block->bbMemoryHavoc & memoryKindSet(ByrefExposed)) != 0); + + if (defNode->OperIsAnyLocal() && hasByrefHavoc) + { + // No need to record these. + return; + } + + // It *may* define byref memory in a non-havoc way. Make a new SSA # -- associate with this node. + unsigned ssaNum = m_pCompiler->lvMemoryPerSsaData.AllocSsaNum(m_allocator); + if (!hasByrefHavoc) + { + m_renameStack.PushMemory(ByrefExposed, block, ssaNum); + m_pCompiler->GetMemorySsaMap(ByrefExposed)->Set(defNode, ssaNum); +#ifdef DEBUG + if (m_pCompiler->verboseSsa) + { + printf("Node "); + Compiler::printTreeID(defNode); + printf(" (in try block) may define memory; ssa # = %d.\n", ssaNum); + } +#endif // DEBUG + + // Now add this SSA # to all phis of the reachable catch blocks. + AddMemoryDefToEHSuccessorPhis(ByrefExposed, block, ssaNum); + } + + if (!defNode->OperIsAnyLocal()) + { + // Add a new def for GcHeap as well + if (m_pCompiler->byrefStatesMatchGcHeapStates) + { + // GcHeap and ByrefExposed share the same stacks, SsaMap, and phis + assert(!hasByrefHavoc); + assert(*m_pCompiler->GetMemorySsaMap(GcHeap)->LookupPointer(defNode) == ssaNum); + assert(block->bbMemorySsaPhiFunc[GcHeap] == block->bbMemorySsaPhiFunc[ByrefExposed]); + } + else + { + if (!hasByrefHavoc) + { + // Allocate a distinct defnum for the GC Heap + ssaNum = m_pCompiler->lvMemoryPerSsaData.AllocSsaNum(m_allocator); + } + + m_renameStack.PushMemory(GcHeap, block, ssaNum); + m_pCompiler->GetMemorySsaMap(GcHeap)->Set(defNode, ssaNum); + AddMemoryDefToEHSuccessorPhis(GcHeap, block, ssaNum); + } + } +} + //------------------------------------------------------------------------ // RenameLclUse: Rename a use of a local variable. // diff --git a/src/coreclr/jit/ssabuilder.h b/src/coreclr/jit/ssabuilder.h index fd1ea275bb99cc..70f0cb23832eb7 100644 --- a/src/coreclr/jit/ssabuilder.h +++ b/src/coreclr/jit/ssabuilder.h @@ -81,6 +81,7 @@ class SsaBuilder // Rename a local or memory definition generated by a local store/GT_CALL node. void RenameDef(GenTree* defNode, BasicBlock* block); unsigned RenamePushDef(GenTree* defNode, BasicBlock* block, unsigned lclNum, bool isFullDef); + void RenamePushMemoryDef(GenTree* defNode, BasicBlock* block); // Rename a use of a local variable. void RenameLclUse(GenTreeLclVarCommon* lclNode, BasicBlock* block); diff --git a/src/coreclr/jit/treelifeupdater.cpp b/src/coreclr/jit/treelifeupdater.cpp index a8f7408b182373..786414fd368976 100644 --- a/src/coreclr/jit/treelifeupdater.cpp +++ b/src/coreclr/jit/treelifeupdater.cpp @@ -295,23 +295,21 @@ void TreeLifeUpdater::UpdateLife(GenTree* tree) } // Note that after lowering, we can see indirect uses and definitions of tracked variables. - GenTreeLclVarCommon* lclVarTree = nullptr; if (tree->OperIsNonPhiLocal()) { - lclVarTree = tree->AsLclVarCommon(); + UpdateLifeVar(tree, tree->AsLclVarCommon()); } else if (tree->OperIsIndir() && tree->AsIndir()->Addr()->OperIs(GT_LCL_ADDR)) { - lclVarTree = tree->AsIndir()->Addr()->AsLclVarCommon(); + UpdateLifeVar(tree, tree->AsIndir()->Addr()->AsLclVarCommon()); } else if (tree->IsCall()) { - lclVarTree = compiler->gtCallGetDefinedRetBufLclAddr(tree->AsCall()); - } - - if (lclVarTree != nullptr) - { - UpdateLifeVar(tree, lclVarTree); + auto visitDef = [=](const LocalDef& def) { + UpdateLifeVar(tree, def.Def); + return GenTree::VisitResult::Continue; + }; + tree->VisitLocalDefs(compiler, visitDef); } } diff --git a/src/coreclr/jit/valuenum.cpp b/src/coreclr/jit/valuenum.cpp index 19a34dba7f6bf6..287289e2cac6e5 100644 --- a/src/coreclr/jit/valuenum.cpp +++ b/src/coreclr/jit/valuenum.cpp @@ -11954,7 +11954,7 @@ void Compiler::fgValueNumberStore(GenTree* store) } else { - assert(!store->DefinesLocal(this, &lclVarTree)); + assert(!store->HasAnyLocalDefs(this)); // If it doesn't define a local, then it might update GcHeap/ByrefExposed. // For the new ByrefExposed VN, we could use an operator here like // VNF_ByrefExposedStore that carries the VNs of the pointer and RHS, then @@ -13809,18 +13809,17 @@ void Compiler::fgValueNumberCall(GenTreeCall* call) } } - // If the call generates a definition, because it uses "return buffer", then VN the local + // If the call generates any definitions, for example because it uses "return buffer", then VN the local // as well. - GenTreeLclVarCommon* lclVarTree = nullptr; - ssize_t offset = 0; - unsigned storeSize = 0; - if (call->DefinesLocal(this, &lclVarTree, /* pIsEntire */ nullptr, &offset, &storeSize)) - { + auto visitDef = [=](const LocalDef& def) { ValueNumPair storeValue; - storeValue.SetBoth(vnStore->VNForExpr(compCurBB, TYP_STRUCT)); + storeValue.SetBoth(vnStore->VNForExpr(compCurBB, lvaGetDesc(def.Def->AsLclVarCommon())->TypeGet())); - fgValueNumberLocalStore(call, lclVarTree, offset, storeSize, storeValue); - } + fgValueNumberLocalStore(call, def.Def, def.Offset, def.Size, storeValue); + return GenTree::VisitResult::Continue; + }; + + call->VisitLocalDefs(this, visitDef); } void Compiler::fgValueNumberCastHelper(GenTreeCall* call) From fba620a121d2360da5f760517a211c540e8e7c69 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 16 Jul 2025 12:24:30 +0200 Subject: [PATCH 03/28] Mitigate some TP diffs --- src/coreclr/jit/compiler.hpp | 40 ++++++++++++++++++++++++++++- src/coreclr/jit/copyprop.cpp | 19 +++++++------- src/coreclr/jit/flowgraph.cpp | 6 ++--- src/coreclr/jit/gentree.h | 3 +++ src/coreclr/jit/liveness.cpp | 6 ++--- src/coreclr/jit/lower.cpp | 6 ++--- src/coreclr/jit/morph.cpp | 9 +++---- src/coreclr/jit/treelifeupdater.cpp | 6 ++--- 8 files changed, 67 insertions(+), 28 deletions(-) diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 172c65d306b7c2..edca6c1db34070 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4750,6 +4750,44 @@ GenTree::VisitResult GenTree::VisitLocalDefs(Compiler* comp, TVisitor visitor) return VisitResult::Continue; } +//------------------------------------------------------------------------ +// VisitLocalDefNodes: Visit GenTreeLclVarCommon nodes representing definitions in the specified node. +// +// Arguments: +// comp - the compiler instance +// visitor - Functor of type GenTree::VisitResult(GenTreeLclVarCommon*) +// +// Return Value: +// VisitResult::Abort if the functor aborted; otherwise VisitResult::Continue. +// +// Notes: +// This function is contractually bound to recognize a superset of stores +// that "LocalAddressVisitor" recognizes and transforms, as it is used to +// detect which trees can define tracked locals. +// +template +GenTree::VisitResult GenTree::VisitLocalDefNodes(Compiler* comp, TVisitor visitor) +{ + if (OperIs(GT_STORE_LCL_VAR)) + { + return visitor(AsLclVarCommon()); + } + if (OperIs(GT_STORE_LCL_FLD)) + { + return visitor(AsLclFld()); + } + if (OperIs(GT_CALL)) + { + GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(AsCall()); + if (lclAddr != nullptr) + { + return visitor(lclAddr); + } + } + + return VisitResult::Continue; +} + //------------------------------------------------------------------------ // HasAnyLocalDefs: // Check if a tree is considered as defining any locals. @@ -4762,7 +4800,7 @@ GenTree::VisitResult GenTree::VisitLocalDefs(Compiler* comp, TVisitor visitor) // inline bool GenTree::HasAnyLocalDefs(Compiler* comp) { - return VisitLocalDefs(comp, [](const LocalDef& def) { + return VisitLocalDefNodes(comp, [](GenTreeLclVarCommon* lcl) { return GenTree::VisitResult::Abort; }) == GenTree::VisitResult::Abort; } diff --git a/src/coreclr/jit/copyprop.cpp b/src/coreclr/jit/copyprop.cpp index 9e50047aa02094..22ebf6a8791346 100644 --- a/src/coreclr/jit/copyprop.cpp +++ b/src/coreclr/jit/copyprop.cpp @@ -51,26 +51,26 @@ void Compiler::optBlockCopyPropPopStacks(BasicBlock* block, LclNumToLiveDefsMap* continue; } - auto visitDef = [=](const LocalDef& def) { - if (def.Def->HasCompositeSsaName()) + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + if (lcl->HasCompositeSsaName()) { - LclVarDsc* varDsc = lvaGetDesc(def.Def); + LclVarDsc* varDsc = lvaGetDesc(lcl); assert(varDsc->lvPromoted); for (unsigned index = 0; index < varDsc->lvFieldCnt; index++) { - popDef(varDsc->lvFieldLclStart + index, def.Def->GetSsaNum(this, index)); + popDef(varDsc->lvFieldLclStart + index, lcl->GetSsaNum(this, index)); } } else { - popDef(def.Def->GetLclNum(), def.Def->GetSsaNum()); + popDef(lcl->GetLclNum(), lcl->GetSsaNum()); } return GenTree::VisitResult::Continue; }; - tree->VisitLocalDefs(this, visitDef); + tree->VisitLocalDefNodes(this, visitDef); } } } @@ -406,15 +406,14 @@ bool Compiler::optBlockCopyProp(BasicBlock* block, LclNumToLiveDefsMap* curSsaNa { treeLifeUpdater.UpdateLife(tree); - GenTreeLclVarCommon* lclDefNode = nullptr; if (tree->OperIsSsaDef()) { - auto visitDef = [=](const LocalDef& def) { - optCopyPropPushDef(def.Def, curSsaName); + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + optCopyPropPushDef(lcl, curSsaName); return GenTree::VisitResult::Continue; }; - tree->VisitLocalDefs(this, visitDef); + tree->VisitLocalDefNodes(this, visitDef); } else if (tree->OperIs(GT_LCL_VAR, GT_LCL_FLD) && tree->AsLclVarCommon()->HasSsaName()) { diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 92a84b0995b647..43575b8c2c495f 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -5391,11 +5391,11 @@ bool FlowGraphNaturalLoop::VisitDefs(TFunc func) return Compiler::WALK_SKIP_SUBTREES; } - auto visitDef = [=](const LocalDef& def) { - return m_func(def.Def) ? GenTree::VisitResult::Continue : GenTree::VisitResult::Abort; + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + return m_func(lcl) ? GenTree::VisitResult::Continue : GenTree::VisitResult::Abort; }; - if (tree->VisitLocalDefs(m_compiler, visitDef) == GenTree::VisitResult::Abort) + if (tree->VisitLocalDefNodes(m_compiler, visitDef) == GenTree::VisitResult::Abort) { return Compiler::WALK_ABORT; } diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 8ea0d2651cfc40..c618e5b84d1027 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -2048,6 +2048,9 @@ struct GenTree template VisitResult VisitLocalDefs(Compiler* comp, TVisitor visitor); + template + VisitResult VisitLocalDefNodes(Compiler* comp, TVisitor visitor); + bool HasAnyLocalDefs(Compiler* comp); GenTreeLclVarCommon* IsImplicitByrefParameterValuePreMorph(Compiler* compiler); diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index ae6294af115eae..08ef18528c1867 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -342,11 +342,11 @@ void Compiler::fgPerNodeLocalVarLiveness(GenTree* tree) } } - auto visitDef = [=](const LocalDef& def) { - fgMarkUseDef(def.Def); + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + fgMarkUseDef(lcl); return GenTree::VisitResult::Continue; }; - call->VisitLocalDefs(this, visitDef); + call->VisitLocalDefNodes(this, visitDef); break; } diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 410cd34039fdaf..6c57e953302c04 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -8636,12 +8636,12 @@ void Lowering::FindInducedParameterRegisterLocals() { hasRegisterKill |= node->IsCall(); - auto visitDefs = [&](const LocalDef& def) { - storedToLocals.Emplace(def.Def->GetLclNum(), true); + auto visitDefs = [&](GenTreeLclVarCommon* lcl) { + storedToLocals.Emplace(lcl->GetLclNum(), true); return GenTree::VisitResult::Continue; }; - node->VisitLocalDefs(comp, visitDefs); + node->VisitLocalDefNodes(comp, visitDefs); if (node->OperIs(GT_LCL_ADDR)) { diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 4802268169946d..02d85faa37797a 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -12635,15 +12635,14 @@ void Compiler::fgMorphTreeDone(GenTree* tree, bool optAssertionPropDone DEBUGARG // Kill active assertions // - GenTreeLclVarCommon* lclVarTree = nullptr; - if ((optAssertionCount > 0)) + if (optAssertionCount > 0) { - auto visitDef = [=](const LocalDef& def) { - fgKillDependentAssertions(def.Def->GetLclNum() DEBUGARG(tree)); + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + fgKillDependentAssertions(lcl->GetLclNum() DEBUGARG(tree)); return GenTree::VisitResult::Continue; }; - tree->VisitLocalDefs(this, visitDef); + tree->VisitLocalDefNodes(this, visitDef); } // Generate assertions diff --git a/src/coreclr/jit/treelifeupdater.cpp b/src/coreclr/jit/treelifeupdater.cpp index 786414fd368976..5f77e0598aa463 100644 --- a/src/coreclr/jit/treelifeupdater.cpp +++ b/src/coreclr/jit/treelifeupdater.cpp @@ -305,11 +305,11 @@ void TreeLifeUpdater::UpdateLife(GenTree* tree) } else if (tree->IsCall()) { - auto visitDef = [=](const LocalDef& def) { - UpdateLifeVar(tree, def.Def); + auto visitDef = [=](GenTreeLclVarCommon* lcl) { + UpdateLifeVar(tree, lcl); return GenTree::VisitResult::Continue; }; - tree->VisitLocalDefs(compiler, visitDef); + tree->VisitLocalDefNodes(compiler, visitDef); } } From 3939f24a3860b61c4091eeaf0eef8aa9ef15dd7d Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 16 Jul 2025 12:30:47 +0200 Subject: [PATCH 04/28] Nit --- src/coreclr/jit/compiler.hpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index edca6c1db34070..10e0f7b124f537 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4740,10 +4740,7 @@ GenTree::VisitResult GenTree::VisitLocalDefs(Compiler* comp, TVisitor visitor) bool isEntire = storeSize == comp->lvaLclExactSize(lclAddr->GetLclNum()); - if (visitor(LocalDef(lclAddr, isEntire, lclAddr->GetLclOffs(), storeSize)) == VisitResult::Abort) - { - return VisitResult::Abort; - } + return visitor(LocalDef(lclAddr, isEntire, lclAddr->GetLclOffs(), storeSize)); } } From 5fec55fda29a07dde3167405e393b32b5f1e8a76 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 16 Jul 2025 16:21:16 +0200 Subject: [PATCH 05/28] WIP --- .../CompilerServices/AsyncHelpers.CoreCLR.cs | 9 +- src/coreclr/inc/corinfo.h | 3 +- src/coreclr/jit/async.cpp | 474 ++++++++++++------ src/coreclr/jit/async.h | 6 + src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/compiler.hpp | 31 +- src/coreclr/jit/gentree.cpp | 44 +- src/coreclr/jit/gentree.h | 10 + src/coreclr/jit/importercalls.cpp | 4 +- src/coreclr/jit/lclmorph.cpp | 15 +- src/coreclr/jit/liveness.cpp | 4 +- src/coreclr/jit/morph.cpp | 2 + src/coreclr/vm/corelib.h | 3 + src/coreclr/vm/jitinterface.cpp | 3 + 14 files changed, 449 insertions(+), 161 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs index fc2b3cb84f3364..9f187aea657c22 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs @@ -611,8 +611,13 @@ private static void CaptureContexts(out Thread currentThread, out ExecutionConte } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static void RestoreExecutionContextWithThread(Thread thread, ExecutionContext? previousExecCtx) + private static void RestoreContexts(bool suspended, Thread thread, ExecutionContext? previousExecCtx, SynchronizationContext? previousSyncCtx) { + if (!suspended && previousSyncCtx != thread._synchronizationContext) + { + thread._synchronizationContext = previousSyncCtx; + } + ExecutionContext? currentExecCtx = thread._executionContext; if (previousExecCtx != currentExecCtx) { @@ -640,5 +645,7 @@ private static void CaptureContinuationContext(ref object context, ref CorInfoCo flags |= CorInfoContinuationFlags.CORINFO_CONTINUATION_CONTINUE_ON_THREAD_POOL; } + + private static Thread GetCurrentThread() => Thread.CurrentThreadAssumedInitialized; } } diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index ba7129a82c8fc2..9b48f7f82fac62 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -1748,7 +1748,8 @@ struct CORINFO_ASYNC_INFO CORINFO_METHOD_HANDLE restoreExecutionContextMethHnd; CORINFO_METHOD_HANDLE captureContinuationContextMethHnd; CORINFO_METHOD_HANDLE captureContextsMethHnd; - CORINFO_METHOD_HANDLE restoreExecutionContextWithThreadMethHnd; + CORINFO_METHOD_HANDLE restoreContextsMethHnd; + CORINFO_METHOD_HANDLE getCurrentThreadMethHnd; }; // Flags passed from JIT to runtime. diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index a21763fc264cc7..14680555cf8294 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -67,133 +67,143 @@ PhaseStatus Compiler::SaveAsyncContexts() PhaseStatus result = PhaseStatus::MODIFIED_NOTHING; -// BasicBlock* curBB = fgFirstBB; -// while (curBB != nullptr) -// { -// BasicBlock* nextBB = curBB->Next(); -// -// for (Statement* stmt : curBB->Statements()) -// { -// GenTree* tree = stmt->GetRootNode(); -// if (tree->OperIs(GT_STORE_LCL_VAR)) -// { -// tree = tree->AsLclVarCommon()->Data(); -// } -// -// if (!tree->IsCall()) -// { -// ValidateNoAsyncSavesNecessaryInStatement(stmt); -// continue; -// } -// -// GenTreeCall* call = tree->AsCall(); -// const AsyncCallInfo& asyncCallInfo = call->GetAsyncInfo(); -// -// // Currently we always expect that ExecutionContenxt and -// // SynchronizationContext to correlate about their save/restore -// // behavior. -// assert((asyncCallInfo.ExecutionContextHandling == ExecutionContextHandling::SaveAndRestore) == asyncCallInfo.SaveAndRestoreSynchronizationContextField); -// -// if (asyncCallInfo.ExecutionContextHandling != ExecutionContextHandling::SaveAndRestore) -// { -// continue; -// } -// -// unsigned callSuspendedIndicatorLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Suspended indicator for [%06u]", dspTreeID(call)))); -// lvaGetDesc(callSuspendedIndicatorLclNum)->lvType = TYP_INT; -// -// call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclAddrNode(callSuspendedIndicatorLclNum, 0))); -// -// unsigned threadLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Thread for [%06u]", dspTreeID(call)))); -// unsigned execCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("ExecutionContext for [%06u]", dspTreeID(call)))); -// unsigned syncCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("SynchronizationContext for [%06u]", dspTreeID(call)))); -// -// JITDUMP("Saving contexts around [%06u], thread = V%02u, ExecutionContext = V%02u, SynchronizationContext = V%02u\n", call->gtTreeID, threadLclNum, execCtxLclNum, syncCtxLclNum); -// -// CORINFO_ASYNC_INFO* asyncInfo = eeGetAsyncInfo(); -// -// GenTreeCall* capture = gtNewCallNode(CT_USER_FUNC, asyncInfo->captureExecutionContextMethHnd, TYP_REF); -// CORINFO_CALL_INFO callInfo = {}; -// callInfo.hMethod = capture->gtCallMethHnd; -// callInfo.methodFlags = info.compCompHnd->getMethodAttribs(callInfo.hMethod); -// impMarkInlineCandidate(capture, MAKE_METHODCONTEXT(callInfo.hMethod), false, &callInfo, compInlineContext); -// -// if (capture->IsInlineCandidate()) -// { -// Statement* captureStmt = fgNewStmtFromTree(capture); -// -// GenTreeRetExpr* retExpr = gtNewInlineCandidateReturnExpr(capture, TYP_REF); -// -// capture->GetSingleInlineCandidateInfo()->retExpr = retExpr; -// GenTree* storeCapture = gtNewTempStore(lclNum, retExpr); -// Statement* storeCaptureStmt = fgNewStmtFromTree(storeCapture); -// -// fgInsertStmtBefore(curBB, stmt, captureStmt); -// fgInsertStmtBefore(curBB, stmt, storeCaptureStmt); -// -// JITDUMP("Inserted capture:\n"); -// DISPSTMT(captureStmt); -// DISPSTMT(storeCaptureStmt); -// } -// else -// { -// GenTree* storeCapture = gtNewTempStore(lclNum, capture); -// Statement* storeCaptureStmt = fgNewStmtFromTree(storeCapture); -// -// fgInsertStmtBefore(curBB, stmt, storeCaptureStmt); -// -// JITDUMP("Inserted capture:\n"); -// DISPSTMT(storeCaptureStmt); -// } -// -// BasicBlock* restoreBB = curBB; -// Statement* restoreAfterStmt = stmt; -// -// if (call->IsInlineCandidate() && (call->gtReturnType != TYP_VOID)) -// { -// restoreAfterStmt = stmt->GetNextStmt(); -// assert(restoreAfterStmt->GetRootNode()->OperIs(GT_RET_EXPR) || -// (restoreAfterStmt->GetRootNode()->OperIs(GT_STORE_LCL_VAR) && -// restoreAfterStmt->GetRootNode()->AsLclVarCommon()->Data()->OperIs(GT_RET_EXPR))); -// } -// -// if (curBB->hasTryIndex()) -// { -//#ifdef FEATURE_EH_WINDOWS_X86 -// IMPL_LIMITATION("Cannot handle insertion of try-finally without funclets"); -//#else -// // Await is inside a try, need to insert try-finally around it. -// restoreBB = InsertTryFinallyForContextRestore(curBB, stmt, restoreAfterStmt); -// restoreAfterStmt = nullptr; -//#endif -// } -// -// GenTreeCall* restore = gtNewCallNode(CT_USER_FUNC, asyncInfo->restoreExecutionContextMethHnd, TYP_VOID); -// restore->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclVarNode(lclNum))); -// -// callInfo = {}; -// callInfo.hMethod = restore->gtCallMethHnd; -// callInfo.methodFlags = info.compCompHnd->getMethodAttribs(callInfo.hMethod); -// impMarkInlineCandidate(restore, MAKE_METHODCONTEXT(callInfo.hMethod), false, &callInfo, compInlineContext); -// -// Statement* restoreStmt = fgNewStmtFromTree(restore); -// if (restoreAfterStmt == nullptr) -// { -// fgInsertStmtNearEnd(restoreBB, restoreStmt); -// } -// else -// { -// fgInsertStmtAfter(restoreBB, restoreAfterStmt, restoreStmt); -// } -// -// JITDUMP("Inserted restore:\n"); -// DISPSTMT(restoreStmt); -// -// result = PhaseStatus::MODIFIED_EVERYTHING; -// } -// -// curBB = nextBB; -// } + BasicBlock* curBB = fgFirstBB; + while (curBB != nullptr) + { + BasicBlock* nextBB = curBB->Next(); + + for (Statement* stmt : curBB->Statements()) + { + GenTree* tree = stmt->GetRootNode(); + if (tree->OperIs(GT_STORE_LCL_VAR)) + { + tree = tree->AsLclVarCommon()->Data(); + } + + if (!tree->IsCall()) + { + ValidateNoAsyncSavesNecessaryInStatement(stmt); + continue; + } + + GenTreeCall* call = tree->AsCall(); + if (!call->IsAsync()) + { + ValidateNoAsyncSavesNecessaryInStatement(stmt); + continue; + } + + const AsyncCallInfo& asyncCallInfo = call->GetAsyncInfo(); + + // Currently we always expect that ExecutionContext and + // SynchronizationContext correlate about their save/restore + // behavior. + assert((asyncCallInfo.ExecutionContextHandling == ExecutionContextHandling::SaveAndRestore) == asyncCallInfo.SaveAndRestoreSynchronizationContextField); + + if (asyncCallInfo.ExecutionContextHandling != ExecutionContextHandling::SaveAndRestore) + { + continue; + } + + unsigned suspendedLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Suspended indicator for [%06u]", dspTreeID(call)))); + LclVarDsc* suspendedLclDsc = lvaGetDesc(suspendedLclNum); + suspendedLclDsc->lvType = TYP_UBYTE; + suspendedLclDsc->lvHasLdAddrOp = true; + + call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclAddrNode(suspendedLclNum, 0)).WellKnown(WellKnownArg::AsyncSuspendedIndicator)); + + unsigned threadLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Thread for [%06u]", dspTreeID(call)))); + unsigned execCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("ExecutionContext for [%06u]", dspTreeID(call)))); + unsigned syncCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("SynchronizationContext for [%06u]", dspTreeID(call)))); + + // Update the locals in the async information. This needs a new + // AsyncCallInfo since the async call may have been cloned. + AsyncCallInfo newAsyncCallInfo = asyncCallInfo.WithLocals(threadLclNum, syncCtxLclNum); + call->asyncInfo = new (this, CMK_Async) AsyncCallInfo(newAsyncCallInfo); + + LclVarDsc* threadLclDsc = lvaGetDesc(threadLclNum); + threadLclDsc->lvType = TYP_REF; + threadLclDsc->lvHasLdAddrOp = true; + + LclVarDsc* execCtxLclDsc = lvaGetDesc(execCtxLclNum); + execCtxLclDsc->lvType = TYP_REF; + execCtxLclDsc->lvHasLdAddrOp = true; + + LclVarDsc* syncCtxLclDsc = lvaGetDesc(syncCtxLclNum); + syncCtxLclDsc->lvType = TYP_REF; + syncCtxLclDsc->lvHasLdAddrOp = true; + + JITDUMP("Saving contexts around [%06u], thread = V%02u, ExecutionContext = V%02u, SynchronizationContext = V%02u\n", call->gtTreeID, threadLclNum, execCtxLclNum, syncCtxLclNum); + + CORINFO_ASYNC_INFO* asyncInfo = eeGetAsyncInfo(); + + GenTreeCall* capture = gtNewCallNode(CT_USER_FUNC, asyncInfo->captureContextsMethHnd, TYP_VOID); + capture->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclAddrNode(syncCtxLclNum, 0))); + capture->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclAddrNode(execCtxLclNum, 0))); + capture->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclAddrNode(threadLclNum, 0))); + + CORINFO_CALL_INFO callInfo = {}; + callInfo.hMethod = capture->gtCallMethHnd; + callInfo.methodFlags = info.compCompHnd->getMethodAttribs(callInfo.hMethod); + impMarkInlineCandidate(capture, MAKE_METHODCONTEXT(callInfo.hMethod), false, &callInfo, compInlineContext); + + Statement* captureStmt = fgNewStmtFromTree(capture); + fgInsertStmtBefore(curBB, stmt, captureStmt); + + JITDUMP("Inserted capture:\n"); + DISPSTMT(captureStmt); + + BasicBlock* restoreBB = curBB; + Statement* restoreAfterStmt = stmt; + + if (call->IsInlineCandidate() && (call->gtReturnType != TYP_VOID)) + { + restoreAfterStmt = stmt->GetNextStmt(); + assert(restoreAfterStmt->GetRootNode()->OperIs(GT_RET_EXPR) || + (restoreAfterStmt->GetRootNode()->OperIs(GT_STORE_LCL_VAR) && + restoreAfterStmt->GetRootNode()->AsLclVarCommon()->Data()->OperIs(GT_RET_EXPR))); + } + + if (curBB->hasTryIndex()) + { +#ifdef FEATURE_EH_WINDOWS_X86 + IMPL_LIMITATION("Cannot handle insertion of try-finally without funclets"); +#else + // Await is inside a try, need to insert try-finally around it. + restoreBB = InsertTryFinallyForContextRestore(curBB, stmt, restoreAfterStmt); + restoreAfterStmt = nullptr; +#endif + } + + GenTreeCall* restore = gtNewCallNode(CT_USER_FUNC, asyncInfo->restoreContextsMethHnd, TYP_VOID); + restore->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclVarNode(syncCtxLclNum))); + restore->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclVarNode(execCtxLclNum))); + restore->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclVarNode(threadLclNum))); + restore->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclVarNode(suspendedLclNum))); + + callInfo = {}; + callInfo.hMethod = restore->gtCallMethHnd; + callInfo.methodFlags = info.compCompHnd->getMethodAttribs(callInfo.hMethod); + impMarkInlineCandidate(restore, MAKE_METHODCONTEXT(callInfo.hMethod), false, &callInfo, compInlineContext); + + Statement* restoreStmt = fgNewStmtFromTree(restore); + if (restoreAfterStmt == nullptr) + { + fgInsertStmtNearEnd(restoreBB, restoreStmt); + } + else + { + fgInsertStmtAfter(restoreBB, restoreAfterStmt, restoreStmt); + } + + JITDUMP("Inserted restore:\n"); + DISPSTMT(restoreStmt); + + result = PhaseStatus::MODIFIED_EVERYTHING; + } + + curBB = nextBB; + } return result; } @@ -380,7 +390,8 @@ class AsyncLiveness void StartBlock(BasicBlock* block); void Update(GenTree* node); bool IsLive(unsigned lclNum); - void GetLiveLocals(jitstd::vector& liveLocals, unsigned fullyDefinedRetBufLcl); + template + void GetLiveLocals(jitstd::vector& liveLocals, Functor includeLocal); private: bool IsLocalCaptureUnnecessary(unsigned lclNum); @@ -558,14 +569,15 @@ bool AsyncLiveness::IsLive(unsigned lclNum) // Get live locals that should be captured at this point. // // Parameters: -// liveLocals - Vector to add live local information into -// fullyDefinedRetBufLcl - Local to skip even if live +// liveLocals - Vector to add live local information into +// includeLocal - Functor to check if a local should be included // -void AsyncLiveness::GetLiveLocals(jitstd::vector& liveLocals, unsigned fullyDefinedRetBufLcl) +template +void AsyncLiveness::GetLiveLocals(jitstd::vector& liveLocals, Functor includeLocal) { for (unsigned lclNum = 0; lclNum < m_numVars; lclNum++) { - if ((lclNum != fullyDefinedRetBufLcl) && IsLive(lclNum)) + if (includeLocal(lclNum) && IsLive(lclNum)) { liveLocals.push_back(LiveLocalInfo(lclNum)); } @@ -795,6 +807,8 @@ void AsyncTransformation::Transform( ContinuationLayout layout = LayOutContinuation(block, call, liveLocals); + ClearSuspendedIndicator(block, call); + CallDefinitionInfo callDefInfo = CanonicalizeCallDefinition(block, call, life); unsigned stateNum = (unsigned)m_resumptionBBs.size(); @@ -802,7 +816,7 @@ void AsyncTransformation::Transform( BasicBlock* suspendBB = CreateSuspension(block, call, stateNum, life, layout); - CreateCheckAndSuspendAfterCall(block, callDefInfo, life, suspendBB, remainder); + CreateCheckAndSuspendAfterCall(block, call, callDefInfo, life, suspendBB, remainder); BasicBlock* resumeBB = CreateResumption(block, *remainder, call, callDefInfo, stateNum, layout); @@ -827,27 +841,35 @@ void AsyncTransformation::CreateLiveSetForSuspension(BasicBlock* AsyncLiveness& life, jitstd::vector& liveLocals) { - unsigned fullyDefinedRetBufLcl = BAD_VAR_NUM; - CallArg* retbufArg = call->gtArgs.GetRetBufferArg(); - if (retbufArg != nullptr) - { - GenTree* retbuf = retbufArg->GetNode(); - if (retbuf->IsLclVarAddr()) + SmallHashTable excludedLocals(m_comp->getAllocator(CMK_Async)); + + auto visitDef = [&](const LocalDef& def) { + if (def.IsEntire) { - LclVarDsc* dsc = m_comp->lvaGetDesc(retbuf->AsLclVarCommon()); - ClassLayout* defLayout = m_comp->typGetObjLayout(call->gtRetClsHnd); - if (defLayout->GetSize() == dsc->lvExactSize()) - { - // This call fully defines this retbuf. There is no need to - // consider it live across the call since it is going to be - // overridden anyway. - fullyDefinedRetBufLcl = retbuf->AsLclVarCommon()->GetLclNum(); - JITDUMP(" V%02u is a fully defined retbuf and will not be considered live\n", fullyDefinedRetBufLcl); - } + JITDUMP(" V%02u is fully defined and will not be considered live\n", def.Def->GetLclNum()); + excludedLocals.AddOrUpdate(def.Def->GetLclNum(), true); } + return GenTree::VisitResult::Continue; + }; + + call->VisitLocalDefs(m_comp, visitDef); + + const AsyncCallInfo& asyncInfo = call->GetAsyncInfo(); + + // These may look live, but they aren't actually live + if (asyncInfo.ThreadObjectLclNum != BAD_VAR_NUM) + { + // This one we define in resumption, but don't model as a proper def (although maybe we ought to) + excludedLocals.AddOrUpdate(asyncInfo.ThreadObjectLclNum, true); + } + + if (asyncInfo.SynchronizationContextLclNum != BAD_VAR_NUM) + { + // This one is only live on the synchronous path, which liveness cannot prove + excludedLocals.AddOrUpdate(asyncInfo.SynchronizationContextLclNum, true); } - life.GetLiveLocals(liveLocals, fullyDefinedRetBufLcl); + life.GetLiveLocals(liveLocals, [&](unsigned lclNum) { return !excludedLocals.Contains(lclNum); }); LiftLIREdges(block, defs, liveLocals); #ifdef DEBUG @@ -1099,6 +1121,76 @@ ContinuationLayout AsyncTransformation::LayOutContinuation(BasicBlock* return layout; } +void AsyncTransformation::ClearSuspendedIndicator(BasicBlock* block, GenTreeCall* call) +{ + CallArg* suspendedArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); + if (suspendedArg == nullptr) + { + return; + } + + GenTree* suspended = suspendedArg->GetNode(); + if (!suspended->IsLclVarAddr() && (!suspended->OperIs(GT_LCL_VAR) || m_comp->lvaVarAddrExposed(suspended->AsLclVarCommon()->GetLclNum()))) + { + // We will need a second use of this, so spill to a local + LIR::Use use(LIR::AsRange(block), &suspendedArg->NodeRef(), call); + use.ReplaceWithLclVar(m_comp); + suspended = use.Def(); + } + + GenTree* value = m_comp->gtNewIconNode(0); + GenTree* storeSuspended = m_comp->gtNewStoreValueNode(TYP_UBYTE, m_comp->gtCloneExpr(suspended), value, GTF_IND_NONFAULTING); + + LIR::AsRange(block).InsertBefore(call, LIR::SeqTree(m_comp, storeSuspended)); +} + +void AsyncTransformation::SetSuspendedIndicator(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call) +{ + CallArg* suspendedArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); + if (suspendedArg == nullptr) + { + return; + } + + GenTree* suspended = suspendedArg->GetNode(); + assert(suspended->IsLclVarAddr() || suspended->OperIs(GT_LCL_VAR)); // Ensured by ClearSuspendedIndicator + + GenTree* value = m_comp->gtNewIconNode(1); + GenTree* storeSuspended = m_comp->gtNewStoreValueNode(TYP_UBYTE, m_comp->gtCloneExpr(suspended), value, GTF_IND_NONFAULTING); + + LIR::AsRange(block).InsertAtEnd(LIR::SeqTree(m_comp, storeSuspended)); + + call->gtArgs.RemoveUnsafe(suspendedArg); + + // Avoid leaving LCL_ADDR around which will DNER the local. + if (suspended->IsLclVarAddr()) + { + LIR::AsRange(callBlock).Remove(suspended); + } + else + { + suspended->SetUnusedValue(); + } +} + +void AsyncTransformation::UpdateThreadObject(BasicBlock* block, GenTreeCall* call) +{ + const AsyncCallInfo& asyncInfo = call->GetAsyncInfo(); + if (asyncInfo.ThreadObjectLclNum == BAD_VAR_NUM) + { + return; + } + + GenTreeCall* getCurrentThread = + m_comp->gtNewCallNode(CT_USER_FUNC, m_asyncInfo->getCurrentThreadMethHnd, TYP_REF); + + m_comp->compCurBB = block; + m_comp->fgMorphTree(getCurrentThread); + + GenTree* store = m_comp->gtNewStoreLclVarNode(asyncInfo.ThreadObjectLclNum, getCurrentThread); + LIR::AsRange(block).InsertAtEnd(LIR::SeqTree(m_comp, store)); +} + //------------------------------------------------------------------------ // AsyncTransformation::CanonicalizeCallDefinition: // Put the call definition in a canonical form. This ensures that either the @@ -1558,12 +1650,14 @@ void AsyncTransformation::FillInDataOnSuspension(const jitstd::vectorGetTrueEdge()->setLikelihood(0); block->GetFalseEdge()->setLikelihood(1); + + JumpThreadSuspensionCheck(block->GetFalseEdge(), call, 0); +} + +void AsyncTransformation::JumpThreadSuspensionCheck(FlowEdge* edge, GenTreeCall* call, uint8_t suspendedValue) +{ + JITDUMP(" Seeing if we can jump thread a check for suspension in " FMT_BB " -> " FMT_BB " with known value %d\n", edge->getSourceBlock()->bbNum, edge->getDestinationBlock()->bbNum, suspendedValue); + + CallArg* suspendedArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); + if (suspendedArg == nullptr) + { + JITDUMP(" No async suspension indicator argument found\n"); + return; + } + + GenTree* suspended = suspendedArg->GetNode(); + if (!suspended->IsLclVarAddr() || (suspended->AsLclVarCommon()->GetLclOffs() != 0) || (m_comp->lvaGetDesc(suspended->AsLclVarCommon())->lvExactSize() != 1)) + { + JITDUMP(" Async suspension indicator is not of right shape\n"); + return; + } + + unsigned suspendedLclNum = suspended->AsLclVarCommon()->GetLclNum(); + BasicBlock* blockTarget = edge->getDestinationBlock(); + if (!blockTarget->KindIs(BBJ_COND)) + { + JITDUMP(" Block target is not BBJ_COND\n"); + return; + } + + GenTree* rootNode = LIR::AsRange(blockTarget).FirstNode(); + + if (!rootNode->OperIs(GT_JTRUE)) + { + JITDUMP(" Block target has other IR\n"); + return; + } + + GenTree* cmp = rootNode->gtGetOp1(); + if (!cmp->OperIs(GT_EQ, GT_NE)) + { + JITDUMP(" Block target is not JTRUE(EQ/NE)\n"); + return; + } + + GenTree* op1 = cmp->gtGetOp1(); + GenTree* op2 = cmp->gtGetOp2(); + if (!op2->IsCnsIntOrI()) + { + std::swap(op1, op2); + } + + if (!op1->OperIs(GT_LCL_VAR) || (op1->AsLclVar()->GetLclNum() != suspendedLclNum)) + { + JITDUMP(" Op1 is not suspended indicator\n"); + return; + } + + if (!op2->IsIntegralConst(0)) + { + JITDUMP(" Op2 is not constant 0\n"); + return; + } + + BasicBlock* actualTarget = (cmp->OperIs(GT_EQ) == (suspendedValue == 0)) ? blockTarget->GetTrueTarget() : blockTarget->GetFalseTarget(); + JITDUMP(" We can jump thread " FMT_BB " -> " FMT_BB " into " FMT_BB " -> " FMT_BB "\n", edge->getSourceBlock()->bbNum, edge->getDestinationBlock()->bbNum, edge->getSourceBlock()->bbNum, actualTarget->bbNum); + + m_comp->fgRedirectEdge(edge, actualTarget); } //------------------------------------------------------------------------ @@ -1634,6 +1796,15 @@ BasicBlock* AsyncTransformation::CreateResumption(BasicBlock* bloc JITDUMP(" Creating resumption " FMT_BB " for state %u\n", resumeBB->bbNum, stateNum); + // Start out by setting the indicator local, if necessary. This will + // usually be optimized by liveness because the resumption path will start + // out with "if (!suspended)", which we can jump thread unless we may have + // exceptional control flow. + SetSuspendedIndicator(resumeBB, block, call); + + // Also the thread object needs to be updated to the current thread. + UpdateThreadObject(resumeBB, call); + // We need to restore data before we restore GC pointers, since restoring // the data may also write the GC pointer fields with nulls. unsigned resumeByteArrLclNum = BAD_VAR_NUM; @@ -1678,6 +1849,9 @@ BasicBlock* AsyncTransformation::CreateResumption(BasicBlock* bloc storeResultBB); } + assert(storeResultBB->KindIs(BBJ_ALWAYS)); + JumpThreadSuspensionCheck(storeResultBB->GetTargetEdge(), call, 1); + return resumeBB; } diff --git a/src/coreclr/jit/async.h b/src/coreclr/jit/async.h index e75f2ac8d157f6..9539d7387e4bf3 100644 --- a/src/coreclr/jit/async.h +++ b/src/coreclr/jit/async.h @@ -84,6 +84,8 @@ class AsyncTransformation GenTreeCall* call, jitstd::vector& liveLocals); + void ClearSuspendedIndicator(BasicBlock* block, GenTreeCall* call); + CallDefinitionInfo CanonicalizeCallDefinition(BasicBlock* block, GenTreeCall* call, AsyncLiveness& life); BasicBlock* CreateSuspension( @@ -95,10 +97,12 @@ class AsyncTransformation void FillInGCPointersOnSuspension(const ContinuationLayout& layout, BasicBlock* suspendBB); void FillInDataOnSuspension(const jitstd::vector& liveLocals, BasicBlock* suspendBB); void CreateCheckAndSuspendAfterCall(BasicBlock* block, + GenTreeCall* call, const CallDefinitionInfo& callDefInfo, AsyncLiveness& life, BasicBlock* suspendBB, BasicBlock** remainder); + void JumpThreadSuspensionCheck(FlowEdge* edge, GenTreeCall* call, uint8_t suspendedValue); BasicBlock* CreateResumption(BasicBlock* block, BasicBlock* remainder, @@ -106,6 +110,8 @@ class AsyncTransformation const CallDefinitionInfo& callDefInfo, unsigned stateNum, const ContinuationLayout& layout); + void SetSuspendedIndicator(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call); + void UpdateThreadObject(BasicBlock* block, GenTreeCall* call); void RestoreFromDataOnResumption(unsigned resumeByteArrLclNum, const jitstd::vector& liveLocals, BasicBlock* resumeBB); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 7e4950a1be12da..e486b36bb8514b 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3740,7 +3740,7 @@ class Compiler bool gtIsTypeof(GenTree* tree, CORINFO_CLASS_HANDLE* handle = nullptr); GenTreeLclVarCommon* gtCallGetDefinedRetBufLclAddr(GenTreeCall* call); - GenTreeLclVarCommon* gtCallGetDefinedAsyncSuspensionIndicatorLclAddr(GenTreeCall* call); + GenTreeLclVarCommon* gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(GenTreeCall* call); //------------------------------------------------------------------------- // Functions to display the trees diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 10e0f7b124f537..49f9a2f2bb17e8 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4733,7 +4733,21 @@ GenTree::VisitResult GenTree::VisitLocalDefs(Compiler* comp, TVisitor visitor) } if (OperIs(GT_CALL)) { - GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(AsCall()); + GenTreeCall* call = AsCall(); + if (call->IsAsync()) + { + GenTreeLclVarCommon* suspendedArg = comp->gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(call); + if (suspendedArg != nullptr) + { + bool isEntire = comp->lvaLclExactSize(suspendedArg->GetLclNum()) == 1; + if (visitor(LocalDef(suspendedArg, isEntire, suspendedArg->GetLclOffs(), 1)) == VisitResult::Abort) + { + return VisitResult::Abort; + } + } + } + + GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(call); if (lclAddr != nullptr) { unsigned storeSize = comp->typGetObjLayout(AsCall()->gtRetClsHnd)->GetSize(); @@ -4775,7 +4789,20 @@ GenTree::VisitResult GenTree::VisitLocalDefNodes(Compiler* comp, TVisitor visito } if (OperIs(GT_CALL)) { - GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(AsCall()); + GenTreeCall* call = AsCall(); + if (call->IsAsync()) + { + GenTreeLclVarCommon* suspendedArg = comp->gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(call); + if (suspendedArg != nullptr) + { + if (visitor(suspendedArg) == VisitResult::Abort) + { + return VisitResult::Abort; + } + } + } + + GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(call); if (lclAddr != nullptr) { return visitor(lclAddr); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 19e3928d0b5328..70093dc11d8996 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -1829,6 +1829,46 @@ void CallArgs::Remove(CallArg* arg) assert(!"Did not find arg to remove in CallArgs::Remove"); } +//--------------------------------------------------------------- +// RemoveUnsafe: Remove an argument from the argument list, without validation. +// +// Parameters: +// arg - The arg to remove. +// +// Remarks: +// This function will break ABI information of other arguments. The caller +// needs to know what they are doing. +// +void CallArgs::RemoveUnsafe(CallArg* arg) +{ + CallArg** slot = &m_lateHead; + while (*slot != nullptr) + { + if (*slot == arg) + { + *slot = arg->GetLateNext(); + break; + } + + slot = &(*slot)->LateNextRef(); + } + + slot = &m_head; + while (*slot != nullptr) + { + if (*slot == arg) + { + *slot = arg->GetNext(); + RemovedWellKnownArg(arg->GetWellKnownArg()); + return; + } + + slot = &(*slot)->NextRef(); + } + + assert(!"Did not find arg to remove in CallArgs::Remove"); +} + #ifdef TARGET_XARCH //--------------------------------------------------------------- // NeedsVzeroupper: Determines if the call needs a vzeroupper emitted before it is invoked @@ -13180,6 +13220,8 @@ const char* Compiler::gtGetWellKnownArgNameForArgMsg(WellKnownArg arg) return "&lcl arr"; case WellKnownArg::RuntimeMethodHandle: return "meth hnd"; + case WellKnownArg::AsyncSuspendedIndicator: + return "async susp"; default: return nullptr; } @@ -19563,7 +19605,7 @@ GenTreeLclVarCommon* Compiler::gtCallGetDefinedRetBufLclAddr(GenTreeCall* call) return node->AsLclVarCommon(); } -GenTreeLclVarCommon* Compiler::gtCallGetDefinedAsyncSuspensionIndicatorLclAddr(GenTreeCall* call) +GenTreeLclVarCommon* Compiler::gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(GenTreeCall* call) { if (!call->IsAsync()) { diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index c3eebc9a93d52c..06450ada3411fa 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4362,7 +4362,16 @@ struct AsyncCallInfo ExecutionContextHandling ExecutionContextHandling = ExecutionContextHandling::None; ContinuationContextHandling ContinuationContextHandling = ContinuationContextHandling::None; bool SaveAndRestoreSynchronizationContextField = false; + unsigned ThreadObjectLclNum = BAD_VAR_NUM; unsigned SynchronizationContextLclNum = BAD_VAR_NUM; + + AsyncCallInfo WithLocals(unsigned threadObjectLclNum, unsigned synchronizationContextLclNum) const + { + AsyncCallInfo value = *this; + value.ThreadObjectLclNum = threadObjectLclNum; + value.SynchronizationContextLclNum = synchronizationContextLclNum; + return value; + } }; // Return type descriptor of a GT_CALL node. @@ -4850,6 +4859,7 @@ class CallArgs CallArg* InsertAfterThisOrFirst(Compiler* comp, const NewCallArg& arg); void PushLateBack(CallArg* arg); void Remove(CallArg* arg); + void RemoveUnsafe(CallArg* arg); template void InternalCopyFrom(Compiler* comp, CallArgs* other, CopyNodeFunc copyFunc); diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index e7914b28834e18..cd9050019f64a5 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -706,6 +706,7 @@ var_types Compiler::impImportCall(OPCODE opcode, JITDUMP("Call is an async task await\n"); asyncInfo.ExecutionContextHandling = ExecutionContextHandling::SaveAndRestore; + asyncInfo.SaveAndRestoreSynchronizationContextField = true; if ((prefixFlags & PREFIX_TASK_AWAIT_CONTINUE_ON_CAPTURED_CONTEXT) != 0) { @@ -729,7 +730,7 @@ var_types Compiler::impImportCall(OPCODE opcode, asyncInfo.ExecutionContextHandling = ExecutionContextHandling::AsyncSaveAndRestore; } - // For tailcalls the context does not need saving/restoring: it will be + // For tailcalls the contexts does not need saving/restoring: they will be // overwritten by the caller anyway. // // More specifically, if we can show that @@ -739,6 +740,7 @@ var_types Compiler::impImportCall(OPCODE opcode, if (tailCallFlags != 0) { asyncInfo.ExecutionContextHandling = ExecutionContextHandling::None; + asyncInfo.SaveAndRestoreSynchronizationContextField = false; } call->AsCall()->SetIsAsync(new (this, CMK_Async) AsyncCallInfo(asyncInfo)); diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 1b3dcc5bd7d535..e35fe8b013c2c3 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1496,9 +1496,20 @@ class LocalAddressVisitor final : public GenTreeVisitor } } - if ((callUser != nullptr) && m_compiler->IsValidLclAddr(lclNum, val.Offset())) + if ((callUser != nullptr) && callUser->IsAsync() && m_compiler->IsValidLclAddr(lclNum, val.Offset())) { + CallArg* suspendedArg = callUser->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); + if ((suspendedArg != nullptr) && (val.Node() == suspendedArg->GetNode())) + { + varDsc->SetDefinedViaAddress(true); + escapeAddr = false; + defFlag = GTF_VAR_DEF; + if ((val.Offset() != 0) || (varDsc->lvExactSize() != 1)) + { + defFlag |= GTF_VAR_USEASG; + } + } } if (escapeAddr) @@ -1521,7 +1532,7 @@ class LocalAddressVisitor final : public GenTreeVisitor // a ByRef to an INT32 when they actually write a SIZE_T or INT64. There are cases where // overwriting these extra 4 bytes corrupts some data (such as a saved register) that leads // to A/V. Whereas previously the JIT64 codegen did not lead to an A/V. - if ((callUser != nullptr) && !varDsc->lvIsParam && !varDsc->lvIsStructField && genActualTypeIsInt(varDsc)) + if ((callUser != nullptr) && !varDsc->lvIsParam && !varDsc->lvIsStructField && genActualTypeIsInt(varDsc) && escapeAddr) { varDsc->lvQuirkToLong = true; JITDUMP("Adding a quirk for the storage size of V%02u of type %s\n", val.LclNum(), diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 08ef18528c1867..cb210f463e86b3 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -66,9 +66,9 @@ void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree) if (compRationalIRForm && (varDsc->lvType != TYP_STRUCT) && !varTypeIsMultiReg(varDsc)) { - // If this is an enregisterable variable that is not marked doNotEnregister, + // If this is an enregisterable variable that is not marked doNotEnregister and not defined via address, // we should only see direct references (not ADDRs). - assert(varDsc->lvDoNotEnregister || tree->OperIs(GT_LCL_VAR, GT_STORE_LCL_VAR)); + assert(varDsc->lvDoNotEnregister || varDsc->lvDefinedViaAddress || tree->OperIs(GT_LCL_VAR, GT_STORE_LCL_VAR)); } if (isUse && !VarSetOps::IsMember(this, fgCurDefSet, varDsc->lvVarIndex)) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 57811f5e107d49..14964b811ea44a 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -678,6 +678,8 @@ const char* getWellKnownArgName(WellKnownArg arg) return "StackArrayLocal"; case WellKnownArg::RuntimeMethodHandle: return "RuntimeMethodHandle"; + case WellKnownArg::AsyncSuspendedIndicator: + return "AsyncSuspendedIndicator"; } return "N/A"; diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 5b894a5312d97c..4a41fcf00cdfb0 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -729,6 +729,9 @@ DEFINE_METHOD(ASYNC_HELPERS, UNSAFE_AWAIT_AWAITER_1, UnsafeAwaitAwaiter, DEFINE_METHOD(ASYNC_HELPERS, CAPTURE_EXECUTION_CONTEXT, CaptureExecutionContext, NoSig) DEFINE_METHOD(ASYNC_HELPERS, RESTORE_EXECUTION_CONTEXT, RestoreExecutionContext, NoSig) DEFINE_METHOD(ASYNC_HELPERS, CAPTURE_CONTINUATION_CONTEXT, CaptureContinuationContext, NoSig) +DEFINE_METHOD(ASYNC_HELPERS, CAPTURE_CONTEXTS, CaptureContexts, NoSig) +DEFINE_METHOD(ASYNC_HELPERS, RESTORE_CONTEXTS, RestoreContexts, NoSig) +DEFINE_METHOD(ASYNC_HELPERS, GET_CURRENT_THREAD, GetCurrentThread, NoSig) DEFINE_CLASS(SPAN_HELPERS, System, SpanHelpers) DEFINE_METHOD(SPAN_HELPERS, MEMSET, Fill, SM_RefByte_Byte_UIntPtr_RetVoid) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index b0e80d386f8f0b..e69a1a71676ac6 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -10258,6 +10258,9 @@ void CEEInfo::getAsyncInfo(CORINFO_ASYNC_INFO* pAsyncInfoOut) pAsyncInfoOut->captureExecutionContextMethHnd = CORINFO_METHOD_HANDLE(CoreLibBinder::GetMethod(METHOD__ASYNC_HELPERS__CAPTURE_EXECUTION_CONTEXT)); pAsyncInfoOut->restoreExecutionContextMethHnd = CORINFO_METHOD_HANDLE(CoreLibBinder::GetMethod(METHOD__ASYNC_HELPERS__RESTORE_EXECUTION_CONTEXT)); pAsyncInfoOut->captureContinuationContextMethHnd = CORINFO_METHOD_HANDLE(CoreLibBinder::GetMethod(METHOD__ASYNC_HELPERS__CAPTURE_CONTINUATION_CONTEXT)); + pAsyncInfoOut->captureContextsMethHnd = CORINFO_METHOD_HANDLE(CoreLibBinder::GetMethod(METHOD__ASYNC_HELPERS__CAPTURE_CONTEXTS)); + pAsyncInfoOut->restoreContextsMethHnd = CORINFO_METHOD_HANDLE(CoreLibBinder::GetMethod(METHOD__ASYNC_HELPERS__RESTORE_CONTEXTS)); + pAsyncInfoOut->getCurrentThreadMethHnd = CORINFO_METHOD_HANDLE(CoreLibBinder::GetMethod(METHOD__ASYNC_HELPERS__GET_CURRENT_THREAD)); EE_TO_JIT_TRANSITION(); } From b9483e7710e2dc289c34cdcb4ea7e26298de1983 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 16 Jul 2025 17:17:35 +0200 Subject: [PATCH 06/28] Attempt at jump threading optimization --- src/coreclr/jit/async.cpp | 99 +++++++++++++++++++++++------------- src/coreclr/jit/async.h | 3 +- src/coreclr/jit/compiler.h | 1 + src/coreclr/jit/compiler.hpp | 23 +++++++-- src/coreclr/jit/gentree.cpp | 28 +++++++++- src/coreclr/jit/gentree.h | 17 +++---- src/coreclr/jit/lclmorph.cpp | 16 ++++++ src/coreclr/jit/lir.cpp | 13 +++++ src/coreclr/jit/lir.h | 1 + src/coreclr/jit/morph.cpp | 10 ++-- 10 files changed, 152 insertions(+), 59 deletions(-) diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index 14680555cf8294..3f204ebb4ba722 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -106,20 +106,13 @@ PhaseStatus Compiler::SaveAsyncContexts() } unsigned suspendedLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Suspended indicator for [%06u]", dspTreeID(call)))); - LclVarDsc* suspendedLclDsc = lvaGetDesc(suspendedLclNum); - suspendedLclDsc->lvType = TYP_UBYTE; - suspendedLclDsc->lvHasLdAddrOp = true; - - call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclAddrNode(suspendedLclNum, 0)).WellKnown(WellKnownArg::AsyncSuspendedIndicator)); - unsigned threadLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Thread for [%06u]", dspTreeID(call)))); unsigned execCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("ExecutionContext for [%06u]", dspTreeID(call)))); unsigned syncCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("SynchronizationContext for [%06u]", dspTreeID(call)))); - // Update the locals in the async information. This needs a new - // AsyncCallInfo since the async call may have been cloned. - AsyncCallInfo newAsyncCallInfo = asyncCallInfo.WithLocals(threadLclNum, syncCtxLclNum); - call->asyncInfo = new (this, CMK_Async) AsyncCallInfo(newAsyncCallInfo); + LclVarDsc* suspendedLclDsc = lvaGetDesc(suspendedLclNum); + suspendedLclDsc->lvType = TYP_UBYTE; + suspendedLclDsc->lvHasLdAddrOp = true; LclVarDsc* threadLclDsc = lvaGetDesc(threadLclNum); threadLclDsc->lvType = TYP_REF; @@ -133,6 +126,12 @@ PhaseStatus Compiler::SaveAsyncContexts() syncCtxLclDsc->lvType = TYP_REF; syncCtxLclDsc->lvHasLdAddrOp = true; + call->asyncInfo->SynchronizationContextLclNum = syncCtxLclNum; + + call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclAddrNode(suspendedLclNum, 0)).WellKnown(WellKnownArg::AsyncSuspendedIndicator)); + call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclVarNode(threadLclNum)).WellKnown(WellKnownArg::AsyncCurrentThread)); + call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclAddrNode(threadLclNum, 0)).WellKnown(WellKnownArg::AsyncCurrentThreadDef)); + JITDUMP("Saving contexts around [%06u], thread = V%02u, ExecutionContext = V%02u, SynchronizationContext = V%02u\n", call->gtTreeID, threadLclNum, execCtxLclNum, syncCtxLclNum); CORINFO_ASYNC_INFO* asyncInfo = eeGetAsyncInfo(); @@ -856,13 +855,6 @@ void AsyncTransformation::CreateLiveSetForSuspension(BasicBlock* const AsyncCallInfo& asyncInfo = call->GetAsyncInfo(); - // These may look live, but they aren't actually live - if (asyncInfo.ThreadObjectLclNum != BAD_VAR_NUM) - { - // This one we define in resumption, but don't model as a proper def (although maybe we ought to) - excludedLocals.AddOrUpdate(asyncInfo.ThreadObjectLclNum, true); - } - if (asyncInfo.SynchronizationContextLclNum != BAD_VAR_NUM) { // This one is only live on the synchronous path, which liveness cannot prove @@ -1173,22 +1165,52 @@ void AsyncTransformation::SetSuspendedIndicator(BasicBlock* block, BasicBlock* c } } -void AsyncTransformation::UpdateThreadObject(BasicBlock* block, GenTreeCall* call) +void AsyncTransformation::UpdateThreadObject(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call) { - const AsyncCallInfo& asyncInfo = call->GetAsyncInfo(); - if (asyncInfo.ThreadObjectLclNum == BAD_VAR_NUM) + CallArg* currentThreadUse = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncCurrentThread); + if (currentThreadUse != nullptr) + { + call->gtArgs.RemoveUnsafe(currentThreadUse); + currentThreadUse->GetNode()->SetUnusedValue(); + } + + CallArg* currentThreadArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncCurrentThreadDef); + if (currentThreadArg == nullptr) { return; } + GenTree* currentThread = currentThreadArg->GetNode(); + + if (!currentThread->IsLclVarAddr() && (!currentThread->OperIs(GT_LCL_VAR) || m_comp->lvaVarAddrExposed(currentThread->AsLclVarCommon()->GetLclNum()))) + { + // We will need to sequence and insert this, so we need to be able to clone it without duplicating side effects. + LIR::Use use(LIR::AsRange(block), ¤tThreadArg->NodeRef(), call); + use.ReplaceWithLclVar(m_comp); + currentThread = use.Def(); + } + GenTreeCall* getCurrentThread = m_comp->gtNewCallNode(CT_USER_FUNC, m_asyncInfo->getCurrentThreadMethHnd, TYP_REF); m_comp->compCurBB = block; m_comp->fgMorphTree(getCurrentThread); - GenTree* store = m_comp->gtNewStoreLclVarNode(asyncInfo.ThreadObjectLclNum, getCurrentThread); - LIR::AsRange(block).InsertAtEnd(LIR::SeqTree(m_comp, store)); + GenTree* storeCurrentThread = m_comp->gtNewStoreValueNode(TYP_UBYTE, m_comp->gtCloneExpr(currentThread), getCurrentThread, GTF_IND_NONFAULTING); + + LIR::AsRange(block).InsertAtEnd(LIR::SeqTree(m_comp, storeCurrentThread)); + + call->gtArgs.RemoveUnsafe(currentThreadArg); + + // Avoid leaving LCL_ADDR around which will DNER the local. + if (currentThread->IsLclVarAddr()) + { + LIR::AsRange(callBlock).Remove(currentThread); + } + else + { + currentThread->SetUnusedValue(); + } } //------------------------------------------------------------------------ @@ -1713,15 +1735,19 @@ void AsyncTransformation::JumpThreadSuspensionCheck(FlowEdge* edge, GenTreeCall* return; } - GenTree* rootNode = LIR::AsRange(blockTarget).FirstNode(); + GenTree* lastNode = LIR::AsRange(blockTarget).LastNode(); + assert(lastNode->OperIs(GT_JTRUE)); - if (!rootNode->OperIs(GT_JTRUE)) + bool isClosed = false; + LIR::ReadOnlyRange jtrueRange = LIR::AsRange(blockTarget).GetTreeRange(lastNode, &isClosed); + + if (!isClosed || (jtrueRange.FirstNode() != LIR::AsRange(blockTarget).FirstNonNopNode())) { JITDUMP(" Block target has other IR\n"); return; } - GenTree* cmp = rootNode->gtGetOp1(); + GenTree* cmp = lastNode->gtGetOp1(); if (!cmp->OperIs(GT_EQ, GT_NE)) { JITDUMP(" Block target is not JTRUE(EQ/NE)\n"); @@ -1796,14 +1822,19 @@ BasicBlock* AsyncTransformation::CreateResumption(BasicBlock* bloc JITDUMP(" Creating resumption " FMT_BB " for state %u\n", resumeBB->bbNum, stateNum); - // Start out by setting the indicator local, if necessary. This will - // usually be optimized by liveness because the resumption path will start - // out with "if (!suspended)", which we can jump thread unless we may have - // exceptional control flow. + // See if we can optimize based on the fact that the suspended indicator + // will be 1 when we rejoin. Usually we can because the resumption path + // will start out with "if (!suspended)". + assert(resumeBB->KindIs(BBJ_ALWAYS)); + JumpThreadSuspensionCheck(resumeBB->GetTargetEdge(), call, 1); + + // Now generate the IR to set the suspension indicator, if necessary. This + // will usually be optimized by liveness because of the jump threading that + // happened above. SetSuspendedIndicator(resumeBB, block, call); // Also the thread object needs to be updated to the current thread. - UpdateThreadObject(resumeBB, call); + UpdateThreadObject(resumeBB, block, call); // We need to restore data before we restore GC pointers, since restoring // the data may also write the GC pointer fields with nulls. @@ -1839,7 +1870,7 @@ BasicBlock* AsyncTransformation::CreateResumption(BasicBlock* bloc if (layout.ExceptionGCDataIndex != UINT_MAX) { - storeResultBB = RethrowExceptionOnResumption(block, remainder, resumeObjectArrLclNum, layout, resumeBB); + storeResultBB = RethrowExceptionOnResumption(block, resumeObjectArrLclNum, layout, resumeBB); } } @@ -1849,9 +1880,6 @@ BasicBlock* AsyncTransformation::CreateResumption(BasicBlock* bloc storeResultBB); } - assert(storeResultBB->KindIs(BBJ_ALWAYS)); - JumpThreadSuspensionCheck(storeResultBB->GetTargetEdge(), call, 1); - return resumeBB; } @@ -2015,7 +2043,6 @@ void AsyncTransformation::RestoreFromGCPointersOnResumption(unsigned // // Parameters: // block - The block containing the async call -// remainder - The block that contains the IR after the (split) async call // resumeObjectArrLclNum - Local that has the continuation object's GC pointers array // layout - Layout information for the continuation object // resumeBB - Basic block to append IR to @@ -2026,7 +2053,6 @@ void AsyncTransformation::RestoreFromGCPointersOnResumption(unsigned // rethrow. // BasicBlock* AsyncTransformation::RethrowExceptionOnResumption(BasicBlock* block, - BasicBlock* remainder, unsigned resumeObjectArrLclNum, const ContinuationLayout& layout, BasicBlock* resumeBB) @@ -2044,6 +2070,7 @@ BasicBlock* AsyncTransformation::RethrowExceptionOnResumption(BasicBlock* FlowEdge* storeResultEdge = m_comp->fgAddRefPred(storeResultBB, resumeBB); assert(resumeBB->KindIs(BBJ_ALWAYS)); + BasicBlock* remainder = resumeBB->GetTarget(); m_comp->fgRemoveRefPred(resumeBB->GetTargetEdge()); resumeBB->SetCond(rethrowEdge, storeResultEdge); diff --git a/src/coreclr/jit/async.h b/src/coreclr/jit/async.h index 9539d7387e4bf3..c19b72464c99ca 100644 --- a/src/coreclr/jit/async.h +++ b/src/coreclr/jit/async.h @@ -111,7 +111,7 @@ class AsyncTransformation unsigned stateNum, const ContinuationLayout& layout); void SetSuspendedIndicator(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call); - void UpdateThreadObject(BasicBlock* block, GenTreeCall* call); + void UpdateThreadObject(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call); void RestoreFromDataOnResumption(unsigned resumeByteArrLclNum, const jitstd::vector& liveLocals, BasicBlock* resumeBB); @@ -119,7 +119,6 @@ class AsyncTransformation const ContinuationLayout& layout, BasicBlock* resumeBB); BasicBlock* RethrowExceptionOnResumption(BasicBlock* block, - BasicBlock* remainder, unsigned resumeObjectArrLclNum, const ContinuationLayout& layout, BasicBlock* resumeBB); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index e486b36bb8514b..849d0b70e520b3 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3741,6 +3741,7 @@ class Compiler GenTreeLclVarCommon* gtCallGetDefinedRetBufLclAddr(GenTreeCall* call); GenTreeLclVarCommon* gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(GenTreeCall* call); + GenTreeLclVarCommon* gtCallGetDefinedAsyncCurrentThreadLclAddr(GenTreeCall* call); //------------------------------------------------------------------------- // Functions to display the trees diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 49f9a2f2bb17e8..f53ea04de802ff 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4745,6 +4745,16 @@ GenTree::VisitResult GenTree::VisitLocalDefs(Compiler* comp, TVisitor visitor) return VisitResult::Abort; } } + + GenTreeLclVarCommon* currentThreadArg = comp->gtCallGetDefinedAsyncCurrentThreadLclAddr(call); + if (currentThreadArg != nullptr) + { + bool isEntire = comp->lvaLclExactSize(currentThreadArg->GetLclNum()) == TARGET_POINTER_SIZE; + if (visitor(LocalDef(currentThreadArg, isEntire, currentThreadArg->GetLclOffs(), TARGET_POINTER_SIZE)) == VisitResult::Abort) + { + return VisitResult::Abort; + } + } } GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(call); @@ -4793,12 +4803,15 @@ GenTree::VisitResult GenTree::VisitLocalDefNodes(Compiler* comp, TVisitor visito if (call->IsAsync()) { GenTreeLclVarCommon* suspendedArg = comp->gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(call); - if (suspendedArg != nullptr) + if ((suspendedArg != nullptr) && (visitor(suspendedArg) == VisitResult::Abort)) { - if (visitor(suspendedArg) == VisitResult::Abort) - { - return VisitResult::Abort; - } + return VisitResult::Abort; + } + + GenTreeLclVarCommon* currentThreadArg = comp->gtCallGetDefinedAsyncCurrentThreadLclAddr(call); + if ((currentThreadArg != nullptr) && (visitor(currentThreadArg) == VisitResult::Abort)) + { + return VisitResult::Abort; } } diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 70093dc11d8996..ef683b1482d004 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -9951,7 +9951,7 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree) } else if (tree->IsAsync()) { - copy->asyncInfo = tree->asyncInfo; + copy->asyncInfo = new (this, CMK_Async) AsyncCallInfo(*tree->asyncInfo); } else if (tree->IsTailPrefixedCall()) { @@ -13222,6 +13222,10 @@ const char* Compiler::gtGetWellKnownArgNameForArgMsg(WellKnownArg arg) return "meth hnd"; case WellKnownArg::AsyncSuspendedIndicator: return "async susp"; + case WellKnownArg::AsyncCurrentThread: + return "async thread"; + case WellKnownArg::AsyncCurrentThreadDef: + return "async thread ="; default: return nullptr; } @@ -19607,7 +19611,7 @@ GenTreeLclVarCommon* Compiler::gtCallGetDefinedRetBufLclAddr(GenTreeCall* call) GenTreeLclVarCommon* Compiler::gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(GenTreeCall* call) { - if (!call->IsAsync()) + if (!call->IsAsync() || !call->GetAsyncInfo().HasSuspensionIndicatorDef) { return nullptr; } @@ -19625,6 +19629,26 @@ GenTreeLclVarCommon* Compiler::gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(Ge return node->AsLclVarCommon(); } +GenTreeLclVarCommon* Compiler::gtCallGetDefinedAsyncCurrentThreadLclAddr(GenTreeCall* call) +{ + if (!call->IsAsync() || !call->GetAsyncInfo().HasCurrentThreadDef) + { + return nullptr; + } + + CallArg* currentThreadArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncCurrentThreadDef); + if (currentThreadArg == nullptr) + { + return nullptr; + } + + GenTree* node = currentThreadArg->GetNode(); + + assert(node->OperIs(GT_LCL_ADDR) && lvaGetDesc(node->AsLclVarCommon())->IsDefinedViaAddress()); + + return node->AsLclVarCommon(); +} + //------------------------------------------------------------------------ // ParseArrayAddress: Rehydrate the array and index expression from ARR_ADDR. // diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 06450ada3411fa..2d96e1ed5cb0be 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4362,16 +4362,9 @@ struct AsyncCallInfo ExecutionContextHandling ExecutionContextHandling = ExecutionContextHandling::None; ContinuationContextHandling ContinuationContextHandling = ContinuationContextHandling::None; bool SaveAndRestoreSynchronizationContextField = false; - unsigned ThreadObjectLclNum = BAD_VAR_NUM; + bool HasSuspensionIndicatorDef = false; + bool HasCurrentThreadDef = false; unsigned SynchronizationContextLclNum = BAD_VAR_NUM; - - AsyncCallInfo WithLocals(unsigned threadObjectLclNum, unsigned synchronizationContextLclNum) const - { - AsyncCallInfo value = *this; - value.ThreadObjectLclNum = threadObjectLclNum; - value.SynchronizationContextLclNum = synchronizationContextLclNum; - return value; - } }; // Return type descriptor of a GT_CALL node. @@ -4645,6 +4638,8 @@ enum class WellKnownArg : unsigned StackArrayLocal, RuntimeMethodHandle, AsyncSuspendedIndicator, + AsyncCurrentThread, + AsyncCurrentThreadDef, }; #ifdef DEBUG @@ -5033,7 +5028,7 @@ struct GenTreeCall final : public GenTree // Only used for unmanaged calls, which cannot be tail-called CorInfoCallConvExtension unmgdCallConv; // Used for async calls - const AsyncCallInfo* asyncInfo; + AsyncCallInfo* asyncInfo; }; #if FEATURE_MULTIREG_RET @@ -5091,7 +5086,7 @@ struct GenTreeCall final : public GenTree #endif } - void SetIsAsync(const AsyncCallInfo* info) + void SetIsAsync(AsyncCallInfo* info) { assert(info != nullptr); gtCallMoreFlags |= GTF_CALL_M_ASYNC; diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index e35fe8b013c2c3..4f7d10fe87359a 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1499,6 +1499,7 @@ class LocalAddressVisitor final : public GenTreeVisitor if ((callUser != nullptr) && callUser->IsAsync() && m_compiler->IsValidLclAddr(lclNum, val.Offset())) { CallArg* suspendedArg = callUser->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); + CallArg* currentThreadArg = callUser->gtArgs.FindWellKnownArg(WellKnownArg::AsyncCurrentThreadDef); if ((suspendedArg != nullptr) && (val.Node() == suspendedArg->GetNode())) { varDsc->SetDefinedViaAddress(true); @@ -1509,6 +1510,21 @@ class LocalAddressVisitor final : public GenTreeVisitor { defFlag |= GTF_VAR_USEASG; } + + callUser->asyncInfo->HasSuspensionIndicatorDef = true; + } + else if ((currentThreadArg != nullptr) && (val.Node() == currentThreadArg->GetNode())) + { + varDsc->SetDefinedViaAddress(true); + escapeAddr = false; + defFlag = GTF_VAR_DEF; + + if ((val.Offset() != 0) || (varDsc->lvExactSize() != TARGET_POINTER_SIZE)) + { + defFlag |= GTF_VAR_USEASG; + } + + callUser->asyncInfo->HasCurrentThreadDef = true; } } diff --git a/src/coreclr/jit/lir.cpp b/src/coreclr/jit/lir.cpp index 9592d4c662d84c..aa86a882273874 100644 --- a/src/coreclr/jit/lir.cpp +++ b/src/coreclr/jit/lir.cpp @@ -355,6 +355,19 @@ GenTree* LIR::ReadOnlyRange::LastNode() const return m_lastNode; } +GenTree* LIR::ReadOnlyRange::FirstNonNopNode() const +{ + for (GenTree* cur = m_firstNode; cur != nullptr; cur = cur->gtNext) + { + if (!cur->OperIs(GT_NOP, GT_IL_OFFSET)) + { + return cur; + } + } + + return nullptr; +} + //------------------------------------------------------------------------ // LIR::ReadOnlyRange::IsEmpty: Returns true if the range is empty; false // otherwise. diff --git a/src/coreclr/jit/lir.h b/src/coreclr/jit/lir.h index 99d011ea32e9bc..cf094786817b23 100644 --- a/src/coreclr/jit/lir.h +++ b/src/coreclr/jit/lir.h @@ -212,6 +212,7 @@ class LIR final GenTree* FirstNode() const; GenTree* LastNode() const; + GenTree* FirstNonNopNode() const; bool IsEmpty() const; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 14964b811ea44a..83f6e97b14e51a 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -680,6 +680,10 @@ const char* getWellKnownArgName(WellKnownArg arg) return "RuntimeMethodHandle"; case WellKnownArg::AsyncSuspendedIndicator: return "AsyncSuspendedIndicator"; + case WellKnownArg::AsyncCurrentThread: + return "AsyncCurrentThread"; + case WellKnownArg::AsyncCurrentThreadDef: + return "AsyncCurrentThreadDef"; } return "N/A"; @@ -1921,10 +1925,10 @@ void CallArgs::DetermineABIInfo(Compiler* comp, GenTreeCall* call) for (CallArg& arg : Args()) { - if (arg.GetWellKnownArg() == WellKnownArg::AsyncSuspendedIndicator) + if ((arg.GetWellKnownArg() == WellKnownArg::AsyncSuspendedIndicator) || (arg.GetWellKnownArg() == WellKnownArg::AsyncCurrentThread) || + (arg.GetWellKnownArg() == WellKnownArg::AsyncCurrentThreadDef)) { - // Represents a definition of a local indicating whether the call - // suspended or not. Removed by async transformation. + // Represents definitions of locals. Expanded out by async transformation. arg.AbiInfo = ABIPassingInformation(comp, 0); continue; } From 8301152c089c9222add9b9d689230abd11f2bbf0 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 16 Jul 2025 17:26:57 +0200 Subject: [PATCH 07/28] Drop jump threading opt for now --- .../CompilerServices/AsyncHelpers.CoreCLR.cs | 6 +- src/coreclr/jit/async.cpp | 143 +----------------- src/coreclr/jit/async.h | 3 - src/coreclr/jit/compiler.h | 1 - src/coreclr/jit/compiler.hpp | 16 -- src/coreclr/jit/gentree.cpp | 24 --- src/coreclr/jit/gentree.h | 2 - src/coreclr/jit/lclmorph.cpp | 14 -- src/coreclr/jit/morph.cpp | 7 +- 9 files changed, 5 insertions(+), 211 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs index 9f187aea657c22..8d37fe5c4de9b0 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs @@ -602,17 +602,17 @@ private static void RestoreExecutionContext(ExecutionContext? previousExecCtx) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static void CaptureContexts(out Thread currentThread, out ExecutionContext? execCtx, out SynchronizationContext? syncCtx) + private static void CaptureContexts(out ExecutionContext? execCtx, out SynchronizationContext? syncCtx) { Thread thread = Thread.CurrentThreadAssumedInitialized; - currentThread = thread; execCtx = thread._executionContext; syncCtx = thread._synchronizationContext; } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private static void RestoreContexts(bool suspended, Thread thread, ExecutionContext? previousExecCtx, SynchronizationContext? previousSyncCtx) + private static void RestoreContexts(bool suspended, ExecutionContext? previousExecCtx, SynchronizationContext? previousSyncCtx) { + Thread thread = Thread.CurrentThreadAssumedInitialized; if (!suspended && previousSyncCtx != thread._synchronizationContext) { thread._synchronizationContext = previousSyncCtx; diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index 3f204ebb4ba722..5724c08309c913 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -106,7 +106,6 @@ PhaseStatus Compiler::SaveAsyncContexts() } unsigned suspendedLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Suspended indicator for [%06u]", dspTreeID(call)))); - unsigned threadLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Thread for [%06u]", dspTreeID(call)))); unsigned execCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("ExecutionContext for [%06u]", dspTreeID(call)))); unsigned syncCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("SynchronizationContext for [%06u]", dspTreeID(call)))); @@ -114,10 +113,6 @@ PhaseStatus Compiler::SaveAsyncContexts() suspendedLclDsc->lvType = TYP_UBYTE; suspendedLclDsc->lvHasLdAddrOp = true; - LclVarDsc* threadLclDsc = lvaGetDesc(threadLclNum); - threadLclDsc->lvType = TYP_REF; - threadLclDsc->lvHasLdAddrOp = true; - LclVarDsc* execCtxLclDsc = lvaGetDesc(execCtxLclNum); execCtxLclDsc->lvType = TYP_REF; execCtxLclDsc->lvHasLdAddrOp = true; @@ -129,17 +124,14 @@ PhaseStatus Compiler::SaveAsyncContexts() call->asyncInfo->SynchronizationContextLclNum = syncCtxLclNum; call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclAddrNode(suspendedLclNum, 0)).WellKnown(WellKnownArg::AsyncSuspendedIndicator)); - call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclVarNode(threadLclNum)).WellKnown(WellKnownArg::AsyncCurrentThread)); - call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclAddrNode(threadLclNum, 0)).WellKnown(WellKnownArg::AsyncCurrentThreadDef)); - JITDUMP("Saving contexts around [%06u], thread = V%02u, ExecutionContext = V%02u, SynchronizationContext = V%02u\n", call->gtTreeID, threadLclNum, execCtxLclNum, syncCtxLclNum); + JITDUMP("Saving contexts around [%06u], ExecutionContext = V%02u, SynchronizationContext = V%02u\n", call->gtTreeID, execCtxLclNum, syncCtxLclNum); CORINFO_ASYNC_INFO* asyncInfo = eeGetAsyncInfo(); GenTreeCall* capture = gtNewCallNode(CT_USER_FUNC, asyncInfo->captureContextsMethHnd, TYP_VOID); capture->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclAddrNode(syncCtxLclNum, 0))); capture->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclAddrNode(execCtxLclNum, 0))); - capture->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclAddrNode(threadLclNum, 0))); CORINFO_CALL_INFO callInfo = {}; callInfo.hMethod = capture->gtCallMethHnd; @@ -177,7 +169,6 @@ PhaseStatus Compiler::SaveAsyncContexts() GenTreeCall* restore = gtNewCallNode(CT_USER_FUNC, asyncInfo->restoreContextsMethHnd, TYP_VOID); restore->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclVarNode(syncCtxLclNum))); restore->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclVarNode(execCtxLclNum))); - restore->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclVarNode(threadLclNum))); restore->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclVarNode(suspendedLclNum))); callInfo = {}; @@ -1165,54 +1156,6 @@ void AsyncTransformation::SetSuspendedIndicator(BasicBlock* block, BasicBlock* c } } -void AsyncTransformation::UpdateThreadObject(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call) -{ - CallArg* currentThreadUse = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncCurrentThread); - if (currentThreadUse != nullptr) - { - call->gtArgs.RemoveUnsafe(currentThreadUse); - currentThreadUse->GetNode()->SetUnusedValue(); - } - - CallArg* currentThreadArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncCurrentThreadDef); - if (currentThreadArg == nullptr) - { - return; - } - - GenTree* currentThread = currentThreadArg->GetNode(); - - if (!currentThread->IsLclVarAddr() && (!currentThread->OperIs(GT_LCL_VAR) || m_comp->lvaVarAddrExposed(currentThread->AsLclVarCommon()->GetLclNum()))) - { - // We will need to sequence and insert this, so we need to be able to clone it without duplicating side effects. - LIR::Use use(LIR::AsRange(block), ¤tThreadArg->NodeRef(), call); - use.ReplaceWithLclVar(m_comp); - currentThread = use.Def(); - } - - GenTreeCall* getCurrentThread = - m_comp->gtNewCallNode(CT_USER_FUNC, m_asyncInfo->getCurrentThreadMethHnd, TYP_REF); - - m_comp->compCurBB = block; - m_comp->fgMorphTree(getCurrentThread); - - GenTree* storeCurrentThread = m_comp->gtNewStoreValueNode(TYP_UBYTE, m_comp->gtCloneExpr(currentThread), getCurrentThread, GTF_IND_NONFAULTING); - - LIR::AsRange(block).InsertAtEnd(LIR::SeqTree(m_comp, storeCurrentThread)); - - call->gtArgs.RemoveUnsafe(currentThreadArg); - - // Avoid leaving LCL_ADDR around which will DNER the local. - if (currentThread->IsLclVarAddr()) - { - LIR::AsRange(callBlock).Remove(currentThread); - } - else - { - currentThread->SetUnusedValue(); - } -} - //------------------------------------------------------------------------ // AsyncTransformation::CanonicalizeCallDefinition: // Put the call definition in a canonical form. This ensures that either the @@ -1705,78 +1648,6 @@ void AsyncTransformation::CreateCheckAndSuspendAfterCall(BasicBlock* block->GetTrueEdge()->setLikelihood(0); block->GetFalseEdge()->setLikelihood(1); - - JumpThreadSuspensionCheck(block->GetFalseEdge(), call, 0); -} - -void AsyncTransformation::JumpThreadSuspensionCheck(FlowEdge* edge, GenTreeCall* call, uint8_t suspendedValue) -{ - JITDUMP(" Seeing if we can jump thread a check for suspension in " FMT_BB " -> " FMT_BB " with known value %d\n", edge->getSourceBlock()->bbNum, edge->getDestinationBlock()->bbNum, suspendedValue); - - CallArg* suspendedArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); - if (suspendedArg == nullptr) - { - JITDUMP(" No async suspension indicator argument found\n"); - return; - } - - GenTree* suspended = suspendedArg->GetNode(); - if (!suspended->IsLclVarAddr() || (suspended->AsLclVarCommon()->GetLclOffs() != 0) || (m_comp->lvaGetDesc(suspended->AsLclVarCommon())->lvExactSize() != 1)) - { - JITDUMP(" Async suspension indicator is not of right shape\n"); - return; - } - - unsigned suspendedLclNum = suspended->AsLclVarCommon()->GetLclNum(); - BasicBlock* blockTarget = edge->getDestinationBlock(); - if (!blockTarget->KindIs(BBJ_COND)) - { - JITDUMP(" Block target is not BBJ_COND\n"); - return; - } - - GenTree* lastNode = LIR::AsRange(blockTarget).LastNode(); - assert(lastNode->OperIs(GT_JTRUE)); - - bool isClosed = false; - LIR::ReadOnlyRange jtrueRange = LIR::AsRange(blockTarget).GetTreeRange(lastNode, &isClosed); - - if (!isClosed || (jtrueRange.FirstNode() != LIR::AsRange(blockTarget).FirstNonNopNode())) - { - JITDUMP(" Block target has other IR\n"); - return; - } - - GenTree* cmp = lastNode->gtGetOp1(); - if (!cmp->OperIs(GT_EQ, GT_NE)) - { - JITDUMP(" Block target is not JTRUE(EQ/NE)\n"); - return; - } - - GenTree* op1 = cmp->gtGetOp1(); - GenTree* op2 = cmp->gtGetOp2(); - if (!op2->IsCnsIntOrI()) - { - std::swap(op1, op2); - } - - if (!op1->OperIs(GT_LCL_VAR) || (op1->AsLclVar()->GetLclNum() != suspendedLclNum)) - { - JITDUMP(" Op1 is not suspended indicator\n"); - return; - } - - if (!op2->IsIntegralConst(0)) - { - JITDUMP(" Op2 is not constant 0\n"); - return; - } - - BasicBlock* actualTarget = (cmp->OperIs(GT_EQ) == (suspendedValue == 0)) ? blockTarget->GetTrueTarget() : blockTarget->GetFalseTarget(); - JITDUMP(" We can jump thread " FMT_BB " -> " FMT_BB " into " FMT_BB " -> " FMT_BB "\n", edge->getSourceBlock()->bbNum, edge->getDestinationBlock()->bbNum, edge->getSourceBlock()->bbNum, actualTarget->bbNum); - - m_comp->fgRedirectEdge(edge, actualTarget); } //------------------------------------------------------------------------ @@ -1822,20 +1693,8 @@ BasicBlock* AsyncTransformation::CreateResumption(BasicBlock* bloc JITDUMP(" Creating resumption " FMT_BB " for state %u\n", resumeBB->bbNum, stateNum); - // See if we can optimize based on the fact that the suspended indicator - // will be 1 when we rejoin. Usually we can because the resumption path - // will start out with "if (!suspended)". - assert(resumeBB->KindIs(BBJ_ALWAYS)); - JumpThreadSuspensionCheck(resumeBB->GetTargetEdge(), call, 1); - - // Now generate the IR to set the suspension indicator, if necessary. This - // will usually be optimized by liveness because of the jump threading that - // happened above. SetSuspendedIndicator(resumeBB, block, call); - // Also the thread object needs to be updated to the current thread. - UpdateThreadObject(resumeBB, block, call); - // We need to restore data before we restore GC pointers, since restoring // the data may also write the GC pointer fields with nulls. unsigned resumeByteArrLclNum = BAD_VAR_NUM; diff --git a/src/coreclr/jit/async.h b/src/coreclr/jit/async.h index c19b72464c99ca..8820b1ac9f7568 100644 --- a/src/coreclr/jit/async.h +++ b/src/coreclr/jit/async.h @@ -102,8 +102,6 @@ class AsyncTransformation AsyncLiveness& life, BasicBlock* suspendBB, BasicBlock** remainder); - void JumpThreadSuspensionCheck(FlowEdge* edge, GenTreeCall* call, uint8_t suspendedValue); - BasicBlock* CreateResumption(BasicBlock* block, BasicBlock* remainder, GenTreeCall* call, @@ -111,7 +109,6 @@ class AsyncTransformation unsigned stateNum, const ContinuationLayout& layout); void SetSuspendedIndicator(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call); - void UpdateThreadObject(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call); void RestoreFromDataOnResumption(unsigned resumeByteArrLclNum, const jitstd::vector& liveLocals, BasicBlock* resumeBB); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 849d0b70e520b3..e486b36bb8514b 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -3741,7 +3741,6 @@ class Compiler GenTreeLclVarCommon* gtCallGetDefinedRetBufLclAddr(GenTreeCall* call); GenTreeLclVarCommon* gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(GenTreeCall* call); - GenTreeLclVarCommon* gtCallGetDefinedAsyncCurrentThreadLclAddr(GenTreeCall* call); //------------------------------------------------------------------------- // Functions to display the trees diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index f53ea04de802ff..f2597987b2f108 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4745,16 +4745,6 @@ GenTree::VisitResult GenTree::VisitLocalDefs(Compiler* comp, TVisitor visitor) return VisitResult::Abort; } } - - GenTreeLclVarCommon* currentThreadArg = comp->gtCallGetDefinedAsyncCurrentThreadLclAddr(call); - if (currentThreadArg != nullptr) - { - bool isEntire = comp->lvaLclExactSize(currentThreadArg->GetLclNum()) == TARGET_POINTER_SIZE; - if (visitor(LocalDef(currentThreadArg, isEntire, currentThreadArg->GetLclOffs(), TARGET_POINTER_SIZE)) == VisitResult::Abort) - { - return VisitResult::Abort; - } - } } GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(call); @@ -4807,12 +4797,6 @@ GenTree::VisitResult GenTree::VisitLocalDefNodes(Compiler* comp, TVisitor visito { return VisitResult::Abort; } - - GenTreeLclVarCommon* currentThreadArg = comp->gtCallGetDefinedAsyncCurrentThreadLclAddr(call); - if ((currentThreadArg != nullptr) && (visitor(currentThreadArg) == VisitResult::Abort)) - { - return VisitResult::Abort; - } } GenTreeLclVarCommon* lclAddr = comp->gtCallGetDefinedRetBufLclAddr(call); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index ef683b1482d004..23de5534f351aa 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -13222,10 +13222,6 @@ const char* Compiler::gtGetWellKnownArgNameForArgMsg(WellKnownArg arg) return "meth hnd"; case WellKnownArg::AsyncSuspendedIndicator: return "async susp"; - case WellKnownArg::AsyncCurrentThread: - return "async thread"; - case WellKnownArg::AsyncCurrentThreadDef: - return "async thread ="; default: return nullptr; } @@ -19629,26 +19625,6 @@ GenTreeLclVarCommon* Compiler::gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(Ge return node->AsLclVarCommon(); } -GenTreeLclVarCommon* Compiler::gtCallGetDefinedAsyncCurrentThreadLclAddr(GenTreeCall* call) -{ - if (!call->IsAsync() || !call->GetAsyncInfo().HasCurrentThreadDef) - { - return nullptr; - } - - CallArg* currentThreadArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncCurrentThreadDef); - if (currentThreadArg == nullptr) - { - return nullptr; - } - - GenTree* node = currentThreadArg->GetNode(); - - assert(node->OperIs(GT_LCL_ADDR) && lvaGetDesc(node->AsLclVarCommon())->IsDefinedViaAddress()); - - return node->AsLclVarCommon(); -} - //------------------------------------------------------------------------ // ParseArrayAddress: Rehydrate the array and index expression from ARR_ADDR. // diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 2d96e1ed5cb0be..b8c9abac017ec1 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4638,8 +4638,6 @@ enum class WellKnownArg : unsigned StackArrayLocal, RuntimeMethodHandle, AsyncSuspendedIndicator, - AsyncCurrentThread, - AsyncCurrentThreadDef, }; #ifdef DEBUG diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 4f7d10fe87359a..4c51ff02a54ee0 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1499,7 +1499,6 @@ class LocalAddressVisitor final : public GenTreeVisitor if ((callUser != nullptr) && callUser->IsAsync() && m_compiler->IsValidLclAddr(lclNum, val.Offset())) { CallArg* suspendedArg = callUser->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); - CallArg* currentThreadArg = callUser->gtArgs.FindWellKnownArg(WellKnownArg::AsyncCurrentThreadDef); if ((suspendedArg != nullptr) && (val.Node() == suspendedArg->GetNode())) { varDsc->SetDefinedViaAddress(true); @@ -1513,19 +1512,6 @@ class LocalAddressVisitor final : public GenTreeVisitor callUser->asyncInfo->HasSuspensionIndicatorDef = true; } - else if ((currentThreadArg != nullptr) && (val.Node() == currentThreadArg->GetNode())) - { - varDsc->SetDefinedViaAddress(true); - escapeAddr = false; - defFlag = GTF_VAR_DEF; - - if ((val.Offset() != 0) || (varDsc->lvExactSize() != TARGET_POINTER_SIZE)) - { - defFlag |= GTF_VAR_USEASG; - } - - callUser->asyncInfo->HasCurrentThreadDef = true; - } } if (escapeAddr) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 83f6e97b14e51a..93cad7ba7ef510 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -680,10 +680,6 @@ const char* getWellKnownArgName(WellKnownArg arg) return "RuntimeMethodHandle"; case WellKnownArg::AsyncSuspendedIndicator: return "AsyncSuspendedIndicator"; - case WellKnownArg::AsyncCurrentThread: - return "AsyncCurrentThread"; - case WellKnownArg::AsyncCurrentThreadDef: - return "AsyncCurrentThreadDef"; } return "N/A"; @@ -1925,8 +1921,7 @@ void CallArgs::DetermineABIInfo(Compiler* comp, GenTreeCall* call) for (CallArg& arg : Args()) { - if ((arg.GetWellKnownArg() == WellKnownArg::AsyncSuspendedIndicator) || (arg.GetWellKnownArg() == WellKnownArg::AsyncCurrentThread) || - (arg.GetWellKnownArg() == WellKnownArg::AsyncCurrentThreadDef)) + if (arg.GetWellKnownArg() == WellKnownArg::AsyncSuspendedIndicator) { // Represents definitions of locals. Expanded out by async transformation. arg.AbiInfo = ABIPassingInformation(comp, 0); From 4a88fe8d88c17821496e4b19befd9fd30e9e2f72 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 16 Jul 2025 17:27:29 +0200 Subject: [PATCH 08/28] Run jit-format --- src/coreclr/jit/async.cpp | 47 +++++++++++++++++++------------ src/coreclr/jit/async.h | 42 +++++++++++++-------------- src/coreclr/jit/gentree.h | 12 ++++---- src/coreclr/jit/importercalls.cpp | 4 +-- src/coreclr/jit/lclmorph.cpp | 9 +++--- src/coreclr/jit/liveness.cpp | 3 +- 6 files changed, 65 insertions(+), 52 deletions(-) diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index 5724c08309c913..752519680cd10b 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -98,38 +98,44 @@ PhaseStatus Compiler::SaveAsyncContexts() // Currently we always expect that ExecutionContext and // SynchronizationContext correlate about their save/restore // behavior. - assert((asyncCallInfo.ExecutionContextHandling == ExecutionContextHandling::SaveAndRestore) == asyncCallInfo.SaveAndRestoreSynchronizationContextField); + assert((asyncCallInfo.ExecutionContextHandling == ExecutionContextHandling::SaveAndRestore) == + asyncCallInfo.SaveAndRestoreSynchronizationContextField); if (asyncCallInfo.ExecutionContextHandling != ExecutionContextHandling::SaveAndRestore) { continue; } - unsigned suspendedLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("Suspended indicator for [%06u]", dspTreeID(call)))); - unsigned execCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("ExecutionContext for [%06u]", dspTreeID(call)))); - unsigned syncCtxLclNum = lvaGrabTemp(false DEBUGARG(printfAlloc("SynchronizationContext for [%06u]", dspTreeID(call)))); + unsigned suspendedLclNum = + lvaGrabTemp(false DEBUGARG(printfAlloc("Suspended indicator for [%06u]", dspTreeID(call)))); + unsigned execCtxLclNum = + lvaGrabTemp(false DEBUGARG(printfAlloc("ExecutionContext for [%06u]", dspTreeID(call)))); + unsigned syncCtxLclNum = + lvaGrabTemp(false DEBUGARG(printfAlloc("SynchronizationContext for [%06u]", dspTreeID(call)))); - LclVarDsc* suspendedLclDsc = lvaGetDesc(suspendedLclNum); - suspendedLclDsc->lvType = TYP_UBYTE; + LclVarDsc* suspendedLclDsc = lvaGetDesc(suspendedLclNum); + suspendedLclDsc->lvType = TYP_UBYTE; suspendedLclDsc->lvHasLdAddrOp = true; - LclVarDsc* execCtxLclDsc = lvaGetDesc(execCtxLclNum); - execCtxLclDsc->lvType = TYP_REF; + LclVarDsc* execCtxLclDsc = lvaGetDesc(execCtxLclNum); + execCtxLclDsc->lvType = TYP_REF; execCtxLclDsc->lvHasLdAddrOp = true; - LclVarDsc* syncCtxLclDsc = lvaGetDesc(syncCtxLclNum); - syncCtxLclDsc->lvType = TYP_REF; + LclVarDsc* syncCtxLclDsc = lvaGetDesc(syncCtxLclNum); + syncCtxLclDsc->lvType = TYP_REF; syncCtxLclDsc->lvHasLdAddrOp = true; call->asyncInfo->SynchronizationContextLclNum = syncCtxLclNum; - call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclAddrNode(suspendedLclNum, 0)).WellKnown(WellKnownArg::AsyncSuspendedIndicator)); + call->gtArgs.PushBack(this, NewCallArg::Primitive(gtNewLclAddrNode(suspendedLclNum, 0)) + .WellKnown(WellKnownArg::AsyncSuspendedIndicator)); - JITDUMP("Saving contexts around [%06u], ExecutionContext = V%02u, SynchronizationContext = V%02u\n", call->gtTreeID, execCtxLclNum, syncCtxLclNum); + JITDUMP("Saving contexts around [%06u], ExecutionContext = V%02u, SynchronizationContext = V%02u\n", + call->gtTreeID, execCtxLclNum, syncCtxLclNum); CORINFO_ASYNC_INFO* asyncInfo = eeGetAsyncInfo(); - GenTreeCall* capture = gtNewCallNode(CT_USER_FUNC, asyncInfo->captureContextsMethHnd, TYP_VOID); + GenTreeCall* capture = gtNewCallNode(CT_USER_FUNC, asyncInfo->captureContextsMethHnd, TYP_VOID); capture->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclAddrNode(syncCtxLclNum, 0))); capture->gtArgs.PushFront(this, NewCallArg::Primitive(gtNewLclAddrNode(execCtxLclNum, 0))); @@ -840,7 +846,7 @@ void AsyncTransformation::CreateLiveSetForSuspension(BasicBlock* excludedLocals.AddOrUpdate(def.Def->GetLclNum(), true); } return GenTree::VisitResult::Continue; - }; + }; call->VisitLocalDefs(m_comp, visitDef); @@ -852,7 +858,9 @@ void AsyncTransformation::CreateLiveSetForSuspension(BasicBlock* excludedLocals.AddOrUpdate(asyncInfo.SynchronizationContextLclNum, true); } - life.GetLiveLocals(liveLocals, [&](unsigned lclNum) { return !excludedLocals.Contains(lclNum); }); + life.GetLiveLocals(liveLocals, [&](unsigned lclNum) { + return !excludedLocals.Contains(lclNum); + }); LiftLIREdges(block, defs, liveLocals); #ifdef DEBUG @@ -1113,7 +1121,8 @@ void AsyncTransformation::ClearSuspendedIndicator(BasicBlock* block, GenTreeCall } GenTree* suspended = suspendedArg->GetNode(); - if (!suspended->IsLclVarAddr() && (!suspended->OperIs(GT_LCL_VAR) || m_comp->lvaVarAddrExposed(suspended->AsLclVarCommon()->GetLclNum()))) + if (!suspended->IsLclVarAddr() && + (!suspended->OperIs(GT_LCL_VAR) || m_comp->lvaVarAddrExposed(suspended->AsLclVarCommon()->GetLclNum()))) { // We will need a second use of this, so spill to a local LIR::Use use(LIR::AsRange(block), &suspendedArg->NodeRef(), call); @@ -1122,7 +1131,8 @@ void AsyncTransformation::ClearSuspendedIndicator(BasicBlock* block, GenTreeCall } GenTree* value = m_comp->gtNewIconNode(0); - GenTree* storeSuspended = m_comp->gtNewStoreValueNode(TYP_UBYTE, m_comp->gtCloneExpr(suspended), value, GTF_IND_NONFAULTING); + GenTree* storeSuspended = + m_comp->gtNewStoreValueNode(TYP_UBYTE, m_comp->gtCloneExpr(suspended), value, GTF_IND_NONFAULTING); LIR::AsRange(block).InsertBefore(call, LIR::SeqTree(m_comp, storeSuspended)); } @@ -1139,7 +1149,8 @@ void AsyncTransformation::SetSuspendedIndicator(BasicBlock* block, BasicBlock* c assert(suspended->IsLclVarAddr() || suspended->OperIs(GT_LCL_VAR)); // Ensured by ClearSuspendedIndicator GenTree* value = m_comp->gtNewIconNode(1); - GenTree* storeSuspended = m_comp->gtNewStoreValueNode(TYP_UBYTE, m_comp->gtCloneExpr(suspended), value, GTF_IND_NONFAULTING); + GenTree* storeSuspended = + m_comp->gtNewStoreValueNode(TYP_UBYTE, m_comp->gtCloneExpr(suspended), value, GTF_IND_NONFAULTING); LIR::AsRange(block).InsertAtEnd(LIR::SeqTree(m_comp, storeSuspended)); diff --git a/src/coreclr/jit/async.h b/src/coreclr/jit/async.h index 8820b1ac9f7568..7cfdb4d3aa86d7 100644 --- a/src/coreclr/jit/async.h +++ b/src/coreclr/jit/async.h @@ -102,29 +102,29 @@ class AsyncTransformation AsyncLiveness& life, BasicBlock* suspendBB, BasicBlock** remainder); - BasicBlock* CreateResumption(BasicBlock* block, - BasicBlock* remainder, - GenTreeCall* call, - const CallDefinitionInfo& callDefInfo, - unsigned stateNum, - const ContinuationLayout& layout); - void SetSuspendedIndicator(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call); - void RestoreFromDataOnResumption(unsigned resumeByteArrLclNum, - const jitstd::vector& liveLocals, - BasicBlock* resumeBB); - void RestoreFromGCPointersOnResumption(unsigned resumeObjectArrLclNum, - const ContinuationLayout& layout, - BasicBlock* resumeBB); - BasicBlock* RethrowExceptionOnResumption(BasicBlock* block, + BasicBlock* CreateResumption(BasicBlock* block, + BasicBlock* remainder, + GenTreeCall* call, + const CallDefinitionInfo& callDefInfo, + unsigned stateNum, + const ContinuationLayout& layout); + void SetSuspendedIndicator(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call); + void RestoreFromDataOnResumption(unsigned resumeByteArrLclNum, + const jitstd::vector& liveLocals, + BasicBlock* resumeBB); + void RestoreFromGCPointersOnResumption(unsigned resumeObjectArrLclNum, + const ContinuationLayout& layout, + BasicBlock* resumeBB); + BasicBlock* RethrowExceptionOnResumption(BasicBlock* block, + unsigned resumeObjectArrLclNum, + const ContinuationLayout& layout, + BasicBlock* resumeBB); + void CopyReturnValueOnResumption(GenTreeCall* call, + const CallDefinitionInfo& callDefInfo, + unsigned resumeByteArrLclNum, unsigned resumeObjectArrLclNum, const ContinuationLayout& layout, - BasicBlock* resumeBB); - void CopyReturnValueOnResumption(GenTreeCall* call, - const CallDefinitionInfo& callDefInfo, - unsigned resumeByteArrLclNum, - unsigned resumeObjectArrLclNum, - const ContinuationLayout& layout, - BasicBlock* storeResultBB); + BasicBlock* storeResultBB); GenTreeIndir* LoadFromOffset(GenTree* base, unsigned offset, diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index b8c9abac017ec1..375d40f8c222c2 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4359,12 +4359,12 @@ enum class ContinuationContextHandling // Additional async call info. struct AsyncCallInfo { - ExecutionContextHandling ExecutionContextHandling = ExecutionContextHandling::None; - ContinuationContextHandling ContinuationContextHandling = ContinuationContextHandling::None; - bool SaveAndRestoreSynchronizationContextField = false; - bool HasSuspensionIndicatorDef = false; - bool HasCurrentThreadDef = false; - unsigned SynchronizationContextLclNum = BAD_VAR_NUM; + ExecutionContextHandling ExecutionContextHandling = ExecutionContextHandling::None; + ContinuationContextHandling ContinuationContextHandling = ContinuationContextHandling::None; + bool SaveAndRestoreSynchronizationContextField = false; + bool HasSuspensionIndicatorDef = false; + bool HasCurrentThreadDef = false; + unsigned SynchronizationContextLclNum = BAD_VAR_NUM; }; // Return type descriptor of a GT_CALL node. diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index cd9050019f64a5..e477121b33dc0b 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -705,7 +705,7 @@ var_types Compiler::impImportCall(OPCODE opcode, { JITDUMP("Call is an async task await\n"); - asyncInfo.ExecutionContextHandling = ExecutionContextHandling::SaveAndRestore; + asyncInfo.ExecutionContextHandling = ExecutionContextHandling::SaveAndRestore; asyncInfo.SaveAndRestoreSynchronizationContextField = true; if ((prefixFlags & PREFIX_TASK_AWAIT_CONTINUE_ON_CAPTURED_CONTEXT) != 0) @@ -739,7 +739,7 @@ var_types Compiler::impImportCall(OPCODE opcode, // context. We do not do that optimization yet. if (tailCallFlags != 0) { - asyncInfo.ExecutionContextHandling = ExecutionContextHandling::None; + asyncInfo.ExecutionContextHandling = ExecutionContextHandling::None; asyncInfo.SaveAndRestoreSynchronizationContextField = false; } diff --git a/src/coreclr/jit/lclmorph.cpp b/src/coreclr/jit/lclmorph.cpp index 4c51ff02a54ee0..7412966fb4232a 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1462,8 +1462,8 @@ class LocalAddressVisitor final : public GenTreeVisitor unsigned lclNum = val.LclNum(); LclVarDsc* varDsc = m_compiler->lvaGetDesc(lclNum); - GenTreeFlags defFlag = GTF_EMPTY; - GenTreeCall* callUser = (user != nullptr) && user->IsCall() ? user->AsCall() : nullptr; + GenTreeFlags defFlag = GTF_EMPTY; + GenTreeCall* callUser = (user != nullptr) && user->IsCall() ? user->AsCall() : nullptr; bool escapeAddr = true; if (m_compiler->opts.compJitOptimizeStructHiddenBuffer && (callUser != nullptr) && m_compiler->IsValidLclAddr(lclNum, val.Offset())) @@ -1503,7 +1503,7 @@ class LocalAddressVisitor final : public GenTreeVisitor { varDsc->SetDefinedViaAddress(true); escapeAddr = false; - defFlag = GTF_VAR_DEF; + defFlag = GTF_VAR_DEF; if ((val.Offset() != 0) || (varDsc->lvExactSize() != 1)) { @@ -1534,7 +1534,8 @@ class LocalAddressVisitor final : public GenTreeVisitor // a ByRef to an INT32 when they actually write a SIZE_T or INT64. There are cases where // overwriting these extra 4 bytes corrupts some data (such as a saved register) that leads // to A/V. Whereas previously the JIT64 codegen did not lead to an A/V. - if ((callUser != nullptr) && !varDsc->lvIsParam && !varDsc->lvIsStructField && genActualTypeIsInt(varDsc) && escapeAddr) + if ((callUser != nullptr) && !varDsc->lvIsParam && !varDsc->lvIsStructField && genActualTypeIsInt(varDsc) && + escapeAddr) { varDsc->lvQuirkToLong = true; JITDUMP("Adding a quirk for the storage size of V%02u of type %s\n", val.LclNum(), diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index cb210f463e86b3..617067e7aeefa8 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -68,7 +68,8 @@ void Compiler::fgMarkUseDef(GenTreeLclVarCommon* tree) { // If this is an enregisterable variable that is not marked doNotEnregister and not defined via address, // we should only see direct references (not ADDRs). - assert(varDsc->lvDoNotEnregister || varDsc->lvDefinedViaAddress || tree->OperIs(GT_LCL_VAR, GT_STORE_LCL_VAR)); + assert(varDsc->lvDoNotEnregister || varDsc->lvDefinedViaAddress || + tree->OperIs(GT_LCL_VAR, GT_STORE_LCL_VAR)); } if (isUse && !VarSetOps::IsMember(this, fgCurDefSet, varDsc->lvVarIndex)) From c2b3e88e93bd8b6bcd59e47c610483a0403cd87a Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 17 Jul 2025 13:58:35 +0200 Subject: [PATCH 09/28] Clean up --- .../src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs | 2 -- src/coreclr/inc/corinfo.h | 1 - src/coreclr/vm/corelib.h | 1 - src/coreclr/vm/jitinterface.cpp | 1 - 4 files changed, 5 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs index 8d37fe5c4de9b0..1f6567c354cf02 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs @@ -645,7 +645,5 @@ private static void CaptureContinuationContext(ref object context, ref CorInfoCo flags |= CorInfoContinuationFlags.CORINFO_CONTINUATION_CONTINUE_ON_THREAD_POOL; } - - private static Thread GetCurrentThread() => Thread.CurrentThreadAssumedInitialized; } } diff --git a/src/coreclr/inc/corinfo.h b/src/coreclr/inc/corinfo.h index 9b48f7f82fac62..3f1cfa876630df 100644 --- a/src/coreclr/inc/corinfo.h +++ b/src/coreclr/inc/corinfo.h @@ -1749,7 +1749,6 @@ struct CORINFO_ASYNC_INFO CORINFO_METHOD_HANDLE captureContinuationContextMethHnd; CORINFO_METHOD_HANDLE captureContextsMethHnd; CORINFO_METHOD_HANDLE restoreContextsMethHnd; - CORINFO_METHOD_HANDLE getCurrentThreadMethHnd; }; // Flags passed from JIT to runtime. diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 4a41fcf00cdfb0..3e47ccfab8068e 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -731,7 +731,6 @@ DEFINE_METHOD(ASYNC_HELPERS, RESTORE_EXECUTION_CONTEXT, RestoreExecutionCon DEFINE_METHOD(ASYNC_HELPERS, CAPTURE_CONTINUATION_CONTEXT, CaptureContinuationContext, NoSig) DEFINE_METHOD(ASYNC_HELPERS, CAPTURE_CONTEXTS, CaptureContexts, NoSig) DEFINE_METHOD(ASYNC_HELPERS, RESTORE_CONTEXTS, RestoreContexts, NoSig) -DEFINE_METHOD(ASYNC_HELPERS, GET_CURRENT_THREAD, GetCurrentThread, NoSig) DEFINE_CLASS(SPAN_HELPERS, System, SpanHelpers) DEFINE_METHOD(SPAN_HELPERS, MEMSET, Fill, SM_RefByte_Byte_UIntPtr_RetVoid) diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index e69a1a71676ac6..2e3b7ba339cdc6 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -10260,7 +10260,6 @@ void CEEInfo::getAsyncInfo(CORINFO_ASYNC_INFO* pAsyncInfoOut) pAsyncInfoOut->captureContinuationContextMethHnd = CORINFO_METHOD_HANDLE(CoreLibBinder::GetMethod(METHOD__ASYNC_HELPERS__CAPTURE_CONTINUATION_CONTEXT)); pAsyncInfoOut->captureContextsMethHnd = CORINFO_METHOD_HANDLE(CoreLibBinder::GetMethod(METHOD__ASYNC_HELPERS__CAPTURE_CONTEXTS)); pAsyncInfoOut->restoreContextsMethHnd = CORINFO_METHOD_HANDLE(CoreLibBinder::GetMethod(METHOD__ASYNC_HELPERS__RESTORE_CONTEXTS)); - pAsyncInfoOut->getCurrentThreadMethHnd = CORINFO_METHOD_HANDLE(CoreLibBinder::GetMethod(METHOD__ASYNC_HELPERS__GET_CURRENT_THREAD)); EE_TO_JIT_TRANSITION(); } From f9e62e10316c0da6cdc799d96e44c34bc368d58c Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 17 Jul 2025 14:05:07 +0200 Subject: [PATCH 10/28] Fix SPMI --- src/coreclr/tools/superpmi/superpmi-shared/agnostic.h | 3 +++ .../tools/superpmi/superpmi-shared/methodcontext.cpp | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h index 1a04979bad37a5..3097e47190040e 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/agnostic.h @@ -205,6 +205,9 @@ struct Agnostic_CORINFO_ASYNC_INFO DWORD continuationsNeedMethodHandle; DWORDLONG captureExecutionContextMethHnd; DWORDLONG restoreExecutionContextMethHnd; + DWORDLONG captureContinuationContextMethHnd; + DWORDLONG captureContextsMethHnd; + DWORDLONG restoreContextsMethHnd; }; struct Agnostic_GetOSRInfo diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index bfdab057de446b..855b22e36497da 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -4486,6 +4486,9 @@ void MethodContext::recGetAsyncInfo(const CORINFO_ASYNC_INFO* pAsyncInfo) value.continuationsNeedMethodHandle = pAsyncInfo->continuationsNeedMethodHandle ? 1 : 0; value.captureExecutionContextMethHnd = CastHandle(pAsyncInfo->captureExecutionContextMethHnd); value.restoreExecutionContextMethHnd = CastHandle(pAsyncInfo->restoreExecutionContextMethHnd); + value.captureContinuationContextMethHnd = CastHandle(pAsyncInfo->captureContinuationContextMethHnd); + value.captureContextsMethHnd = CastHandle(pAsyncInfo->captureContextsMethHnd); + value.restoreContextsMethHnd = CastHandle(pAsyncInfo->restoreContextsMethHnd); GetAsyncInfo->Add(0, value); DEBUG_REC(dmpGetAsyncInfo(0, value)); @@ -4495,7 +4498,6 @@ void MethodContext::dmpGetAsyncInfo(DWORD key, const Agnostic_CORINFO_ASYNC_INFO printf("GetAsyncInfo key %u value contClsHnd-%016" PRIX64 " contNextFldHnd-%016" PRIX64 " contResumeFldHnd-%016" PRIX64 " contStateFldHnd-%016" PRIX64 " contFlagsFldHnd-%016" PRIX64 " contDataFldHnd-%016" PRIX64 " contGCDataFldHnd-%016" PRIX64 " contsNeedMethodHandle-%d", key, value.continuationClsHnd, value.continuationNextFldHnd, value.continuationResumeFldHnd, - value.continuationStateFldHnd, value.continuationFlagsFldHnd, value.continuationDataFldHnd, value.continuationGCDataFldHnd, value.continuationsNeedMethodHandle); } void MethodContext::repGetAsyncInfo(CORINFO_ASYNC_INFO* pAsyncInfoOut) @@ -4511,6 +4513,9 @@ void MethodContext::repGetAsyncInfo(CORINFO_ASYNC_INFO* pAsyncInfoOut) pAsyncInfoOut->continuationsNeedMethodHandle = value.continuationsNeedMethodHandle != 0; pAsyncInfoOut->captureExecutionContextMethHnd = (CORINFO_METHOD_HANDLE)value.captureExecutionContextMethHnd; pAsyncInfoOut->restoreExecutionContextMethHnd = (CORINFO_METHOD_HANDLE)value.restoreExecutionContextMethHnd; + pAsyncInfoOut->captureContinuationContextMethHnd = (CORINFO_METHOD_HANDLE)value.captureContinuationContextMethHnd; + pAsyncInfoOut->captureContextsMethHnd = (CORINFO_METHOD_HANDLE)value.captureContextsMethHnd; + pAsyncInfoOut->restoreContextsMethHnd = (CORINFO_METHOD_HANDLE)value.restoreContextsMethHnd; DEBUG_REP(dmpGetAsyncInfo(0, value)); } From 576d1380b353922d4523b1147a4795e8f0541406 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 17 Jul 2025 14:19:18 +0200 Subject: [PATCH 11/28] Fix build --- src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index 713c379ed79410..66c7b548efd8d2 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -4498,6 +4498,7 @@ void MethodContext::dmpGetAsyncInfo(DWORD key, const Agnostic_CORINFO_ASYNC_INFO printf("GetAsyncInfo key %u value contClsHnd-%016" PRIX64 " contNextFldHnd-%016" PRIX64 " contResumeFldHnd-%016" PRIX64 " contStateFldHnd-%016" PRIX64 " contFlagsFldHnd-%016" PRIX64 " contDataFldHnd-%016" PRIX64 " contGCDataFldHnd-%016" PRIX64 " contsNeedMethodHandle-%d", key, value.continuationClsHnd, value.continuationNextFldHnd, value.continuationResumeFldHnd, + value.continuationStateFldHnd, value.continuationFlagsFldHnd, value.continuationDataFldHnd, value.continuationGCDataFldHnd, value.continuationsNeedMethodHandle); } void MethodContext::repGetAsyncInfo(CORINFO_ASYNC_INFO* pAsyncInfoOut) From 39549becceffd16cc1c8e6b372d3c6f44d7039a5 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 17 Jul 2025 14:57:51 +0200 Subject: [PATCH 12/28] JIT: Fix unique successors in `BBswtDesc` in runtime async --- src/coreclr/jit/async.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index 5e12889c53dd6a..94a2eb42be8ea0 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -2394,8 +2394,8 @@ void AsyncTransformation::CreateResumptionSwitch() } } - BBswtDesc* const swtDesc = - new (m_comp, CMK_BasicBlock) BBswtDesc(cases, (unsigned)numCases, succs, numUniqueSuccs, true); + BBswtDesc* const swtDesc = new (m_comp, CMK_BasicBlock) + BBswtDesc(succs, numUniqueSuccs, cases, (unsigned)numCases, /* hasDefault */ true); switchBB->SetSwitch(swtDesc); } From a38902ba282764c0f724919ae5a09ccb6873a52d Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 17 Jul 2025 15:17:53 +0200 Subject: [PATCH 13/28] Clean up --- src/coreclr/jit/async.cpp | 18 ++++++++++++++++++ src/coreclr/jit/gentree.cpp | 10 ++++++++++ src/coreclr/jit/gentree.h | 1 - src/coreclr/jit/lir.cpp | 13 ------------- src/coreclr/jit/lir.h | 1 - src/coreclr/jit/morph.cpp | 22 +++++++++++++--------- 6 files changed, 41 insertions(+), 24 deletions(-) diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index 94a2eb42be8ea0..0f09b3ee4080b3 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -1111,6 +1111,14 @@ ContinuationLayout AsyncTransformation::LayOutContinuation(BasicBlock* return layout; } +//------------------------------------------------------------------------ +// AsyncTransformation::ClearSuspendedIndicator: +// Generate IR to clear the value of the suspended indicator local. +// +// Parameters: +// block - Block to generate IR into +// call - The async call (not contained in "block") +// void AsyncTransformation::ClearSuspendedIndicator(BasicBlock* block, GenTreeCall* call) { CallArg* suspendedArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); @@ -1136,6 +1144,16 @@ void AsyncTransformation::ClearSuspendedIndicator(BasicBlock* block, GenTreeCall LIR::AsRange(block).InsertBefore(call, LIR::SeqTree(m_comp, storeSuspended)); } +//------------------------------------------------------------------------ +// AsyncTransformation::SetSuspendedIndicator: +// Generate IR to set the value of the suspended indicator local, and remove +// the argument from the call. +// +// Parameters: +// block - Block to generate IR into +// callBlock - Block containing the call +// call - The async call +// void AsyncTransformation::SetSuspendedIndicator(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call) { CallArg* suspendedArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index d98e6a1977736f..d125fef708d1aa 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -19602,6 +19602,16 @@ GenTreeLclVarCommon* Compiler::gtCallGetDefinedRetBufLclAddr(GenTreeCall* call) return node->AsLclVarCommon(); } +//------------------------------------------------------------------------ +// gtCallGetDefinedAsyncSuspendedIndicatorLclAddr: +// Get the tree corresponding to the address of the indicator local that this call defines. +// +// Parameters: +// call - the Call node +// +// Returns: +// A tree representing the address of a local. +// GenTreeLclVarCommon* Compiler::gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(GenTreeCall* call) { if (!call->IsAsync() || !call->GetAsyncInfo().HasSuspensionIndicatorDef) diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 375d40f8c222c2..88207626ee7480 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4363,7 +4363,6 @@ struct AsyncCallInfo ContinuationContextHandling ContinuationContextHandling = ContinuationContextHandling::None; bool SaveAndRestoreSynchronizationContextField = false; bool HasSuspensionIndicatorDef = false; - bool HasCurrentThreadDef = false; unsigned SynchronizationContextLclNum = BAD_VAR_NUM; }; diff --git a/src/coreclr/jit/lir.cpp b/src/coreclr/jit/lir.cpp index aa86a882273874..9592d4c662d84c 100644 --- a/src/coreclr/jit/lir.cpp +++ b/src/coreclr/jit/lir.cpp @@ -355,19 +355,6 @@ GenTree* LIR::ReadOnlyRange::LastNode() const return m_lastNode; } -GenTree* LIR::ReadOnlyRange::FirstNonNopNode() const -{ - for (GenTree* cur = m_firstNode; cur != nullptr; cur = cur->gtNext) - { - if (!cur->OperIs(GT_NOP, GT_IL_OFFSET)) - { - return cur; - } - } - - return nullptr; -} - //------------------------------------------------------------------------ // LIR::ReadOnlyRange::IsEmpty: Returns true if the range is empty; false // otherwise. diff --git a/src/coreclr/jit/lir.h b/src/coreclr/jit/lir.h index cf094786817b23..99d011ea32e9bc 100644 --- a/src/coreclr/jit/lir.h +++ b/src/coreclr/jit/lir.h @@ -212,7 +212,6 @@ class LIR final GenTree* FirstNode() const; GenTree* LastNode() const; - GenTree* FirstNonNopNode() const; bool IsEmpty() const; diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index 5666f566df1a46..11e501c2f2b13d 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -1844,8 +1844,6 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call const CORINFO_CLASS_HANDLE argSigClass = arg.GetSignatureClassHandle(); ClassLayout* argLayout = argSigClass == NO_CLASS_HANDLE ? nullptr : comp->typGetObjLayout(argSigClass); - ABIPassingInformation abiInfo; - // Some well known args have custom register assignment. // These should not affect the placement of any other args or stack space required. // Example: on AMD64 R10 and R11 are used for indirect VSD (generic interface) and cookie calls. @@ -1854,20 +1852,26 @@ void CallArgs::AddFinalArgsAndDetermineABIInfo(Compiler* comp, GenTreeCall* call if (nonStdRegNum == REG_NA) { - abiInfo = classifier.Classify(comp, argSigType, argLayout, arg.GetWellKnownArg()); + if (arg.GetWellKnownArg() == WellKnownArg::AsyncSuspendedIndicator) + { + // Represents definition of a local. Expanded out by async transformation. + arg.AbiInfo = ABIPassingInformation(comp, 0); + } + else + { + arg.AbiInfo = classifier.Classify(comp, argSigType, argLayout, arg.GetWellKnownArg()); + } } else { ABIPassingSegment segment = ABIPassingSegment::InRegister(nonStdRegNum, 0, TARGET_POINTER_SIZE); - abiInfo = ABIPassingInformation::FromSegmentByValue(comp, segment); + arg.AbiInfo = ABIPassingInformation::FromSegmentByValue(comp, segment); } JITDUMP("Argument %u ABI info: ", GetIndex(&arg)); - DBEXEC(VERBOSE, abiInfo.Dump()); - - arg.AbiInfo = abiInfo; + DBEXEC(VERBOSE, arg.AbiInfo.Dump()); - for (const ABIPassingSegment& segment : abiInfo.Segments()) + for (const ABIPassingSegment& segment : arg.AbiInfo.Segments()) { if (segment.IsPassedOnStack()) { @@ -1923,7 +1927,7 @@ void CallArgs::DetermineABIInfo(Compiler* comp, GenTreeCall* call) { if (arg.GetWellKnownArg() == WellKnownArg::AsyncSuspendedIndicator) { - // Represents definitions of locals. Expanded out by async transformation. + // Represents definition of a local. Expanded out by async transformation. arg.AbiInfo = ABIPassingInformation(comp, 0); continue; } From f91aa6a4c737f7dc5b3b8d5605e0daae41442c3b Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 17 Jul 2025 15:58:52 +0200 Subject: [PATCH 14/28] Expand test --- .../synchronization-context.cs | 72 ++++++++++++++++++- 1 file changed, 69 insertions(+), 3 deletions(-) diff --git a/src/tests/async/synchronization-context/synchronization-context.cs b/src/tests/async/synchronization-context/synchronization-context.cs index e028ee09c9b061..36e61d1535f5c9 100644 --- a/src/tests/async/synchronization-context/synchronization-context.cs +++ b/src/tests/async/synchronization-context/synchronization-context.cs @@ -11,13 +11,13 @@ public class Async2SynchronizationContext { [Fact] - public static void TestSyncContexts() + public static void TestSyncContextContinue() { SynchronizationContext prevContext = SynchronizationContext.Current; try { SynchronizationContext.SetSynchronizationContext(new MySyncContext()); - TestSyncContext().GetAwaiter().GetResult(); + TestSyncContextContinueAsync().GetAwaiter().GetResult(); } finally { @@ -25,7 +25,7 @@ public static void TestSyncContexts() } } - private static async Task TestSyncContext() + private static async Task TestSyncContextContinueAsync() { MySyncContext context = (MySyncContext)SynchronizationContext.Current; await WrappedYieldToThreadPool(suspend: false); @@ -104,4 +104,70 @@ public void OnCompleted(Action continuation) public void GetResult() { } } + + [Fact] + public static void TestSyncContextSaveRestore() + { + SynchronizationContext prevContext = SynchronizationContext.Current; + try + { + SynchronizationContext.SetSynchronizationContext(new SyncContextWithoutRestore()); + TestSyncContextSaveRestoreAsync().GetAwaiter().GetResult(); + } + finally + { + SynchronizationContext.SetSynchronizationContext(prevContext); + } + } + + private static async Task TestSyncContextSaveRestoreAsync() + { + Assert.True(SynchronizationContext.Current is SyncContextWithoutRestore); + await ClearSyncContext(); + Assert.True(SynchronizationContext.Current is SyncContextWithoutRestore); + } + + private static async Task ClearSyncContext() + { + SynchronizationContext.SetSynchronizationContext(null); + } + + [Fact] + public static void TestSyncContextNotRestored() + { + SynchronizationContext prevContext = SynchronizationContext.Current; + try + { + SynchronizationContext.SetSynchronizationContext(new SyncContextWithoutRestore()); + TestSyncContextNotRestoredAsync().GetAwaiter().GetResult(); + } + finally + { + SynchronizationContext.SetSynchronizationContext(prevContext); + } + } + + private static async Task TestSyncContextNotRestoredAsync() + { + Assert.True(SynchronizationContext.Current is SyncContextWithoutRestore); + await SuspendThenClearSyncContext(); + Assert.Null(SynchronizationContext.Current); + } + + private static async Task SuspendThenClearSyncContext() + { + await Task.Yield(); + SynchronizationContext.SetSynchronizationContext(null); + } + + private class SyncContextWithoutRestore : SynchronizationContext + { + public override void Post(SendOrPostCallback d, object state) + { + ThreadPool.UnsafeQueueUserWorkItem(_ => + { + d(state); + }, null); + } + } } From c7865458ce8c86150d7a3c68a2d1585fdbae3a57 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 17 Jul 2025 16:08:15 +0200 Subject: [PATCH 15/28] Nit --- .../async/synchronization-context/synchronization-context.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/src/tests/async/synchronization-context/synchronization-context.csproj b/src/tests/async/synchronization-context/synchronization-context.csproj index 1c9f61351ff305..1ae294349c376f 100644 --- a/src/tests/async/synchronization-context/synchronization-context.csproj +++ b/src/tests/async/synchronization-context/synchronization-context.csproj @@ -1,7 +1,6 @@ True - portable From aa31bfb04f6828004357596864619307cb43f318 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 17 Jul 2025 16:39:04 +0200 Subject: [PATCH 16/28] Fix release build --- 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 7412966fb4232a..e403d662b5b683 100644 --- a/src/coreclr/jit/lclmorph.cpp +++ b/src/coreclr/jit/lclmorph.cpp @@ -1501,7 +1501,7 @@ class LocalAddressVisitor final : public GenTreeVisitor CallArg* suspendedArg = callUser->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); if ((suspendedArg != nullptr) && (val.Node() == suspendedArg->GetNode())) { - varDsc->SetDefinedViaAddress(true); + INDEBUG(varDsc->SetDefinedViaAddress(true)); escapeAddr = false; defFlag = GTF_VAR_DEF; From 9e3a6863486cd5aec813ee911800208019571b69 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 17 Jul 2025 20:10:54 +0200 Subject: [PATCH 17/28] Fix dangling pointer bug --- src/coreclr/jit/async.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index 0f09b3ee4080b3..3fa892b8ebd5a1 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -2088,6 +2088,9 @@ void AsyncTransformation::CopyReturnValueOnResumption(GenTreeCall* LIR::AsRange(storeResultBB).InsertAtEnd(LIR::SeqTree(m_comp, storeResultBase)); resultBase = m_comp->gtNewLclVarNode(resultBaseVar, TYP_REF); + + // Can be reallocated by above call to GetResultBaseVar + resultLcl = m_comp->lvaGetDesc(callDefInfo.DefinitionNode); } assert(callDefInfo.DefinitionNode->OperIs(GT_STORE_LCL_VAR)); From c78ca392cacd47bf88c5b9886e7b3c22445e539a Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 17 Jul 2025 17:24:50 +0200 Subject: [PATCH 18/28] Update JIT-EE GUID --- src/coreclr/inc/jiteeversionguid.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/coreclr/inc/jiteeversionguid.h b/src/coreclr/inc/jiteeversionguid.h index 7043a7fa2f55ac..fe1aaade4fb3b9 100644 --- a/src/coreclr/inc/jiteeversionguid.h +++ b/src/coreclr/inc/jiteeversionguid.h @@ -37,11 +37,11 @@ #include -constexpr GUID JITEEVersionIdentifier = { /* d24a67e0-9e57-4c9e-ad31-5785df2526f2 */ - 0xd24a67e0, - 0x9e57, - 0x4c9e, - {0xad, 0x31, 0x57, 0x85, 0xdf, 0x25, 0x26, 0xf2} +constexpr GUID JITEEVersionIdentifier = { /* 2d40ec46-2e41-4a8b-8349-3c1267b95821 */ + 0x2d40ec46, + 0x2e41, + 0x4a8b, + {0x83, 0x49, 0x3c, 0x12, 0x67, 0xb9, 0x58, 0x21} }; #endif // JIT_EE_VERSIONING_GUID_H From 24cd099e3ee521c7189ddaec0a6d5b7b2efb069f Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 17 Jul 2025 17:26:09 +0200 Subject: [PATCH 19/28] Redisable runtime async --- src/coreclr/clrdefinitions.cmake | 2 +- src/tests/async/Directory.Build.targets | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 src/tests/async/Directory.Build.targets diff --git a/src/coreclr/clrdefinitions.cmake b/src/coreclr/clrdefinitions.cmake index 010a1ad1ed74d9..18b6c457977877 100644 --- a/src/coreclr/clrdefinitions.cmake +++ b/src/coreclr/clrdefinitions.cmake @@ -152,7 +152,7 @@ if(FEATURE_OBJCMARSHAL) add_compile_definitions(FEATURE_OBJCMARSHAL) endif() -add_compile_definitions(FEATURE_RUNTIME_ASYNC) +# add_compile_definitions(FEATURE_RUNTIME_ASYNC) add_compile_definitions($<${FEATURE_JAVAMARSHAL}:FEATURE_JAVAMARSHAL>) diff --git a/src/tests/async/Directory.Build.targets b/src/tests/async/Directory.Build.targets new file mode 100644 index 00000000000000..5a4d413a9e1f62 --- /dev/null +++ b/src/tests/async/Directory.Build.targets @@ -0,0 +1,13 @@ + + + + true + + + + + true + + + + From f40c25581bb8c8ab4511f694c6beea9a0e422087 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 21 Jul 2025 13:13:20 +0200 Subject: [PATCH 20/28] Require AsyncSuspendedIndicator to be present when HasSuspensionIndicatorDef --- src/coreclr/jit/async.cpp | 1 + src/coreclr/jit/gentree.cpp | 6 +----- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index 3fa892b8ebd5a1..79311bb6e650bb 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -1172,6 +1172,7 @@ void AsyncTransformation::SetSuspendedIndicator(BasicBlock* block, BasicBlock* c LIR::AsRange(block).InsertAtEnd(LIR::SeqTree(m_comp, storeSuspended)); call->gtArgs.RemoveUnsafe(suspendedArg); + call->asyncInfo->HasSuspensionIndicatorDef = false; // Avoid leaving LCL_ADDR around which will DNER the local. if (suspended->IsLclVarAddr()) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index d125fef708d1aa..12ef5b2f1d8895 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -19620,11 +19620,7 @@ GenTreeLclVarCommon* Compiler::gtCallGetDefinedAsyncSuspendedIndicatorLclAddr(Ge } CallArg* asyncSuspensionIndicatorArg = call->gtArgs.FindWellKnownArg(WellKnownArg::AsyncSuspendedIndicator); - if (asyncSuspensionIndicatorArg == nullptr) - { - return nullptr; - } - + assert(asyncSuspensionIndicatorArg != nullptr); GenTree* node = asyncSuspensionIndicatorArg->GetNode(); assert(node->OperIs(GT_LCL_ADDR) && lvaGetDesc(node->AsLclVarCommon())->IsDefinedViaAddress()); From 2c073abc86e613fc2d7450146a9ccc4e123fe0ba Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 21 Jul 2025 13:13:36 +0200 Subject: [PATCH 21/28] Update async comments --- src/coreclr/jit/async.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index 79311bb6e650bb..c789d5ab8d4c75 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -6,11 +6,11 @@ // machines. The following key operations are performed: // // 1. Early, after import but before inlining: for async calls that require -// ExecutionContext save/restore semantics, ExecutionContext capture and +// ExecutionContext/SynchronizationContext save/restore semantics, capture and // restore calls are inserted around the async call site. This ensures proper // context flow across await boundaries when the continuation may run on -// different threads or synchronization contexts. The captured ExecutionContext -// is stored in a temporary local and restored after the async call completes, +// different threads or synchronization contexts. The captured contexts +// are stored in temporary locals and restored after the async call completes, // with special handling for calls inside try regions using try-finally blocks. // // Later, right before lowering the actual transformation to a state machine is @@ -47,7 +47,7 @@ //------------------------------------------------------------------------ // Compiler::SaveAsyncContexts: -// Insert code to save and restore ExecutionContext around async call sites. +// Insert code to save and restore contexts around async call sites. // // Returns: // Suitable phase status. From 074151724ce44c90979db2cb32e056caf51971e8 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 21 Jul 2025 14:27:21 +0200 Subject: [PATCH 22/28] Assert NumPosts --- .../synchronization-context/synchronization-context.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/tests/async/synchronization-context/synchronization-context.cs b/src/tests/async/synchronization-context/synchronization-context.cs index 36e61d1535f5c9..87371c31172c97 100644 --- a/src/tests/async/synchronization-context/synchronization-context.cs +++ b/src/tests/async/synchronization-context/synchronization-context.cs @@ -156,14 +156,22 @@ private static async Task TestSyncContextNotRestoredAsync() private static async Task SuspendThenClearSyncContext() { + Assert.True(SynchronizationContext.Current is SyncContextWithoutRestore); + SyncContextWithoutRestore syncCtx = (SyncContextWithoutRestore)SyncContextWithoutRestore.Current; + Assert.Equal(0, syncCtx.NumPosts); + await Task.Yield(); - SynchronizationContext.SetSynchronizationContext(null); + Assert.Null(SynchronizationContext.Current); + Assert.Equal(1, syncCtx.NumPosts); } private class SyncContextWithoutRestore : SynchronizationContext { + public int NumPosts; + public override void Post(SendOrPostCallback d, object state) { + NumPosts++; ThreadPool.UnsafeQueueUserWorkItem(_ => { d(state); From 8066afb6b9a0bf60474fde26f4a7e64fe08fe956 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 21 Jul 2025 14:27:34 +0200 Subject: [PATCH 23/28] Add another test that we capture the sync context from before the call --- .../synchronization-context.cs | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/src/tests/async/synchronization-context/synchronization-context.cs b/src/tests/async/synchronization-context/synchronization-context.cs index 87371c31172c97..e3a5e2eed606b4 100644 --- a/src/tests/async/synchronization-context/synchronization-context.cs +++ b/src/tests/async/synchronization-context/synchronization-context.cs @@ -178,4 +178,39 @@ public override void Post(SendOrPostCallback d, object state) }, null); } } + + [Fact] + public static void TestContinueOnCorrectSyncContext() + { + SynchronizationContext prevContext = SynchronizationContext.Current; + try + { + TestContinueOnCorrectSyncContextAsync().GetAwaiter().GetResult(); + } + finally + { + SynchronizationContext.SetSynchronizationContext(prevContext); + } + } + + private static async Task TestContinueOnCorrectSyncContextAsync() + { + MySyncContext context1 = new MySyncContext(); + MySyncContext context2 = new MySyncContext(); + + SynchronizationContext.SetSynchronizationContext(context1); + await SetContext(context2, suspend: false); + Assert.True(SynchronizationContext.Current == context1); + + await SetContext(context2, suspend: true); + Assert.True(SynchronizationContext.Current == context1); + } + + private static async Task SetContext(SynchronizationContext context, bool suspend) + { + SynchronizationContext.SetSynchronizationContext(context); + + if (suspend) + await Task.Yield(); + } } From ddd75846805a59db1568a4f43dd3762e746daa00 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 21 Jul 2025 14:28:58 +0200 Subject: [PATCH 24/28] Use sync context from before call as continuation context --- .../CompilerServices/AsyncHelpers.CoreCLR.cs | 3 +- src/coreclr/jit/async.cpp | 23 +++++++- src/coreclr/jit/async.h | 58 +++++++++---------- 3 files changed, 50 insertions(+), 34 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs index 1f6567c354cf02..ca5b1064c374f7 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs @@ -625,9 +625,8 @@ private static void RestoreContexts(bool suspended, ExecutionContext? previousEx } } - private static void CaptureContinuationContext(ref object context, ref CorInfoContinuationFlags flags) + private static void CaptureContinuationContext(ref object context, SynchronizationContext syncCtx, ref CorInfoContinuationFlags flags) { - SynchronizationContext? syncCtx = Thread.CurrentThreadAssumedInitialized._synchronizationContext; if (syncCtx != null && syncCtx.GetType() != typeof(SynchronizationContext)) { flags |= CorInfoContinuationFlags.CORINFO_CONTINUATION_CONTINUE_ON_CAPTURED_SYNCHRONIZATION_CONTEXT; diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index c789d5ab8d4c75..eca20bfaa16d51 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -1330,7 +1330,7 @@ BasicBlock* AsyncTransformation::CreateSuspension( if (layout.GCRefsCount > 0) { - FillInGCPointersOnSuspension(layout, suspendBB); + FillInGCPointersOnSuspension(call, layout, suspendBB); } if (layout.DataSize > 0) @@ -1410,10 +1410,13 @@ GenTreeCall* AsyncTransformation::CreateAllocContinuationCall(AsyncLiveness& lif // parts that need to be stored. // // Parameters: +// call - The async call that is being transformed // layout - Layout information // suspendBB - Basic block to add IR to. // -void AsyncTransformation::FillInGCPointersOnSuspension(const ContinuationLayout& layout, BasicBlock* suspendBB) +void AsyncTransformation::FillInGCPointersOnSuspension(GenTreeCall* call, + const ContinuationLayout& layout, + BasicBlock* suspendBB) { unsigned objectArrLclNum = GetGCDataArrayVar(); @@ -1502,14 +1505,20 @@ void AsyncTransformation::FillInGCPointersOnSuspension(const ContinuationLayout& if (layout.ContinuationContextGCDataIndex != UINT_MAX) { + const AsyncCallInfo& callInfo = call->GetAsyncInfo(); + assert(callInfo.SaveAndRestoreSynchronizationContextField && + (callInfo.SynchronizationContextLclNum != BAD_VAR_NUM)); + // Insert call AsyncHelpers.CaptureContinuationContext(ref // newContinuation.GCData[ContinuationContextGCDataIndex], ref newContinuation.Flags). GenTree* contextElementPlaceholder = m_comp->gtNewZeroConNode(TYP_BYREF); + GenTree* syncContextPlaceholder = m_comp->gtNewNull(); GenTree* flagsPlaceholder = m_comp->gtNewZeroConNode(TYP_BYREF); GenTreeCall* captureCall = m_comp->gtNewCallNode(CT_USER_FUNC, m_asyncInfo->captureContinuationContextMethHnd, TYP_VOID); captureCall->gtArgs.PushFront(m_comp, NewCallArg::Primitive(flagsPlaceholder)); + captureCall->gtArgs.PushFront(m_comp, NewCallArg::Primitive(syncContextPlaceholder)); captureCall->gtArgs.PushFront(m_comp, NewCallArg::Primitive(contextElementPlaceholder)); m_comp->compCurBB = suspendBB; @@ -1531,7 +1540,15 @@ void AsyncTransformation::FillInGCPointersOnSuspension(const ContinuationLayout& use.ReplaceWith(contextElementOffset); LIR::AsRange(suspendBB).Remove(contextElementPlaceholder); - // And now replace flagsPlaceholder with actual address of the flags + // Then do the sync context + gotUse = LIR::AsRange(suspendBB).TryGetUse(syncContextPlaceholder, &use); + assert(gotUse); + GenTree* syncContextLcl = m_comp->gtNewLclvNode(callInfo.SynchronizationContextLclNum, TYP_REF); + LIR::AsRange(suspendBB).InsertBefore(syncContextPlaceholder, syncContextLcl); + use.ReplaceWith(syncContextLcl); + LIR::AsRange(suspendBB).Remove(syncContextPlaceholder); + + // And finally replace flagsPlaceholder with actual address of the flags gotUse = LIR::AsRange(suspendBB).TryGetUse(flagsPlaceholder, &use); assert(gotUse); diff --git a/src/coreclr/jit/async.h b/src/coreclr/jit/async.h index 7cfdb4d3aa86d7..e30aaf760e6395 100644 --- a/src/coreclr/jit/async.h +++ b/src/coreclr/jit/async.h @@ -94,37 +94,37 @@ class AsyncTransformation GenTree* prevContinuation, unsigned gcRefsCount, unsigned int dataSize); - void FillInGCPointersOnSuspension(const ContinuationLayout& layout, BasicBlock* suspendBB); - void FillInDataOnSuspension(const jitstd::vector& liveLocals, BasicBlock* suspendBB); - void CreateCheckAndSuspendAfterCall(BasicBlock* block, - GenTreeCall* call, - const CallDefinitionInfo& callDefInfo, - AsyncLiveness& life, - BasicBlock* suspendBB, - BasicBlock** remainder); - BasicBlock* CreateResumption(BasicBlock* block, - BasicBlock* remainder, - GenTreeCall* call, - const CallDefinitionInfo& callDefInfo, - unsigned stateNum, - const ContinuationLayout& layout); - void SetSuspendedIndicator(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call); - void RestoreFromDataOnResumption(unsigned resumeByteArrLclNum, - const jitstd::vector& liveLocals, - BasicBlock* resumeBB); - void RestoreFromGCPointersOnResumption(unsigned resumeObjectArrLclNum, - const ContinuationLayout& layout, - BasicBlock* resumeBB); - BasicBlock* RethrowExceptionOnResumption(BasicBlock* block, - unsigned resumeObjectArrLclNum, - const ContinuationLayout& layout, - BasicBlock* resumeBB); - void CopyReturnValueOnResumption(GenTreeCall* call, - const CallDefinitionInfo& callDefInfo, - unsigned resumeByteArrLclNum, + void FillInGCPointersOnSuspension(GenTreeCall* call, const ContinuationLayout& layout, BasicBlock* suspendBB); + void FillInDataOnSuspension(const jitstd::vector& liveLocals, BasicBlock* suspendBB); + void CreateCheckAndSuspendAfterCall(BasicBlock* block, + GenTreeCall* call, + const CallDefinitionInfo& callDefInfo, + AsyncLiveness& life, + BasicBlock* suspendBB, + BasicBlock** remainder); + BasicBlock* CreateResumption(BasicBlock* block, + BasicBlock* remainder, + GenTreeCall* call, + const CallDefinitionInfo& callDefInfo, + unsigned stateNum, + const ContinuationLayout& layout); + void SetSuspendedIndicator(BasicBlock* block, BasicBlock* callBlock, GenTreeCall* call); + void RestoreFromDataOnResumption(unsigned resumeByteArrLclNum, + const jitstd::vector& liveLocals, + BasicBlock* resumeBB); + void RestoreFromGCPointersOnResumption(unsigned resumeObjectArrLclNum, + const ContinuationLayout& layout, + BasicBlock* resumeBB); + BasicBlock* RethrowExceptionOnResumption(BasicBlock* block, unsigned resumeObjectArrLclNum, const ContinuationLayout& layout, - BasicBlock* storeResultBB); + BasicBlock* resumeBB); + void CopyReturnValueOnResumption(GenTreeCall* call, + const CallDefinitionInfo& callDefInfo, + unsigned resumeByteArrLclNum, + unsigned resumeObjectArrLclNum, + const ContinuationLayout& layout, + BasicBlock* storeResultBB); GenTreeIndir* LoadFromOffset(GenTree* base, unsigned offset, From ce2f8347d9d6c82c30e8a02719b15e94203e9004 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Mon, 21 Jul 2025 14:30:38 +0200 Subject: [PATCH 25/28] Reset continuation context handling for tailcalls: --- src/coreclr/jit/importercalls.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 798e1d3b9dd8a9..e1570911031abf 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -740,6 +740,7 @@ var_types Compiler::impImportCall(OPCODE opcode, if (tailCallFlags != 0) { asyncInfo.ExecutionContextHandling = ExecutionContextHandling::None; + asyncInfo.ContinuationContextHandling = ContinuationContextHandling::None; asyncInfo.SaveAndRestoreSynchronizationContextField = false; } From 04051546b547b758a10866beaad9cb6a3fcd3394 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 23 Jul 2025 15:10:29 +0200 Subject: [PATCH 26/28] Add some more comments --- src/coreclr/jit/async.cpp | 7 +++++-- src/coreclr/jit/gentree.h | 31 +++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index eca20bfaa16d51..34feee03ab0850 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -1509,8 +1509,11 @@ void AsyncTransformation::FillInGCPointersOnSuspension(GenTreeCall* assert(callInfo.SaveAndRestoreSynchronizationContextField && (callInfo.SynchronizationContextLclNum != BAD_VAR_NUM)); - // Insert call AsyncHelpers.CaptureContinuationContext(ref - // newContinuation.GCData[ContinuationContextGCDataIndex], ref newContinuation.Flags). + // Insert call + // AsyncHelpers.CaptureContinuationContext( + // ref newContinuation.GCData[ContinuationContextGCDataIndex], + // syncContextFromBeforeCall, + // ref newContinuation.Flags). GenTree* contextElementPlaceholder = m_comp->gtNewZeroConNode(TYP_BYREF); GenTree* syncContextPlaceholder = m_comp->gtNewNull(); GenTree* flagsPlaceholder = m_comp->gtNewZeroConNode(TYP_BYREF); diff --git a/src/coreclr/jit/gentree.h b/src/coreclr/jit/gentree.h index 88207626ee7480..fbe7aa21aac328 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4359,6 +4359,37 @@ enum class ContinuationContextHandling // Additional async call info. struct AsyncCallInfo { + // The following information is used to implement the proper observable handling of `ExecutionContext`, + // `SynchronizationContext` and `TaskScheduler` in async methods. + // + // The breakdown of the handling is as follows: + // + // - For custom awaitables there is no special handling of `SynchronizationContext` or `TaskScheduler`. All the handling + // that exists is custom implemented by the user. In this case "ContinuationContextHandling == None" and + // "SaveAndRestoreSynchronizationContextField == false". + // + // - For custom awaitables there _is_ special handling of `ExecutionContext`: when the custom awaitable suspends, the + // JIT ensures that the `ExecutionContext` will be captured on suspension and restored when the continuation is running. + // This is represented by "ExecutionContextHandling == AsyncSaveAndRestore". + // + // - For task awaits there is special handling of `SynchronizationContext` and `TaskScheduler` in multiple ways: + // + // * The JIT ensures that `Thread.CurrentThread._synchronizationContext` is saved and restored around synchronously + // finishing calls. This is represented by "SaveAndRestoreSynchronizationContextField == true". + // + // * The JIT/runtime/BCL ensure that when the callee suspends, the caller will eventually be resumed on the + // `SynchronizationContext`/`TaskScheduler` present before the call started, depending on the configuration of the + // task await by the user. This resumption can be inlined if the `SynchronizationContext` is current when the + // continuation is about to run, and otherwise will be posted to it. This is represented by + // "ContinuationContextHandling == ContinueOnCapturedContext/ContinueOnThreadPool". + // + // * When the callee suspends restoration of `Thread.CurrentThread._synchronizationContext` is left up to the custom + // implementation of the `SynchronizationContext`, it must not be done by the JIT. + // + // - For task awaits the runtime/BCL ensure that `Thread.CurrentThread._executionContext` is captured before the call + // and restored after it. This happens consistently regardless of whether the callee finishes synchronously or not. This + // is represented by "ExecutionContextHandling == SaveAndRestore". + // ExecutionContextHandling ExecutionContextHandling = ExecutionContextHandling::None; ContinuationContextHandling ContinuationContextHandling = ContinuationContextHandling::None; bool SaveAndRestoreSynchronizationContextField = false; From b1329a70331f75a8e565aaec545b1d8141db1239 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 23 Jul 2025 15:19:31 +0200 Subject: [PATCH 27/28] Run jit-format --- 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 fbe7aa21aac328..67be572a3b04a3 100644 --- a/src/coreclr/jit/gentree.h +++ b/src/coreclr/jit/gentree.h @@ -4364,18 +4364,18 @@ struct AsyncCallInfo // // The breakdown of the handling is as follows: // - // - For custom awaitables there is no special handling of `SynchronizationContext` or `TaskScheduler`. All the handling - // that exists is custom implemented by the user. In this case "ContinuationContextHandling == None" and + // - For custom awaitables there is no special handling of `SynchronizationContext` or `TaskScheduler`. All the + // handling that exists is custom implemented by the user. In this case "ContinuationContextHandling == None" and // "SaveAndRestoreSynchronizationContextField == false". // - // - For custom awaitables there _is_ special handling of `ExecutionContext`: when the custom awaitable suspends, the - // JIT ensures that the `ExecutionContext` will be captured on suspension and restored when the continuation is running. - // This is represented by "ExecutionContextHandling == AsyncSaveAndRestore". + // - For custom awaitables there _is_ special handling of `ExecutionContext`: when the custom awaitable suspends, + // the JIT ensures that the `ExecutionContext` will be captured on suspension and restored when the continuation is + // running. This is represented by "ExecutionContextHandling == AsyncSaveAndRestore". // // - For task awaits there is special handling of `SynchronizationContext` and `TaskScheduler` in multiple ways: // - // * The JIT ensures that `Thread.CurrentThread._synchronizationContext` is saved and restored around synchronously - // finishing calls. This is represented by "SaveAndRestoreSynchronizationContextField == true". + // * The JIT ensures that `Thread.CurrentThread._synchronizationContext` is saved and restored around + // synchronously finishing calls. This is represented by "SaveAndRestoreSynchronizationContextField == true". // // * The JIT/runtime/BCL ensure that when the callee suspends, the caller will eventually be resumed on the // `SynchronizationContext`/`TaskScheduler` present before the call started, depending on the configuration of the @@ -4383,12 +4383,12 @@ struct AsyncCallInfo // continuation is about to run, and otherwise will be posted to it. This is represented by // "ContinuationContextHandling == ContinueOnCapturedContext/ContinueOnThreadPool". // - // * When the callee suspends restoration of `Thread.CurrentThread._synchronizationContext` is left up to the custom - // implementation of the `SynchronizationContext`, it must not be done by the JIT. + // * When the callee suspends restoration of `Thread.CurrentThread._synchronizationContext` is left up to the + // custom implementation of the `SynchronizationContext`, it must not be done by the JIT. // - // - For task awaits the runtime/BCL ensure that `Thread.CurrentThread._executionContext` is captured before the call - // and restored after it. This happens consistently regardless of whether the callee finishes synchronously or not. This - // is represented by "ExecutionContextHandling == SaveAndRestore". + // - For task awaits the runtime/BCL ensure that `Thread.CurrentThread._executionContext` is captured before the + // call and restored after it. This happens consistently regardless of whether the callee finishes synchronously or + // not. This is represented by "ExecutionContextHandling == SaveAndRestore". // ExecutionContextHandling ExecutionContextHandling = ExecutionContextHandling::None; ContinuationContextHandling ContinuationContextHandling = ContinuationContextHandling::None; From 6510e3711f9397c1e125406f6f7ba75ce87fffc9 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 23 Jul 2025 16:16:21 +0200 Subject: [PATCH 28/28] Make SyncContext parameter the first one --- .../CompilerServices/AsyncHelpers.CoreCLR.cs | 2 +- src/coreclr/jit/async.cpp | 28 +++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs index ca5b1064c374f7..ba703738d3c7c3 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/AsyncHelpers.CoreCLR.cs @@ -625,7 +625,7 @@ private static void RestoreContexts(bool suspended, ExecutionContext? previousEx } } - private static void CaptureContinuationContext(ref object context, SynchronizationContext syncCtx, ref CorInfoContinuationFlags flags) + private static void CaptureContinuationContext(SynchronizationContext syncCtx, ref object context, ref CorInfoContinuationFlags flags) { if (syncCtx != null && syncCtx.GetType() != typeof(SynchronizationContext)) { diff --git a/src/coreclr/jit/async.cpp b/src/coreclr/jit/async.cpp index 34feee03ab0850..d0c2fb4369abc5 100644 --- a/src/coreclr/jit/async.cpp +++ b/src/coreclr/jit/async.cpp @@ -1511,27 +1511,35 @@ void AsyncTransformation::FillInGCPointersOnSuspension(GenTreeCall* // Insert call // AsyncHelpers.CaptureContinuationContext( - // ref newContinuation.GCData[ContinuationContextGCDataIndex], // syncContextFromBeforeCall, + // ref newContinuation.GCData[ContinuationContextGCDataIndex], // ref newContinuation.Flags). - GenTree* contextElementPlaceholder = m_comp->gtNewZeroConNode(TYP_BYREF); GenTree* syncContextPlaceholder = m_comp->gtNewNull(); + GenTree* contextElementPlaceholder = m_comp->gtNewZeroConNode(TYP_BYREF); GenTree* flagsPlaceholder = m_comp->gtNewZeroConNode(TYP_BYREF); GenTreeCall* captureCall = m_comp->gtNewCallNode(CT_USER_FUNC, m_asyncInfo->captureContinuationContextMethHnd, TYP_VOID); captureCall->gtArgs.PushFront(m_comp, NewCallArg::Primitive(flagsPlaceholder)); - captureCall->gtArgs.PushFront(m_comp, NewCallArg::Primitive(syncContextPlaceholder)); captureCall->gtArgs.PushFront(m_comp, NewCallArg::Primitive(contextElementPlaceholder)); + captureCall->gtArgs.PushFront(m_comp, NewCallArg::Primitive(syncContextPlaceholder)); m_comp->compCurBB = suspendBB; m_comp->fgMorphTree(captureCall); LIR::AsRange(suspendBB).InsertAtEnd(LIR::SeqTree(m_comp, captureCall)); - // Now replace contextElementPlaceholder with actual address of the context element + // Replace sync context placeholder with actual sync context from before call LIR::Use use; - bool gotUse = LIR::AsRange(suspendBB).TryGetUse(contextElementPlaceholder, &use); + bool gotUse = LIR::AsRange(suspendBB).TryGetUse(syncContextPlaceholder, &use); + assert(gotUse); + GenTree* syncContextLcl = m_comp->gtNewLclvNode(callInfo.SynchronizationContextLclNum, TYP_REF); + LIR::AsRange(suspendBB).InsertBefore(syncContextPlaceholder, syncContextLcl); + use.ReplaceWith(syncContextLcl); + LIR::AsRange(suspendBB).Remove(syncContextPlaceholder); + + // Replace contextElementPlaceholder with actual address of the context element + gotUse = LIR::AsRange(suspendBB).TryGetUse(contextElementPlaceholder, &use); assert(gotUse); GenTree* objectArr = m_comp->gtNewLclvNode(objectArrLclNum, TYP_REF); @@ -1543,15 +1551,7 @@ void AsyncTransformation::FillInGCPointersOnSuspension(GenTreeCall* use.ReplaceWith(contextElementOffset); LIR::AsRange(suspendBB).Remove(contextElementPlaceholder); - // Then do the sync context - gotUse = LIR::AsRange(suspendBB).TryGetUse(syncContextPlaceholder, &use); - assert(gotUse); - GenTree* syncContextLcl = m_comp->gtNewLclvNode(callInfo.SynchronizationContextLclNum, TYP_REF); - LIR::AsRange(suspendBB).InsertBefore(syncContextPlaceholder, syncContextLcl); - use.ReplaceWith(syncContextLcl); - LIR::AsRange(suspendBB).Remove(syncContextPlaceholder); - - // And finally replace flagsPlaceholder with actual address of the flags + // Replace flagsPlaceholder with actual address of the flags gotUse = LIR::AsRange(suspendBB).TryGetUse(flagsPlaceholder, &use); assert(gotUse);