From 586760845e8e34c43c8130dbfa7d147dced51d9d Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 7 Jul 2026 21:15:35 +0200 Subject: [PATCH 1/3] Model it directly --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/optcse.cpp | 87 ++++++++++++++++++++++++++++++++------ src/coreclr/jit/optcse.h | 2 + 3 files changed, 78 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 8ab52df52815b2..da70365b91948d 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7781,7 +7781,7 @@ class Compiler void optPrintCSEDataFlowSet(EXPSET_VALARG_TP cseDataFlowSet, bool includeBits = true); EXPSET_TP cseCallKillsMask; // Computed once - A mask that is used to kill available CSEs at callsites - EXPSET_TP cseAsyncKillsMask; // Computed once - A mask that is used to kill available BYREF CSEs at async suspension points + EXPSET_TP cseAsyncKillsMask; // Computed once - A mask that is used to kill available CSEs at async suspension points static const size_t s_optCSEhashSizeInitial; static const size_t s_optCSEhashGrowthFactor; diff --git a/src/coreclr/jit/optcse.cpp b/src/coreclr/jit/optcse.cpp index d14d9ebd570bb9..8e75fff31ba444 100644 --- a/src/coreclr/jit/optcse.cpp +++ b/src/coreclr/jit/optcse.cpp @@ -465,6 +465,79 @@ void CSEdsc::ComputeNumLocals(Compiler* compiler) numLocalOccurrences = lcv.m_occurrences; } +//--------------------------------------------------------------------------- +// IsKilledAcrossAsyncSuspensions: +// Check if this CSE is considered killed across async suspension. +// +// Arguments: +// compiler - compiler instance +// +// Returns: +// True for byref and helpers that return values dependent on current thread. +// +bool CSEdsc::IsKilledAcrossAsyncSuspensions(Compiler* compiler) +{ + GenTree* tree = csdTreeList.tslTree; + if (tree->TypeIs(TYP_BYREF)) + { + return true; + } + + if (tree->TypeIs(TYP_STRUCT)) + { + ClassLayout* layout = tree->GetLayout(compiler); + if (layout->HasGCByRef()) + { + return true; + } + } + + // Handle static bases. Most are byref typed, so handled above, but some of + // these are native int typed (e.g. the pinned variants). + ValueNum vn = tree->gtVNPair.GetLiberal(); + vn = compiler->vnStore->VNNormalValue(vn); + VNFuncApp func; + if (compiler->vnStore->GetVNFunc(vn, &func)) + { + switch (func.GetFunc()) + { + case VNF_GetGcstaticBase: + case VNF_GetNongcstaticBase: + case VNF_GetdynamicGcstaticBase: + case VNF_GetdynamicNongcstaticBase: + case VNF_GetdynamicGcstaticBaseNoctor: + case VNF_GetdynamicNongcstaticBaseNoctor: + case VNF_ReadyToRunStaticBaseGC: + case VNF_ReadyToRunStaticBaseNonGC: + case VNF_ReadyToRunStaticBaseThread: + case VNF_ReadyToRunStaticBaseThreadNoctor: + case VNF_ReadyToRunStaticBaseThreadNonGC: + case VNF_ReadyToRunGenericStaticBase: + case VNF_GetpinnedGcstaticBase: + case VNF_GetpinnedNongcstaticBase: + case VNF_GetpinnedGcstaticBaseNoctor: + case VNF_GetpinnedNongcstaticBaseNoctor: + case VNF_GetGcthreadstaticBase: + case VNF_GetNongcthreadstaticBase: + case VNF_GetGcthreadstaticBaseNoctor: + case VNF_GetNongcthreadstaticBaseNoctor: + case VNF_GetdynamicGcthreadstaticBase: + case VNF_GetdynamicNongcthreadstaticBase: + case VNF_GetdynamicGcthreadstaticBaseNoctor: + case VNF_GetdynamicGcthreadstaticBaseNoctorOptimized: + case VNF_GetdynamicNongcthreadstaticBaseNoctor: + case VNF_GetdynamicNongcthreadstaticBaseNoctorOptimized: + case VNF_GetdynamicNongcthreadstaticBaseNoctorOptimized2: + case VNF_GetdynamicNongcthreadstaticBaseNoctorOptimized2NoJitOpt: + return true; + default: + break; + } + } + + return false; +} + /***************************************************************************** * * Initialize the Value Number CSE tracking logic. @@ -1113,21 +1186,11 @@ void Compiler::optValnumCSE_SetUpAsyncByrefKills() { CSEdsc* dsc = optCSEtab[inx - 1]; assert(dsc->csdIndex == inx); - bool isByRef = false; - if (dsc->csdTreeList.tslTree->TypeIs(TYP_BYREF)) - { - isByRef = true; - } - else if (dsc->csdTreeList.tslTree->TypeIs(TYP_STRUCT)) - { - ClassLayout* layout = dsc->csdTreeList.tslTree->GetLayout(this); - isByRef = layout->HasGCByRef(); - } - if (isByRef) + if (dsc->IsKilledAcrossAsyncSuspensions(this)) { // We generate a bit pattern like: 1111111100111100 where there - // are 0s only for the byref CSEs. + // are 0s only for the CSEs that are killed. BitVecOps::RemoveElemD(cseLivenessTraits, cseAsyncKillsMask, getCSEAvailBit(inx)); BitVecOps::RemoveElemD(cseLivenessTraits, cseAsyncKillsMask, getCSEAvailCrossCallBit(inx)); anyAsyncKills = true; diff --git a/src/coreclr/jit/optcse.h b/src/coreclr/jit/optcse.h index 4dd088afb78c0e..97c2dde09ce688 100644 --- a/src/coreclr/jit/optcse.h +++ b/src/coreclr/jit/optcse.h @@ -436,6 +436,8 @@ struct CSEdsc } void ComputeNumLocals(Compiler* compiler); + + bool IsKilledAcrossAsyncSuspensions(Compiler* compiler); }; // The following class nested within CSE_Heuristic encapsulates the information From 837b8ceeecfc1258cbfe63252373c9b0adb03274 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 7 Jul 2026 21:15:37 +0200 Subject: [PATCH 2/3] Revert "Model it directly" This reverts commit 586760845e8e34c43c8130dbfa7d147dced51d9d. --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/optcse.cpp | 87 ++++++-------------------------------- src/coreclr/jit/optcse.h | 2 - 3 files changed, 13 insertions(+), 78 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index da70365b91948d..8ab52df52815b2 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7781,7 +7781,7 @@ class Compiler void optPrintCSEDataFlowSet(EXPSET_VALARG_TP cseDataFlowSet, bool includeBits = true); EXPSET_TP cseCallKillsMask; // Computed once - A mask that is used to kill available CSEs at callsites - EXPSET_TP cseAsyncKillsMask; // Computed once - A mask that is used to kill available CSEs at async suspension points + EXPSET_TP cseAsyncKillsMask; // Computed once - A mask that is used to kill available BYREF CSEs at async suspension points static const size_t s_optCSEhashSizeInitial; static const size_t s_optCSEhashGrowthFactor; diff --git a/src/coreclr/jit/optcse.cpp b/src/coreclr/jit/optcse.cpp index 8e75fff31ba444..d14d9ebd570bb9 100644 --- a/src/coreclr/jit/optcse.cpp +++ b/src/coreclr/jit/optcse.cpp @@ -465,79 +465,6 @@ void CSEdsc::ComputeNumLocals(Compiler* compiler) numLocalOccurrences = lcv.m_occurrences; } -//--------------------------------------------------------------------------- -// IsKilledAcrossAsyncSuspensions: -// Check if this CSE is considered killed across async suspension. -// -// Arguments: -// compiler - compiler instance -// -// Returns: -// True for byref and helpers that return values dependent on current thread. -// -bool CSEdsc::IsKilledAcrossAsyncSuspensions(Compiler* compiler) -{ - GenTree* tree = csdTreeList.tslTree; - if (tree->TypeIs(TYP_BYREF)) - { - return true; - } - - if (tree->TypeIs(TYP_STRUCT)) - { - ClassLayout* layout = tree->GetLayout(compiler); - if (layout->HasGCByRef()) - { - return true; - } - } - - // Handle static bases. Most are byref typed, so handled above, but some of - // these are native int typed (e.g. the pinned variants). - ValueNum vn = tree->gtVNPair.GetLiberal(); - vn = compiler->vnStore->VNNormalValue(vn); - VNFuncApp func; - if (compiler->vnStore->GetVNFunc(vn, &func)) - { - switch (func.GetFunc()) - { - case VNF_GetGcstaticBase: - case VNF_GetNongcstaticBase: - case VNF_GetdynamicGcstaticBase: - case VNF_GetdynamicNongcstaticBase: - case VNF_GetdynamicGcstaticBaseNoctor: - case VNF_GetdynamicNongcstaticBaseNoctor: - case VNF_ReadyToRunStaticBaseGC: - case VNF_ReadyToRunStaticBaseNonGC: - case VNF_ReadyToRunStaticBaseThread: - case VNF_ReadyToRunStaticBaseThreadNoctor: - case VNF_ReadyToRunStaticBaseThreadNonGC: - case VNF_ReadyToRunGenericStaticBase: - case VNF_GetpinnedGcstaticBase: - case VNF_GetpinnedNongcstaticBase: - case VNF_GetpinnedGcstaticBaseNoctor: - case VNF_GetpinnedNongcstaticBaseNoctor: - case VNF_GetGcthreadstaticBase: - case VNF_GetNongcthreadstaticBase: - case VNF_GetGcthreadstaticBaseNoctor: - case VNF_GetNongcthreadstaticBaseNoctor: - case VNF_GetdynamicGcthreadstaticBase: - case VNF_GetdynamicNongcthreadstaticBase: - case VNF_GetdynamicGcthreadstaticBaseNoctor: - case VNF_GetdynamicGcthreadstaticBaseNoctorOptimized: - case VNF_GetdynamicNongcthreadstaticBaseNoctor: - case VNF_GetdynamicNongcthreadstaticBaseNoctorOptimized: - case VNF_GetdynamicNongcthreadstaticBaseNoctorOptimized2: - case VNF_GetdynamicNongcthreadstaticBaseNoctorOptimized2NoJitOpt: - return true; - default: - break; - } - } - - return false; -} - /***************************************************************************** * * Initialize the Value Number CSE tracking logic. @@ -1186,11 +1113,21 @@ void Compiler::optValnumCSE_SetUpAsyncByrefKills() { CSEdsc* dsc = optCSEtab[inx - 1]; assert(dsc->csdIndex == inx); + bool isByRef = false; + if (dsc->csdTreeList.tslTree->TypeIs(TYP_BYREF)) + { + isByRef = true; + } + else if (dsc->csdTreeList.tslTree->TypeIs(TYP_STRUCT)) + { + ClassLayout* layout = dsc->csdTreeList.tslTree->GetLayout(this); + isByRef = layout->HasGCByRef(); + } - if (dsc->IsKilledAcrossAsyncSuspensions(this)) + if (isByRef) { // We generate a bit pattern like: 1111111100111100 where there - // are 0s only for the CSEs that are killed. + // are 0s only for the byref CSEs. BitVecOps::RemoveElemD(cseLivenessTraits, cseAsyncKillsMask, getCSEAvailBit(inx)); BitVecOps::RemoveElemD(cseLivenessTraits, cseAsyncKillsMask, getCSEAvailCrossCallBit(inx)); anyAsyncKills = true; diff --git a/src/coreclr/jit/optcse.h b/src/coreclr/jit/optcse.h index 97c2dde09ce688..4dd088afb78c0e 100644 --- a/src/coreclr/jit/optcse.h +++ b/src/coreclr/jit/optcse.h @@ -436,8 +436,6 @@ struct CSEdsc } void ComputeNumLocals(Compiler* compiler); - - bool IsKilledAcrossAsyncSuspensions(Compiler* compiler); }; // The following class nested within CSE_Heuristic encapsulates the information From 468f791c10487ee45a03d159672acd2c30c59b6e Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 7 Jul 2026 21:19:02 +0200 Subject: [PATCH 3/3] JIT: Make all static base helpers byref typed in async functions There were a handful of static base helpers that we treated as returning `TYP_I_IMPL`. At the same time VN considers these helpers to be invariant, allowing them to be CSE'd. However, these helpers are not invariant in async functions. Initially I explicitly marked VNs referring to these helpers as killed across async suspensions in CSE (see first commits on this PR). However, figuring out which VNs refer to those helpers is actually non-trivial when you take VNPhiDef and other functions into account. Instead this PR just changes the type of these helpers to let the standard byref handling pick them up. --- src/coreclr/jit/flowgraph.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 984022eeb991a4..5f67d303cc403d 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -758,7 +758,11 @@ GenTreeCall* Compiler::fgGetStaticsCCtorHelper(CORINFO_CLASS_HANDLE cls, CorInfo case CORINFO_HELP_GETPINNED_GCSTATIC_BASE: case CORINFO_HELP_GETPINNED_NONGCSTATIC_BASE: - type = TYP_I_IMPL; + // In async calls we model these helpers as byrefs to get "killed + // across suspensions" behavior for free, while also properly + // ensuring derived addresses are byref typed and are treated + // similarly. + type = compIsAsync() ? TYP_BYREF : TYP_I_IMPL; break; case CORINFO_HELP_INITCLASS: