From 01c2a1864fa09d381de00e73618cb4a6f8d0128b Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Thu, 30 Jan 2025 17:10:29 -0800 Subject: [PATCH 01/10] Replace HELPER_METHOD_FRAME with DynamicHelperFrame in JIT_Patchpoint and JIT_PartialCompilationPatchpoint - Consolidate the 2 functions so we only need 1 copy of the C++ code - Add asm helpers in the current officially supported architectures (The community ports will come later) --- src/coreclr/vm/amd64/AsmHelpers.asm | 19 + src/coreclr/vm/amd64/unixasmhelpers.S | 16 + src/coreclr/vm/arm/asmhelpers.S | 16 + src/coreclr/vm/arm64/asmhelpers.S | 17 + src/coreclr/vm/arm64/asmhelpers.asm | 18 + src/coreclr/vm/callingconvention.h | 24 ++ src/coreclr/vm/i386/asmhelpers.asm | 23 ++ src/coreclr/vm/jithelpers.cpp | 510 +++++++++----------------- src/coreclr/vm/jitinterface.h | 4 + src/coreclr/vm/prestub.cpp | 11 - 10 files changed, 314 insertions(+), 344 deletions(-) diff --git a/src/coreclr/vm/amd64/AsmHelpers.asm b/src/coreclr/vm/amd64/AsmHelpers.asm index 95714fd0757321..380893996db617 100644 --- a/src/coreclr/vm/amd64/AsmHelpers.asm +++ b/src/coreclr/vm/amd64/AsmHelpers.asm @@ -447,6 +447,25 @@ NESTED_ENTRY OnCallCountThresholdReachedStub, _TEXT TAILJMP_RAX NESTED_END OnCallCountThresholdReachedStub, _TEXT +extern JIT_PatchpointWorkerWorkerWithPolicy:proc + +NESTED_ENTRY JIT_Patchpoint, _TEXT + PROLOG_WITH_TRANSITION_BLOCK + + lea rcx, [rsp + __PWTB_TransitionBlock] ; TransitionBlock * + call JIT_PatchpointWorkerWorkerWithPolicy + + EPILOG_WITH_TRANSITION_BLOCK_RETURN + TAILJMP_RAX +NESTED_END JIT_Patchpoint, _TEXT + +; first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL +LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT + mov rdx, rcx + xor rcx, rcx + jmp JIT_Patchpoint +LEAF_END JIT_PartialCompilationPatchpoint, _TEXT + endif ; FEATURE_TIERED_COMPILATION LEAF_ENTRY JIT_PollGC, _TEXT diff --git a/src/coreclr/vm/amd64/unixasmhelpers.S b/src/coreclr/vm/amd64/unixasmhelpers.S index 7e404a2a09cf6c..8767da3e74b0b8 100644 --- a/src/coreclr/vm/amd64/unixasmhelpers.S +++ b/src/coreclr/vm/amd64/unixasmhelpers.S @@ -195,4 +195,20 @@ NESTED_ENTRY OnCallCountThresholdReachedStub, _TEXT, NoHandler TAILJMP_RAX NESTED_END OnCallCountThresholdReachedStub, _TEXT +NESTED_ENTRY JIT_Patchpoint, _TEXT, NoHandler + PROLOG_WITH_TRANSITION_BLOCK + + lea rdi, [rsp + __PWTB_TransitionBlock] // TransitionBlock * + call C_FUNC(JIT_PatchpointWorkerWorkerWithPolicy) + + EPILOG_WITH_TRANSITION_BLOCK_RETURN +NESTED_END JIT_Patchpoint, _TEXT + +; first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL +LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT + mov rsi, rdi + xor rdi, rdi + jmp JIT_Patchpoint +LEAF_END JIT_PartialCompilationPatchpoint, _TEXT + #endif // FEATURE_TIERED_COMPILATION diff --git a/src/coreclr/vm/arm/asmhelpers.S b/src/coreclr/vm/arm/asmhelpers.S index 5017a582f3ab8c..9d2ac0b8c7bc4b 100644 --- a/src/coreclr/vm/arm/asmhelpers.S +++ b/src/coreclr/vm/arm/asmhelpers.S @@ -913,6 +913,22 @@ ProbeLoop: EPILOG_BRANCH_REG r12 NESTED_END OnCallCountThresholdReachedStub, _TEXT + NESTED_ENTRY JIT_Patchpoint, _TEXT, NoHandler + PROLOG_WITH_TRANSITION_BLOCK + + add r0, sp, #__PWTB_TransitionBlock // TransitionBlock * + bl C_FUNC(JIT_PatchpointWorkerWorkerWithPolicy) + + EPILOG_WITH_TRANSITION_BLOCK_RETURN + NESTED_END JIT_Patchpoint, _TEXT + + // first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL + LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT + mov r1, r0 + mov r0, #0 + jmp JIT_Patchpoint + LEAF_END JIT_PartialCompilationPatchpoint, _TEXT + #endif // FEATURE_TIERED_COMPILATION LEAF_ENTRY JIT_PollGC, _TEXT diff --git a/src/coreclr/vm/arm64/asmhelpers.S b/src/coreclr/vm/arm64/asmhelpers.S index a6801090ad9b25..06fa3920289b15 100644 --- a/src/coreclr/vm/arm64/asmhelpers.S +++ b/src/coreclr/vm/arm64/asmhelpers.S @@ -747,6 +747,23 @@ NESTED_ENTRY OnCallCountThresholdReachedStub, _TEXT, NoHandler EPILOG_BRANCH_REG x9 NESTED_END OnCallCountThresholdReachedStub, _TEXT +NESTED_ENTRY JIT_Patchpoint, _TEXT, NoHandler + PROLOG_WITH_TRANSITION_BLOCK + + add x0, sp, #__PWTB_TransitionBlock ; TransitionBlock * + bl C_FUNC(JIT_PatchpointWorkerWorkerWithPolicy) + + EPILOG_WITH_TRANSITION_BLOCK_RETURN +NESTED_END JIT_Patchpoint, _TEXT + +// first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL +LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT + mov x1, x0 + mov x0, #0 + jmp JIT_Patchpoint +LEAF_END JIT_PartialCompilationPatchpoint, _TEXT + + #endif // FEATURE_TIERED_COMPILATION LEAF_ENTRY JIT_ValidateIndirectCall, _TEXT diff --git a/src/coreclr/vm/arm64/asmhelpers.asm b/src/coreclr/vm/arm64/asmhelpers.asm index e240919a7395f0..c3c41e4a076a02 100644 --- a/src/coreclr/vm/arm64/asmhelpers.asm +++ b/src/coreclr/vm/arm64/asmhelpers.asm @@ -1139,6 +1139,24 @@ __HelperNakedFuncName SETS "$helper":CC:"Naked" EPILOG_BRANCH_REG x9 NESTED_END + IMPORT JIT_PatchpointWorkerWorkerWithPolicy + + NESTED_ENTRY JIT_Patchpoint + PROLOG_WITH_TRANSITION_BLOCK + + add x0, sp, #__PWTB_TransitionBlock ; TransitionBlock * + bl JIT_PatchpointWorkerWorkerWithPolicy + + EPILOG_WITH_TRANSITION_BLOCK_RETURN + NESTED_END + + // first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL + LEAF_ENTRY JIT_PartialCompilationPatchpoint + mov x1, x0 + mov x0, #0 + jmp JIT_Patchpoint + LEAF_END + #endif ; FEATURE_TIERED_COMPILATION LEAF_ENTRY JIT_ValidateIndirectCall diff --git a/src/coreclr/vm/callingconvention.h b/src/coreclr/vm/callingconvention.h index d15c724a20b5e3..41539478e78723 100644 --- a/src/coreclr/vm/callingconvention.h +++ b/src/coreclr/vm/callingconvention.h @@ -2209,4 +2209,28 @@ inline BOOL IsRetBuffPassedAsFirstArg() #endif } +inline TADDR GetFirstArgumentRegisterValuePtr(TransitionBlock * pTransitionBlock) +{ + TADDR pArgument = (TADDR)pTransitionBlock + TransitionBlock::GetOffsetOfArgumentRegisters(); +#ifdef TARGET_X86 + // x86 is special as always + pArgument += offsetof(ArgumentRegisters, ECX); +#endif + + return pArgument; +} + +inline TADDR GetSecondArgumentRegisterValuePtr(TransitionBlock * pTransitionBlock) +{ + TADDR pArgument = (TADDR)pTransitionBlock + TransitionBlock::GetOffsetOfArgumentRegisters(); +#ifdef TARGET_X86 + // x86 is special as always + pArgument += offsetof(ArgumentRegisters, EDX); +#else + pArgument += sizeof(TADDR); +#endif + + return pArgument; +} + #endif // __CALLING_CONVENTION_INCLUDED diff --git a/src/coreclr/vm/i386/asmhelpers.asm b/src/coreclr/vm/i386/asmhelpers.asm index 2f1d12c6d9a71e..25fb40488ee235 100644 --- a/src/coreclr/vm/i386/asmhelpers.asm +++ b/src/coreclr/vm/i386/asmhelpers.asm @@ -1394,6 +1394,29 @@ _OnCallCountThresholdReachedStub@0 proc public ret _OnCallCountThresholdReachedStub@0 endp +_JIT_Patchpoint@0 proc public + STUB_PROLOG + mov esi, esp + push esi ; TransitionBlock * + + add x0, sp, #__PWTB_TransitionBlock ; TransitionBlock * + call _JIT_PatchpointWorkerWorkerWithPolicy@4 + + STUB_EPILOG + ret +_JIT_Patchpoint@0 endp + +// first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL +_JIT_PartialCompilationPatchpoint@0 proc public + mov edx, ecx + xor ecx, ecx + jmp JIT_Patchpoint + + ; This will never be executed. It is just to help out stack-walking logic + ; which disassembles the epilog to unwind the stack. + ret +_JIT_PartialCompilationPatchpoint@0 endp + endif ; FEATURE_TIERED_COMPILATION end diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index eeac550b3c0979..4cc41cb16020c7 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2341,22 +2341,6 @@ static PCODE JitPatchpointWorker(MethodDesc* pMD, EECodeInfo& codeInfo, int ilOf return osrVariant; } -// Helper method wrapper to set up a frame so we can invoke methods that might GC -HCIMPL3(PCODE, JIT_Patchpoint_Framed, MethodDesc* pMD, EECodeInfo& codeInfo, int ilOffset) -{ - PCODE result = (PCODE)NULL; - - HELPER_METHOD_FRAME_BEGIN_RET_0(); - - GCX_PREEMP(); - result = JitPatchpointWorker(pMD, codeInfo, ilOffset); - - HELPER_METHOD_FRAME_END(); - - return result; -} -HCIMPLEND - // Jit helper invoked at a patchpoint. // // Checks to see if this is a known patchpoint, if not, @@ -2368,17 +2352,23 @@ HCIMPLEND // Currently, counter is a pointer into the Tier0 method stack // frame so we have exclusive access. -void JIT_Patchpoint(int* counter, int ilOffset) +extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransitionBlock) { // BEGIN_PRESERVE_LAST_ERROR; DWORD dwLastError = ::GetLastError(); // This method may not return normally - STATIC_CONTRACT_GC_NOTRIGGER; + STATIC_CONTRACT_NOTHROW; + STATIC_CONTRACT_GC_TRIGGERS; STATIC_CONTRACT_MODE_COOPERATIVE; + PTR_PCODE pReturnAddress = (PTR_PCODE)(((BYTE*)pTransitionBlock) + TransitionBlock::GetOffsetOfReturnAddress()); + PCODE ip = *pReturnAddress; + int* counter = *(int**)GetFirstArgumentRegisterValuePtr(pTransitionBlock); + int ilOffset = *(int*)GetSecondArgumentRegisterValuePtr(pTransitionBlock); + int hitCount = 1; // This will stay at 1 for forced transition scenarios, but will be updated to the actual hit count for normal patch points + // Patchpoint identity is the helper return address - PCODE ip = (PCODE)_ReturnAddress(); // Fetch or setup patchpoint info for this patchpoint. EECodeInfo codeInfo(ip); @@ -2386,8 +2376,10 @@ void JIT_Patchpoint(int* counter, int ilOffset) LoaderAllocator* allocator = pMD->GetLoaderAllocator(); OnStackReplacementManager* manager = allocator->GetOnStackReplacementManager(); PerPatchpointInfo * ppInfo = manager->GetPerPatchpointInfo(ip); - PCODE osrMethodCode = (PCODE)NULL; + bool isNewMethod = false; + PCODE osrMethodCode = (PCODE)NULL; + // In the current prototype, counter is shared by all patchpoints // in a method, so no matter what happens below, we don't want to @@ -2402,8 +2394,13 @@ void JIT_Patchpoint(int* counter, int ilOffset) // // In the prototype, counter is a location in a stack frame, // so we can update it without worrying about other threads. - const int counterBump = g_pConfig->OSR_CounterBump(); - *counter = counterBump; + bool forceTransition = counter == NULL; + + if (!forceTransition) + { + const int counterBump = g_pConfig->OSR_CounterBump(); + *counter = counterBump; + } #ifdef _DEBUG const int ppId = ppInfo->m_patchpointId; @@ -2412,10 +2409,19 @@ void JIT_Patchpoint(int* counter, int ilOffset) // Is this a patchpoint that was previously marked as invalid? If so, just return to the Tier0 method. if ((ppInfo->m_flags & PerPatchpointInfo::patchpoint_invalid) == PerPatchpointInfo::patchpoint_invalid) { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", - ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); + if (!forceTransition) + { + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", + ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); - goto DONE; + goto DONE; + } + else + { + LOG((LF_TIEREDCOMPILATION, LL_FATALERROR, "Jit_PartialCompilationPatchpoint: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", + ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); + EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); + } } // See if we have an OSR method for this patchpoint. @@ -2438,7 +2444,7 @@ void JIT_Patchpoint(int* counter, int ilOffset) const int lowId = g_pConfig->OSR_LowId(); const int highId = g_pConfig->OSR_HighId(); - if ((ppId < lowId) || (ppId > highId)) + if (((ppId < lowId) || (ppId > highId)) && !forceTransition) { LOG((LF_TIEREDCOMPILATION, LL_INFO10, "Jit_Patchpoint: ignoring patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); @@ -2446,101 +2452,172 @@ void JIT_Patchpoint(int* counter, int ilOffset) } #endif - // Second, only request the OSR method if this patchpoint has - // been hit often enough. - // - // Note the initial invocation of the helper depends on the - // initial counter value baked into jitted code (call this J); - // subsequent invocations depend on the counter bump (call - // this B). - // - // J and B may differ, so the total number of loop iterations - // before an OSR method is created is: - // - // J, if hitLimit <= 1; - // J + (hitLimit-1)* B, if hitLimit > 1; - // - // Current thinking is: - // - // J should be in the range of tens to hundreds, so that newly - // called Tier0 methods that already have OSR methods - // available can transition to OSR methods quickly, but - // methods called only a few times do not invoke this - // helper and so create PerPatchpoint runtime state. - // - // B should be in the range of hundreds to thousands, so that - // we're not too eager to create OSR methods (since there is - // some jit cost), but are eager enough to transition before - // we run too much Tier0 code. - // - const int hitLimit = g_pConfig->OSR_HitLimit(); - const int hitCount = InterlockedIncrement(&ppInfo->m_patchpointCount); - const int hitLogLevel = (hitCount == 1) ? LL_INFO10 : LL_INFO1000; + if (!forceTransition) + { + // Second, only request the OSR method if this patchpoint has + // been hit often enough. + // + // Note the initial invocation of the helper depends on the + // initial counter value baked into jitted code (call this J); + // subsequent invocations depend on the counter bump (call + // this B). + // + // J and B may differ, so the total number of loop iterations + // before an OSR method is created is: + // + // J, if hitLimit <= 1; + // J + (hitLimit-1)* B, if hitLimit > 1; + // + // Current thinking is: + // + // J should be in the range of tens to hundreds, so that newly + // called Tier0 methods that already have OSR methods + // available can transition to OSR methods quickly, but + // methods called only a few times do not invoke this + // helper and so create PerPatchpoint runtime state. + // + // B should be in the range of hundreds to thousands, so that + // we're not too eager to create OSR methods (since there is + // some jit cost), but are eager enough to transition before + // we run too much Tier0 code. + // + const int hitLimit = g_pConfig->OSR_HitLimit(); + hitCount = InterlockedIncrement(&ppInfo->m_patchpointCount); + const int hitLogLevel = (hitCount == 1) ? LL_INFO10 : LL_INFO1000; - LOG((LF_TIEREDCOMPILATION, hitLogLevel, "Jit_Patchpoint: patchpoint [%d] (0x%p) hit %d in Method=0x%pM (%s::%s) [il offset %d] (limit %d)\n", - ppId, ip, hitCount, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset, hitLimit)); + LOG((LF_TIEREDCOMPILATION, hitLogLevel, "Jit_Patchpoint: patchpoint [%d] (0x%p) hit %d in Method=0x%pM (%s::%s) [il offset %d] (limit %d)\n", + ppId, ip, hitCount, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset, hitLimit)); - // Defer, if we haven't yet reached the limit - if (hitCount < hitLimit) - { - goto DONE; + // Defer, if we haven't yet reached the limit + if (hitCount < hitLimit) + { + goto DONE; + } } // Third, make sure no other thread is trying to create the OSR method. LONG oldFlags = ppInfo->m_flags; - if ((oldFlags & PerPatchpointInfo::patchpoint_triggered) == PerPatchpointInfo::patchpoint_triggered) + if (!forceTransition) { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); - goto DONE; - } + if ((oldFlags & PerPatchpointInfo::patchpoint_triggered) == PerPatchpointInfo::patchpoint_triggered) + { + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + goto DONE; + } - LONG newFlags = oldFlags | PerPatchpointInfo::patchpoint_triggered; - BOOL triggerTransition = InterlockedCompareExchange(&ppInfo->m_flags, newFlags, oldFlags) == oldFlags; + LONG newFlags = oldFlags | PerPatchpointInfo::patchpoint_triggered; + BOOL triggerTransition = InterlockedCompareExchange(&ppInfo->m_flags, newFlags, oldFlags) == oldFlags; - if (!triggerTransition) - { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); - goto DONE; + if (!triggerTransition) + { + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + goto DONE; + } } + } - // Time to create the OSR method. - // - // We currently do this synchronously. We could instead queue - // up a request on some worker thread, like we do for - // rejitting, and return control to the Tier0 method. It may - // eventually return here, if the patchpoint is hit often - // enough. - // - // There is a chance the async version will create methods - // that are never used (just like there is a chance that Tier1 - // methods are ever called). - // - // In this prototype we want to expose bugs in the jitted code - // for OSR methods, so we stick with synchronous creation. - LOG((LF_TIEREDCOMPILATION, LL_INFO10, "Jit_Patchpoint: patchpoint [%d] (0x%p) TRIGGER at count %d\n", ppId, ip, hitCount)); + if (osrMethodCode == NULL) + { + MAKE_CURRENT_THREAD_AVAILABLE(); + + #ifdef _DEBUG + Thread::ObjectRefFlush(CURRENT_THREAD); + #endif + + FrameWithCookie frame(pTransitionBlock, 0); + DynamicHelperFrame * pFrame = &frame; - // Invoke the helper to build the OSR method - osrMethodCode = HCCALL3(JIT_Patchpoint_Framed, pMD, codeInfo, ilOffset); + pFrame->Push(CURRENT_THREAD); + + INSTALL_MANAGED_EXCEPTION_DISPATCHER; + INSTALL_UNWIND_AND_CONTINUE_HANDLER; + + GCX_PREEMP(); + + osrMethodCode = ppInfo->m_osrMethodCode; + if ((osrMethodCode == (PCODE)NULL) && forceTransition) + { + // Another thread has already triggered the OSR method, wait until it is available, or the other thread abandons trying to JIT the OSR method + DWORD backoffs = 0; + while (osrMethodCode = ppInfo->m_osrMethodCode, osrMethodCode == (PCODE)NULL) + { + LONG oldFlags = ppInfo->m_flags; + if ((oldFlags & PerPatchpointInfo::patchpoint_triggered) == PerPatchpointInfo::patchpoint_triggered) + { + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_PartialCompilationPatchpoint: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + __SwitchToThread(0, backoffs++); + continue; + } + + LONG newFlags = ppInfo->m_flags | PerPatchpointInfo::patchpoint_triggered; + BOOL triggerTransition = InterlockedCompareExchange(&ppInfo->m_flags, newFlags, oldFlags) == oldFlags; + + if (!triggerTransition) + { + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_PartialCompilationPatchpoint: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + __SwitchToThread(0, backoffs++); + continue; + } + } + } - // If that failed, mark the patchpoint as invalid. if (osrMethodCode == (PCODE)NULL) { - // Unexpected, but not fatal - STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "Jit_Patchpoint: patchpoint (0x%p) OSR method creation failed," - " marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, pMD, ilOffset); + // Time to create the OSR method. + // + // We currently do this synchronously. We could instead queue + // up a request on some worker thread, like we do for + // rejitting, and return control to the Tier0 method. It may + // eventually return here, if the patchpoint is hit often + // enough. + // + // There is a chance the async version will create methods + // that are never used (just like there is a chance that Tier1 + // methods are ever called). + // + // In this prototype we want to expose bugs in the jitted code + // for OSR methods, so we stick with synchronous creation. + LOG((LF_TIEREDCOMPILATION, LL_INFO10, "Jit_Patchpoint: patchpoint [%d] (0x%p) TRIGGER at count %d\n", ppId, ip, hitCount)); - InterlockedOr(&ppInfo->m_flags, (LONG)PerPatchpointInfo::patchpoint_invalid); - goto DONE; + // Invoke the helper to build the OSR method + osrMethodCode = JitPatchpointWorker(pMD, codeInfo, ilOffset); + + // If that failed, mark the patchpoint as invalid. + if (osrMethodCode == (PCODE)NULL) + { + // Unexpected, but not fatal + STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "Jit_Patchpoint: patchpoint (0x%p) OSR method creation failed," + " marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, pMD, ilOffset); + + InterlockedOr(&ppInfo->m_flags, (LONG)PerPatchpointInfo::patchpoint_invalid); + + if (forceTransition) + { + EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); + } + } + else + { + // We've successfully created the osr method; make it available. + _ASSERTE(ppInfo->m_osrMethodCode == (PCODE)NULL); + ppInfo->m_osrMethodCode = osrMethodCode; + isNewMethod = true; + } } - // We've successfully created the osr method; make it available. - _ASSERTE(ppInfo->m_osrMethodCode == (PCODE)NULL); - ppInfo->m_osrMethodCode = osrMethodCode; - isNewMethod = true; + UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; + + pFrame->Pop(CURRENT_THREAD); + } + + if (osrMethodCode == (PCODE)NULL) + { + _ASSERTE(!forceTransition); + goto DONE; } // If we get here, we have code to transition to... - _ASSERTE(osrMethodCode != (PCODE)NULL); { Thread *pThread = GetThread(); @@ -2670,239 +2747,6 @@ void JIT_Patchpoint(int* counter, int ilOffset) ::SetLastError(dwLastError); } -// Jit helper invoked at a partial compilation patchpoint. -// -// Similar to Jit_Patchpoint, but invoked when execution -// reaches a point in a method where the continuation -// was never jitted (eg an exceptional path). -// -// Unlike regular patchpoints, partial compilation patchpoints -// must always transition. -// -HCIMPL1(VOID, JIT_PartialCompilationPatchpoint, int ilOffset) -{ - FCALL_CONTRACT; - - // BEGIN_PRESERVE_LAST_ERROR; - DWORD dwLastError = ::GetLastError(); - PerPatchpointInfo* ppInfo = NULL; - bool isNewMethod = false; - CONTEXT* pFrameContext = NULL; - -#if !defined(TARGET_WINDOWS) || !defined(TARGET_AMD64) - CONTEXT originalFrameContext; - originalFrameContext.ContextFlags = CONTEXT_FULL; - pFrameContext = &originalFrameContext; -#endif - - // Patchpoint identity is the helper return address - PCODE ip = (PCODE)_ReturnAddress(); - -#ifdef _DEBUG - // Friendly ID number - int ppId = 0; -#endif - - HELPER_METHOD_FRAME_BEGIN_0(); - - // Fetch or setup patchpoint info for this patchpoint. - EECodeInfo codeInfo(ip); - MethodDesc* pMD = codeInfo.GetMethodDesc(); - LoaderAllocator* allocator = pMD->GetLoaderAllocator(); - OnStackReplacementManager* manager = allocator->GetOnStackReplacementManager(); - ppInfo = manager->GetPerPatchpointInfo(ip); - -#ifdef _DEBUG - ppId = ppInfo->m_patchpointId; -#endif - - // See if we have an OSR method for this patchpoint. - DWORD backoffs = 0; - - // Enable GC while we jit or wait for the continuation to be jitted. - { - GCX_PREEMP(); - - while (ppInfo->m_osrMethodCode == (PCODE)NULL) - { - // Invalid patchpoints are fatal, for partial compilation patchpoints - // - if ((ppInfo->m_flags & PerPatchpointInfo::patchpoint_invalid) == PerPatchpointInfo::patchpoint_invalid) - { - LOG((LF_TIEREDCOMPILATION, LL_FATALERROR, "Jit_PartialCompilationPatchpoint: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", - ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); - EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); - } - - // Make sure no other thread is trying to create the OSR method. - // - LONG oldFlags = ppInfo->m_flags; - if ((oldFlags & PerPatchpointInfo::patchpoint_triggered) == PerPatchpointInfo::patchpoint_triggered) - { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_PartialCompilationPatchpoint: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); - __SwitchToThread(0, backoffs++); - continue; - } - - // Make sure we win the race to create the OSR method - // - LONG newFlags = ppInfo->m_flags | PerPatchpointInfo::patchpoint_triggered; - BOOL triggerTransition = InterlockedCompareExchange(&ppInfo->m_flags, newFlags, oldFlags) == oldFlags; - - if (!triggerTransition) - { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_PartialCompilationPatchpoint: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); - __SwitchToThread(0, backoffs++); - continue; - } - - // Invoke the helper to build the OSR method - // - // TODO: may not want to optimize this part of the method, if it's truly partial compilation - // and can't possibly rejoin into the main flow. - // - // (but consider: throw path in method with try/catch, OSR method will contain more than just the throw?) - // - LOG((LF_TIEREDCOMPILATION, LL_INFO10, "Jit_PartialCompilationPatchpoint: patchpoint [%d] (0x%p) TRIGGER\n", ppId, ip)); - PCODE newMethodCode = JitPatchpointWorker(pMD, codeInfo, ilOffset); - - // If that failed, mark the patchpoint as invalid. - // This is fatal, for partial compilation patchpoints - // - if (newMethodCode == (PCODE)NULL) - { - STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "Jit_PartialCompilationPatchpoint: patchpoint (0x%p) OSR method creation failed," - " marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, pMD, ilOffset); - InterlockedOr(&ppInfo->m_flags, (LONG)PerPatchpointInfo::patchpoint_invalid); - EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); - break; - } - - // We've successfully created the osr method; make it available. - _ASSERTE(ppInfo->m_osrMethodCode == (PCODE)NULL); - ppInfo->m_osrMethodCode = newMethodCode; - isNewMethod = true; - } - } - - // If we get here, we have code to transition to... - PCODE osrMethodCode = ppInfo->m_osrMethodCode; - _ASSERTE(osrMethodCode != (PCODE)NULL); - - Thread *pThread = GetThread(); - -#ifdef FEATURE_HIJACK - // We can't crawl the stack of a thread that currently has a hijack pending - // (since the hijack routine won't be recognized by any code manager). So we - // Undo any hijack, the EE will re-attempt it later. - pThread->UnhijackThread(); -#endif - - // Find context for the original method -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) - DWORD contextSize = 0; - ULONG64 xStateCompactionMask = 0; - DWORD contextFlags = CONTEXT_FULL; - if (Thread::AreShadowStacksEnabled()) - { - xStateCompactionMask = XSTATE_MASK_CET_U; - contextFlags |= CONTEXT_XSTATE; - } - - // The initialize call should fail but return contextSize - BOOL success = g_pfnInitializeContext2 ? - g_pfnInitializeContext2(NULL, contextFlags, NULL, &contextSize, xStateCompactionMask) : - InitializeContext(NULL, contextFlags, NULL, &contextSize); - - _ASSERTE(!success && (GetLastError() == ERROR_INSUFFICIENT_BUFFER)); - - PVOID pBuffer = _alloca(contextSize); - success = g_pfnInitializeContext2 ? - g_pfnInitializeContext2(pBuffer, contextFlags, &pFrameContext, &contextSize, xStateCompactionMask) : - InitializeContext(pBuffer, contextFlags, &pFrameContext, &contextSize); - _ASSERTE(success); -#else // TARGET_WINDOWS && TARGET_AMD64 - _ASSERTE(pFrameContext != nullptr); -#endif // TARGET_WINDOWS && TARGET_AMD64 - - // Find context for the original method - RtlCaptureContext(pFrameContext); - -#if defined(TARGET_WINDOWS) && defined(TARGET_AMD64) - if (Thread::AreShadowStacksEnabled()) - { - pFrameContext->ContextFlags |= CONTEXT_XSTATE; - SetXStateFeaturesMask(pFrameContext, xStateCompactionMask); - SetSSP(pFrameContext, _rdsspq()); - } -#endif // TARGET_WINDOWS && TARGET_AMD64 - - // Walk back to the original method frame - Thread::VirtualUnwindToFirstManagedCallFrame(pFrameContext); - - // Remember original method FP and SP because new method will inherit them. - UINT_PTR currentSP = GetSP(pFrameContext); - UINT_PTR currentFP = GetFP(pFrameContext); - - // We expect to be back at the right IP - if ((UINT_PTR)ip != GetIP(pFrameContext)) - { - // Should be fatal - STRESS_LOG2(LF_TIEREDCOMPILATION, LL_INFO10, "Jit_PartialCompilationPatchpoint: patchpoint (0x%p) TRANSITION" - " unexpected context IP 0x%p\n", ip, GetIP(pFrameContext)); - EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); - } - - // Now unwind back to the original method caller frame. - EECodeInfo callerCodeInfo(GetIP(pFrameContext)); - ULONG_PTR establisherFrame = 0; - PVOID handlerData = NULL; - RtlVirtualUnwind(UNW_FLAG_NHANDLER, callerCodeInfo.GetModuleBase(), GetIP(pFrameContext), callerCodeInfo.GetFunctionEntry(), - pFrameContext, &handlerData, &establisherFrame, NULL); - - // Now, set FP and SP back to the values they had just before this helper was called, - // since the new method must have access to the original method frame. - // - // TODO: if we access the patchpointInfo here, we can read out the FP-SP delta from there and - // use that to adjust the stack, likely saving some stack space. - -#if defined(TARGET_AMD64) - // If calls push the return address, we need to simulate that here, so the OSR - // method sees the "expected" SP misalgnment on entry. - _ASSERTE(currentSP % 16 == 0); - currentSP -= 8; - -#if defined(TARGET_WINDOWS) - DWORD64 ssp = GetSSP(pFrameContext); - if (ssp != 0) - { - SetSSP(pFrameContext, ssp - 8); - } -#endif // TARGET_WINDOWS - - pFrameContext->Rbp = currentFP; -#endif // TARGET_AMD64 - - SetSP(pFrameContext, currentSP); - - // Note we can get here w/o triggering, if there is an existing OSR method and - // we hit the patchpoint. - const int transitionLogLevel = isNewMethod ? LL_INFO10 : LL_INFO1000; - LOG((LF_TIEREDCOMPILATION, transitionLogLevel, "Jit_PartialCompilationPatchpoint: patchpoint [%d] (0x%p) TRANSITION to ip 0x%p\n", ppId, ip, osrMethodCode)); - - // Install new entry point as IP - SetIP(pFrameContext, osrMethodCode); - - // This method doesn't return normally so we have to manually restore things. - HELPER_METHOD_FRAME_END(); - ENDFORBIDGC(); - ::SetLastError(dwLastError); - - // Transition! - __asan_handle_no_return(); - ClrRestoreNonvolatileContext(pFrameContext); -} -HCIMPLEND #else diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 1d20db72f5f244..7e07f5a8afdabd 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -103,6 +103,10 @@ BOOL LoadDynamicInfoEntry(Module *currentModule, #endif // TARGET_X86 +// These must be implemented by the JIT in order to support the "jithelpers" functionality. +EXTERN_C FCDECL2(void, JIT_Patchpoint, int* counter, int ilOffset); +EXTERN_C FCDECL1(void, JIT_PartialCompilationPatchpoint, int ilOffset); + // // JIT HELPER ALIASING FOR PORTABILITY. // diff --git a/src/coreclr/vm/prestub.cpp b/src/coreclr/vm/prestub.cpp index a816f93b50c13b..ce0e1049654fe9 100644 --- a/src/coreclr/vm/prestub.cpp +++ b/src/coreclr/vm/prestub.cpp @@ -3558,17 +3558,6 @@ static PCODE getHelperForStaticBase(Module * pModule, CORCOMPILE_FIXUP_BLOB_KIND return pHelper; } -TADDR GetFirstArgumentRegisterValuePtr(TransitionBlock * pTransitionBlock) -{ - TADDR pArgument = (TADDR)pTransitionBlock + TransitionBlock::GetOffsetOfArgumentRegisters(); -#ifdef TARGET_X86 - // x86 is special as always - pArgument += offsetof(ArgumentRegisters, ECX); -#endif - - return pArgument; -} - void ProcessDynamicDictionaryLookup(TransitionBlock * pTransitionBlock, Module * pModule, ModuleBase * pInfoModule, From ac49b21754e756f8544a8a827d67523409817deb Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 31 Jan 2025 09:38:01 -0800 Subject: [PATCH 02/10] Attempt 2 at the asm helpers, hopefully fewer silly mistakes --- src/coreclr/vm/amd64/unixasmhelpers.S | 2 +- src/coreclr/vm/arm/asmhelpers.S | 16 ---------------- src/coreclr/vm/arm64/asmhelpers.S | 5 ++--- src/coreclr/vm/arm64/asmhelpers.asm | 2 +- src/coreclr/vm/i386/asmhelpers.asm | 23 ----------------------- src/coreclr/vm/loongarch64/asmhelpers.S | 16 ++++++++++++++++ src/coreclr/vm/riscv64/asmhelpers.S | 16 ++++++++++++++++ 7 files changed, 36 insertions(+), 44 deletions(-) diff --git a/src/coreclr/vm/amd64/unixasmhelpers.S b/src/coreclr/vm/amd64/unixasmhelpers.S index 8767da3e74b0b8..fc9a6ba5b69137 100644 --- a/src/coreclr/vm/amd64/unixasmhelpers.S +++ b/src/coreclr/vm/amd64/unixasmhelpers.S @@ -204,7 +204,7 @@ NESTED_ENTRY JIT_Patchpoint, _TEXT, NoHandler EPILOG_WITH_TRANSITION_BLOCK_RETURN NESTED_END JIT_Patchpoint, _TEXT -; first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL +// first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT mov rsi, rdi xor rdi, rdi diff --git a/src/coreclr/vm/arm/asmhelpers.S b/src/coreclr/vm/arm/asmhelpers.S index 9d2ac0b8c7bc4b..5017a582f3ab8c 100644 --- a/src/coreclr/vm/arm/asmhelpers.S +++ b/src/coreclr/vm/arm/asmhelpers.S @@ -913,22 +913,6 @@ ProbeLoop: EPILOG_BRANCH_REG r12 NESTED_END OnCallCountThresholdReachedStub, _TEXT - NESTED_ENTRY JIT_Patchpoint, _TEXT, NoHandler - PROLOG_WITH_TRANSITION_BLOCK - - add r0, sp, #__PWTB_TransitionBlock // TransitionBlock * - bl C_FUNC(JIT_PatchpointWorkerWorkerWithPolicy) - - EPILOG_WITH_TRANSITION_BLOCK_RETURN - NESTED_END JIT_Patchpoint, _TEXT - - // first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL - LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT - mov r1, r0 - mov r0, #0 - jmp JIT_Patchpoint - LEAF_END JIT_PartialCompilationPatchpoint, _TEXT - #endif // FEATURE_TIERED_COMPILATION LEAF_ENTRY JIT_PollGC, _TEXT diff --git a/src/coreclr/vm/arm64/asmhelpers.S b/src/coreclr/vm/arm64/asmhelpers.S index 06fa3920289b15..a89e059cbefc63 100644 --- a/src/coreclr/vm/arm64/asmhelpers.S +++ b/src/coreclr/vm/arm64/asmhelpers.S @@ -750,7 +750,7 @@ NESTED_END OnCallCountThresholdReachedStub, _TEXT NESTED_ENTRY JIT_Patchpoint, _TEXT, NoHandler PROLOG_WITH_TRANSITION_BLOCK - add x0, sp, #__PWTB_TransitionBlock ; TransitionBlock * + add x0, sp, #__PWTB_TransitionBlock // TransitionBlock * bl C_FUNC(JIT_PatchpointWorkerWorkerWithPolicy) EPILOG_WITH_TRANSITION_BLOCK_RETURN @@ -760,10 +760,9 @@ NESTED_END JIT_Patchpoint, _TEXT LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT mov x1, x0 mov x0, #0 - jmp JIT_Patchpoint + b JIT_Patchpoint LEAF_END JIT_PartialCompilationPatchpoint, _TEXT - #endif // FEATURE_TIERED_COMPILATION LEAF_ENTRY JIT_ValidateIndirectCall, _TEXT diff --git a/src/coreclr/vm/arm64/asmhelpers.asm b/src/coreclr/vm/arm64/asmhelpers.asm index c3c41e4a076a02..cd7c06500ee315 100644 --- a/src/coreclr/vm/arm64/asmhelpers.asm +++ b/src/coreclr/vm/arm64/asmhelpers.asm @@ -1154,7 +1154,7 @@ __HelperNakedFuncName SETS "$helper":CC:"Naked" LEAF_ENTRY JIT_PartialCompilationPatchpoint mov x1, x0 mov x0, #0 - jmp JIT_Patchpoint + b JIT_Patchpoint LEAF_END #endif ; FEATURE_TIERED_COMPILATION diff --git a/src/coreclr/vm/i386/asmhelpers.asm b/src/coreclr/vm/i386/asmhelpers.asm index 25fb40488ee235..2f1d12c6d9a71e 100644 --- a/src/coreclr/vm/i386/asmhelpers.asm +++ b/src/coreclr/vm/i386/asmhelpers.asm @@ -1394,29 +1394,6 @@ _OnCallCountThresholdReachedStub@0 proc public ret _OnCallCountThresholdReachedStub@0 endp -_JIT_Patchpoint@0 proc public - STUB_PROLOG - mov esi, esp - push esi ; TransitionBlock * - - add x0, sp, #__PWTB_TransitionBlock ; TransitionBlock * - call _JIT_PatchpointWorkerWorkerWithPolicy@4 - - STUB_EPILOG - ret -_JIT_Patchpoint@0 endp - -// first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL -_JIT_PartialCompilationPatchpoint@0 proc public - mov edx, ecx - xor ecx, ecx - jmp JIT_Patchpoint - - ; This will never be executed. It is just to help out stack-walking logic - ; which disassembles the epilog to unwind the stack. - ret -_JIT_PartialCompilationPatchpoint@0 endp - endif ; FEATURE_TIERED_COMPILATION end diff --git a/src/coreclr/vm/loongarch64/asmhelpers.S b/src/coreclr/vm/loongarch64/asmhelpers.S index 6be20320f9850c..5a5d86fe3800f1 100644 --- a/src/coreclr/vm/loongarch64/asmhelpers.S +++ b/src/coreclr/vm/loongarch64/asmhelpers.S @@ -1091,6 +1091,22 @@ NESTED_ENTRY OnCallCountThresholdReachedStub, _TEXT, NoHandler EPILOG_BRANCH_REG $t4 NESTED_END OnCallCountThresholdReachedStub, _TEXT +NESTED_ENTRY JIT_Patchpoint, _TEXT, NoHandler + PROLOG_WITH_TRANSITION_BLOCK + + addi.d $a0, $sp, __PWTB_TransitionBlock // TransitionBlock * + bl C_FUNC(JIT_PatchpointWorkerWorkerWithPolicy) + + EPILOG_WITH_TRANSITION_BLOCK_RETURN +NESTED_END JIT_Patchpoint, _TEXT + +// first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL +LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT + ori $a1, $a0, 0 + ori $a0, $zero, 0 + b JIT_Patchpoint +LEAF_END JIT_PartialCompilationPatchpoint, _TEXT + #endif // FEATURE_TIERED_COMPILATION // ------------------------------------------------------------------ diff --git a/src/coreclr/vm/riscv64/asmhelpers.S b/src/coreclr/vm/riscv64/asmhelpers.S index 12b2918ae31e63..ffd998567c0d33 100644 --- a/src/coreclr/vm/riscv64/asmhelpers.S +++ b/src/coreclr/vm/riscv64/asmhelpers.S @@ -958,6 +958,22 @@ NESTED_ENTRY OnCallCountThresholdReachedStub, _TEXT, NoHandler EPILOG_BRANCH_REG t4 NESTED_END OnCallCountThresholdReachedStub, _TEXT +NESTED_ENTRY JIT_Patchpoint, _TEXT, NoHandler + PROLOG_WITH_TRANSITION_BLOCK + + addi a0, sp, __PWTB_TransitionBlock // TransitionBlock * + call C_FUNC(JIT_PatchpointWorkerWorkerWithPolicy) + + EPILOG_WITH_TRANSITION_BLOCK_RETURN +NESTED_END JIT_Patchpoint, _TEXT + +// first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL +LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT + addi a1, a0, 0 + addi a0, zero, 0 + b JIT_Patchpoint +LEAF_END JIT_PartialCompilationPatchpoint, _TEXT + #endif // FEATURE_TIERED_COMPILATION // ------------------------------------------------------------------ From c1b79abd98fcf3e3f36eff4ce6013fbad9ce691b Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 31 Jan 2025 09:50:27 -0800 Subject: [PATCH 03/10] Fix Windows X86 build --- src/coreclr/vm/jitinterface.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 7e07f5a8afdabd..47bd80c0ac5e6a 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -104,7 +104,7 @@ BOOL LoadDynamicInfoEntry(Module *currentModule, #endif // TARGET_X86 // These must be implemented by the JIT in order to support the "jithelpers" functionality. -EXTERN_C FCDECL2(void, JIT_Patchpoint, int* counter, int ilOffset); +EXTERN_C void JIT_Patchpoint(int* counter, int ilOffset); EXTERN_C FCDECL1(void, JIT_PartialCompilationPatchpoint, int ilOffset); // From 483aedb019527c7b512e8b69b272c41ec8edceb7 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Fri, 31 Jan 2025 11:42:49 -0800 Subject: [PATCH 04/10] Fixes for issues found in CI as well as some corrections to assembly noted by @am11 --- src/coreclr/vm/amd64/unixasmhelpers.S | 2 +- src/coreclr/vm/arm64/asmhelpers.S | 2 +- src/coreclr/vm/jithelpers.cpp | 2 +- src/coreclr/vm/loongarch64/asmhelpers.S | 6 +++--- src/coreclr/vm/riscv64/asmhelpers.S | 6 +++--- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/coreclr/vm/amd64/unixasmhelpers.S b/src/coreclr/vm/amd64/unixasmhelpers.S index fc9a6ba5b69137..10ab11933caee4 100644 --- a/src/coreclr/vm/amd64/unixasmhelpers.S +++ b/src/coreclr/vm/amd64/unixasmhelpers.S @@ -208,7 +208,7 @@ NESTED_END JIT_Patchpoint, _TEXT LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT mov rsi, rdi xor rdi, rdi - jmp JIT_Patchpoint + jmp C_FUNC(JIT_Patchpoint) LEAF_END JIT_PartialCompilationPatchpoint, _TEXT #endif // FEATURE_TIERED_COMPILATION diff --git a/src/coreclr/vm/arm64/asmhelpers.S b/src/coreclr/vm/arm64/asmhelpers.S index a89e059cbefc63..fdfa833d250f7b 100644 --- a/src/coreclr/vm/arm64/asmhelpers.S +++ b/src/coreclr/vm/arm64/asmhelpers.S @@ -760,7 +760,7 @@ NESTED_END JIT_Patchpoint, _TEXT LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT mov x1, x0 mov x0, #0 - b JIT_Patchpoint + b C_FUNC(JIT_Patchpoint) LEAF_END JIT_PartialCompilationPatchpoint, _TEXT #endif // FEATURE_TIERED_COMPILATION diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 4cc41cb16020c7..ec247d5af59954 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2516,7 +2516,7 @@ extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransiti } } - if (osrMethodCode == NULL) + if (osrMethodCode == (PCODE)NULL) { MAKE_CURRENT_THREAD_AVAILABLE(); diff --git a/src/coreclr/vm/loongarch64/asmhelpers.S b/src/coreclr/vm/loongarch64/asmhelpers.S index 5a5d86fe3800f1..abf769e7b52e64 100644 --- a/src/coreclr/vm/loongarch64/asmhelpers.S +++ b/src/coreclr/vm/loongarch64/asmhelpers.S @@ -1102,9 +1102,9 @@ NESTED_END JIT_Patchpoint, _TEXT // first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT - ori $a1, $a0, 0 - ori $a0, $zero, 0 - b JIT_Patchpoint + move $a1, $a0 + li.d $a0, 0 + b C_FUNC(JIT_Patchpoint) LEAF_END JIT_PartialCompilationPatchpoint, _TEXT #endif // FEATURE_TIERED_COMPILATION diff --git a/src/coreclr/vm/riscv64/asmhelpers.S b/src/coreclr/vm/riscv64/asmhelpers.S index ffd998567c0d33..75c5f7609c93bf 100644 --- a/src/coreclr/vm/riscv64/asmhelpers.S +++ b/src/coreclr/vm/riscv64/asmhelpers.S @@ -969,9 +969,9 @@ NESTED_END JIT_Patchpoint, _TEXT // first arg register holds iloffset, which needs to be moved to the second register, and the first register filled with NULL LEAF_ENTRY JIT_PartialCompilationPatchpoint, _TEXT - addi a1, a0, 0 - addi a0, zero, 0 - b JIT_Patchpoint + mv a1, a0 + li a0, 0 + j C_FUNC(JIT_Patchpoint) LEAF_END JIT_PartialCompilationPatchpoint, _TEXT #endif // FEATURE_TIERED_COMPILATION From b10459b6c1c3e626664b6f48732c433cb6f5acb3 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 3 Feb 2025 10:44:16 -0800 Subject: [PATCH 05/10] Refactor the logic for the patchpoints to extract the two policies into two easily distinguished functions. --- src/coreclr/vm/codeman.h | 8 +- src/coreclr/vm/jithelpers.cpp | 398 +++++++++++++++++++------------- src/coreclr/vm/jitinterface.cpp | 4 +- 3 files changed, 247 insertions(+), 163 deletions(-) diff --git a/src/coreclr/vm/codeman.h b/src/coreclr/vm/codeman.h index 87b8bfd27f03a6..61bd8b07cdb32c 100644 --- a/src/coreclr/vm/codeman.h +++ b/src/coreclr/vm/codeman.h @@ -2477,7 +2477,7 @@ class EECodeInfo TADDR GetSavedMethodCode(); - TADDR GetStartAddress(); + TADDR GetStartAddress() const; BOOL IsValid() { @@ -2505,15 +2505,15 @@ class EECodeInfo } // This returns a pointer to the start of an instruction; conceptually, a PINSTR. - TADDR GetCodeAddress() + TADDR GetCodeAddress() const { LIMITED_METHOD_DAC_CONTRACT; return PCODEToPINSTR(m_codeAddress); } - NativeCodeVersion GetNativeCodeVersion(); + NativeCodeVersion GetNativeCodeVersion() const; - MethodDesc * GetMethodDesc() + MethodDesc * GetMethodDesc() const { LIMITED_METHOD_DAC_CONTRACT; return m_pMD; diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index ec247d5af59954..6e89323ad547ca 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2296,7 +2296,7 @@ HCIMPLEND // // Returns the address of the jitted code. // Returns NULL if osr method can't be created. -static PCODE JitPatchpointWorker(MethodDesc* pMD, EECodeInfo& codeInfo, int ilOffset) +static PCODE JitPatchpointWorker(MethodDesc* pMD, const EECodeInfo& codeInfo, int ilOffset) { STANDARD_VM_CONTRACT; PCODE osrVariant = (PCODE)NULL; @@ -2341,47 +2341,20 @@ static PCODE JitPatchpointWorker(MethodDesc* pMD, EECodeInfo& codeInfo, int ilOf return osrVariant; } -// Jit helper invoked at a patchpoint. -// -// Checks to see if this is a known patchpoint, if not, -// an entry is added to the patchpoint table. -// -// When the patchpoint has been hit often enough to trigger -// a transition, create an OSR method. -// -// Currently, counter is a pointer into the Tier0 method stack -// frame so we have exclusive access. - -extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransitionBlock) +PCODE JIT_PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* counter, int ilOffset, PerPatchpointInfo * ppInfo, const EECodeInfo& codeInfo, bool *pIsNewMethod) { - // BEGIN_PRESERVE_LAST_ERROR; - DWORD dwLastError = ::GetLastError(); - - // This method may not return normally STATIC_CONTRACT_NOTHROW; STATIC_CONTRACT_GC_TRIGGERS; STATIC_CONTRACT_MODE_COOPERATIVE; - PTR_PCODE pReturnAddress = (PTR_PCODE)(((BYTE*)pTransitionBlock) + TransitionBlock::GetOffsetOfReturnAddress()); - PCODE ip = *pReturnAddress; - int* counter = *(int**)GetFirstArgumentRegisterValuePtr(pTransitionBlock); - int ilOffset = *(int*)GetSecondArgumentRegisterValuePtr(pTransitionBlock); - int hitCount = 1; // This will stay at 1 for forced transition scenarios, but will be updated to the actual hit count for normal patch points - - // Patchpoint identity is the helper return address + // See if we have an OSR method for this patchpoint. + PCODE osrMethodCode = ppInfo->m_osrMethodCode; + *pIsNewMethod = false; + TADDR ip = codeInfo.GetCodeAddress(); - // Fetch or setup patchpoint info for this patchpoint. - EECodeInfo codeInfo(ip); MethodDesc* pMD = codeInfo.GetMethodDesc(); - LoaderAllocator* allocator = pMD->GetLoaderAllocator(); - OnStackReplacementManager* manager = allocator->GetOnStackReplacementManager(); - PerPatchpointInfo * ppInfo = manager->GetPerPatchpointInfo(ip); - - bool isNewMethod = false; - PCODE osrMethodCode = (PCODE)NULL; - - // In the current prototype, counter is shared by all patchpoints + // In the current implementation, counter is shared by all patchpoints // in a method, so no matter what happens below, we don't want to // impair those other patchpoints. // @@ -2392,41 +2365,23 @@ extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransiti // // So we always reset the counter to the bump value. // - // In the prototype, counter is a location in a stack frame, + // In the implementation, counter is a location in a stack frame, // so we can update it without worrying about other threads. - bool forceTransition = counter == NULL; - - if (!forceTransition) - { - const int counterBump = g_pConfig->OSR_CounterBump(); - *counter = counterBump; - } + const int counterBump = g_pConfig->OSR_CounterBump(); + *counter = counterBump; #ifdef _DEBUG const int ppId = ppInfo->m_patchpointId; #endif - // Is this a patchpoint that was previously marked as invalid? If so, just return to the Tier0 method. if ((ppInfo->m_flags & PerPatchpointInfo::patchpoint_invalid) == PerPatchpointInfo::patchpoint_invalid) { - if (!forceTransition) - { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", - ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "JIT_PatchpointOptimizationPolicy: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", + ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); - goto DONE; - } - else - { - LOG((LF_TIEREDCOMPILATION, LL_FATALERROR, "Jit_PartialCompilationPatchpoint: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", - ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); - EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); - } + goto DONE; } - // See if we have an OSR method for this patchpoint. - osrMethodCode = ppInfo->m_osrMethodCode; - if (osrMethodCode == (PCODE)NULL) { // No OSR method yet, let's see if we should create one. @@ -2444,80 +2399,71 @@ extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransiti const int lowId = g_pConfig->OSR_LowId(); const int highId = g_pConfig->OSR_HighId(); - if (((ppId < lowId) || (ppId > highId)) && !forceTransition) + if ((ppId < lowId) || (ppId > highId)) { - LOG((LF_TIEREDCOMPILATION, LL_INFO10, "Jit_Patchpoint: ignoring patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", + LOG((LF_TIEREDCOMPILATION, LL_INFO10, "JIT_PatchpointOptimizationPolicy: ignoring patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); goto DONE; } #endif - if (!forceTransition) - { - // Second, only request the OSR method if this patchpoint has - // been hit often enough. - // - // Note the initial invocation of the helper depends on the - // initial counter value baked into jitted code (call this J); - // subsequent invocations depend on the counter bump (call - // this B). - // - // J and B may differ, so the total number of loop iterations - // before an OSR method is created is: - // - // J, if hitLimit <= 1; - // J + (hitLimit-1)* B, if hitLimit > 1; - // - // Current thinking is: - // - // J should be in the range of tens to hundreds, so that newly - // called Tier0 methods that already have OSR methods - // available can transition to OSR methods quickly, but - // methods called only a few times do not invoke this - // helper and so create PerPatchpoint runtime state. - // - // B should be in the range of hundreds to thousands, so that - // we're not too eager to create OSR methods (since there is - // some jit cost), but are eager enough to transition before - // we run too much Tier0 code. - // - const int hitLimit = g_pConfig->OSR_HitLimit(); - hitCount = InterlockedIncrement(&ppInfo->m_patchpointCount); - const int hitLogLevel = (hitCount == 1) ? LL_INFO10 : LL_INFO1000; + // Second, only request the OSR method if this patchpoint has + // been hit often enough. + // + // Note the initial invocation of the helper depends on the + // initial counter value baked into jitted code (call this J); + // subsequent invocations depend on the counter bump (call + // this B). + // + // J and B may differ, so the total number of loop iterations + // before an OSR method is created is: + // + // J, if hitLimit <= 1; + // J + (hitLimit-1)* B, if hitLimit > 1; + // + // Current thinking is: + // + // J should be in the range of tens to hundreds, so that newly + // called Tier0 methods that already have OSR methods + // available can transition to OSR methods quickly, but + // methods called only a few times do not invoke this + // helper and so create PerPatchpoint runtime state. + // + // B should be in the range of hundreds to thousands, so that + // we're not too eager to create OSR methods (since there is + // some jit cost), but are eager enough to transition before + // we run too much Tier0 code. + // + const int hitLimit = g_pConfig->OSR_HitLimit(); + const int hitCount = InterlockedIncrement(&ppInfo->m_patchpointCount); + const int hitLogLevel = (hitCount == 1) ? LL_INFO10 : LL_INFO1000; - LOG((LF_TIEREDCOMPILATION, hitLogLevel, "Jit_Patchpoint: patchpoint [%d] (0x%p) hit %d in Method=0x%pM (%s::%s) [il offset %d] (limit %d)\n", - ppId, ip, hitCount, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset, hitLimit)); + LOG((LF_TIEREDCOMPILATION, hitLogLevel, "JIT_PatchpointOptimizationPolicy: patchpoint [%d] (0x%p) hit %d in Method=0x%pM (%s::%s) [il offset %d] (limit %d)\n", + ppId, ip, hitCount, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset, hitLimit)); - // Defer, if we haven't yet reached the limit - if (hitCount < hitLimit) - { - goto DONE; - } + // Defer, if we haven't yet reached the limit + if (hitCount < hitLimit) + { + goto DONE; } // Third, make sure no other thread is trying to create the OSR method. LONG oldFlags = ppInfo->m_flags; - if (!forceTransition) + if ((oldFlags & PerPatchpointInfo::patchpoint_triggered) == PerPatchpointInfo::patchpoint_triggered) { - if ((oldFlags & PerPatchpointInfo::patchpoint_triggered) == PerPatchpointInfo::patchpoint_triggered) - { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); - goto DONE; - } + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "JIT_PatchpointOptimizationPolicy: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + goto DONE; + } - LONG newFlags = oldFlags | PerPatchpointInfo::patchpoint_triggered; - BOOL triggerTransition = InterlockedCompareExchange(&ppInfo->m_flags, newFlags, oldFlags) == oldFlags; + LONG newFlags = oldFlags | PerPatchpointInfo::patchpoint_triggered; + BOOL triggerTransition = InterlockedCompareExchange(&ppInfo->m_flags, newFlags, oldFlags) == oldFlags; - if (!triggerTransition) - { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_Patchpoint: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); - goto DONE; - } + if (!triggerTransition) + { + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "JIT_PatchpointOptimizationPolicy: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + goto DONE; } - } - if (osrMethodCode == (PCODE)NULL) - { MAKE_CURRENT_THREAD_AVAILABLE(); #ifdef _DEBUG @@ -2535,32 +2481,6 @@ extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransiti GCX_PREEMP(); osrMethodCode = ppInfo->m_osrMethodCode; - if ((osrMethodCode == (PCODE)NULL) && forceTransition) - { - // Another thread has already triggered the OSR method, wait until it is available, or the other thread abandons trying to JIT the OSR method - DWORD backoffs = 0; - while (osrMethodCode = ppInfo->m_osrMethodCode, osrMethodCode == (PCODE)NULL) - { - LONG oldFlags = ppInfo->m_flags; - if ((oldFlags & PerPatchpointInfo::patchpoint_triggered) == PerPatchpointInfo::patchpoint_triggered) - { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_PartialCompilationPatchpoint: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); - __SwitchToThread(0, backoffs++); - continue; - } - - LONG newFlags = ppInfo->m_flags | PerPatchpointInfo::patchpoint_triggered; - BOOL triggerTransition = InterlockedCompareExchange(&ppInfo->m_flags, newFlags, oldFlags) == oldFlags; - - if (!triggerTransition) - { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "Jit_PartialCompilationPatchpoint: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); - __SwitchToThread(0, backoffs++); - continue; - } - } - } - if (osrMethodCode == (PCODE)NULL) { // Time to create the OSR method. @@ -2575,9 +2495,9 @@ extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransiti // that are never used (just like there is a chance that Tier1 // methods are ever called). // - // In this prototype we want to expose bugs in the jitted code + // We want to expose bugs in the jitted code // for OSR methods, so we stick with synchronous creation. - LOG((LF_TIEREDCOMPILATION, LL_INFO10, "Jit_Patchpoint: patchpoint [%d] (0x%p) TRIGGER at count %d\n", ppId, ip, hitCount)); + LOG((LF_TIEREDCOMPILATION, LL_INFO10, "JIT_PatchpointOptimizationPolicy: patchpoint [%d] (0x%p) TRIGGER at count %d\n", ppId, ip, hitCount)); // Invoke the helper to build the OSR method osrMethodCode = JitPatchpointWorker(pMD, codeInfo, ilOffset); @@ -2586,23 +2506,13 @@ extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransiti if (osrMethodCode == (PCODE)NULL) { // Unexpected, but not fatal - STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "Jit_Patchpoint: patchpoint (0x%p) OSR method creation failed," + STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "JIT_PatchpointOptimizationPolicy: patchpoint (0x%p) OSR method creation failed," " marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, pMD, ilOffset); InterlockedOr(&ppInfo->m_flags, (LONG)PerPatchpointInfo::patchpoint_invalid); - - if (forceTransition) - { - EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); - } - } - else - { - // We've successfully created the osr method; make it available. - _ASSERTE(ppInfo->m_osrMethodCode == (PCODE)NULL); - ppInfo->m_osrMethodCode = osrMethodCode; - isNewMethod = true; } + + *pIsNewMethod = true; } UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; @@ -2610,10 +2520,184 @@ extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransiti pFrame->Pop(CURRENT_THREAD); } + return osrMethodCode; + +DONE: + return (PCODE)NULL; +} + +PCODE JIT_PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* counter, int ilOffset, PerPatchpointInfo * ppInfo, const EECodeInfo& codeInfo, bool *pIsNewMethod) +{ + STATIC_CONTRACT_NOTHROW; + STATIC_CONTRACT_GC_TRIGGERS; + STATIC_CONTRACT_MODE_COOPERATIVE; + + *pIsNewMethod = false; + MethodDesc* pMD = codeInfo.GetMethodDesc(); + TADDR ip = codeInfo.GetCodeAddress(); + +#ifdef _DEBUG + const int ppId = ppInfo->m_patchpointId; +#endif + + if ((ppInfo->m_flags & PerPatchpointInfo::patchpoint_invalid) == PerPatchpointInfo::patchpoint_invalid) + { + LOG((LF_TIEREDCOMPILATION, LL_FATALERROR, "JIT_PatchpointRequiredPolicy: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", + ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); + EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); + } + + MAKE_CURRENT_THREAD_AVAILABLE(); + +#ifdef _DEBUG + Thread::ObjectRefFlush(CURRENT_THREAD); +#endif + + FrameWithCookie frame(pTransitionBlock, 0); + DynamicHelperFrame * pFrame = &frame; + + pFrame->Push(CURRENT_THREAD); + + INSTALL_MANAGED_EXCEPTION_DISPATCHER; + INSTALL_UNWIND_AND_CONTINUE_HANDLER; + + { + GCX_PREEMP(); + + DWORD backoffs = 0; + while (ppInfo->m_osrMethodCode == (PCODE)NULL) + { + // Invalid patchpoints are fatal, for partial compilation patchpoints + // + if ((ppInfo->m_flags & PerPatchpointInfo::patchpoint_invalid) == PerPatchpointInfo::patchpoint_invalid) + { + LOG((LF_TIEREDCOMPILATION, LL_FATALERROR, "JIT_PatchpointRequiredPolicy: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", + ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); + EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); + } + + // Make sure no other thread is trying to create the OSR method. + // + LONG oldFlags = ppInfo->m_flags; + if ((oldFlags & PerPatchpointInfo::patchpoint_triggered) == PerPatchpointInfo::patchpoint_triggered) + { + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "JIT_PatchpointRequiredPolicy: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + __SwitchToThread(0, backoffs++); + continue; + } + + // Make sure we win the race to create the OSR method + // + LONG newFlags = ppInfo->m_flags | PerPatchpointInfo::patchpoint_triggered; + BOOL triggerTransition = InterlockedCompareExchange(&ppInfo->m_flags, newFlags, oldFlags) == oldFlags; + + if (!triggerTransition) + { + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "JIT_PatchpointRequiredPolicy: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + __SwitchToThread(0, backoffs++); + continue; + } + + // Invoke the helper to build the OSR method + // + // TODO: may not want to optimize this part of the method, if it's truly partial compilation + // and can't possibly rejoin into the main flow. + // + // (but consider: throw path in method with try/catch, OSR method will contain more than just the throw?) + // + LOG((LF_TIEREDCOMPILATION, LL_INFO10, "JIT_PatchpointRequiredPolicy: patchpoint [%d] (0x%p) TRIGGER\n", ppId, ip)); + PCODE newMethodCode = JitPatchpointWorker(pMD, codeInfo, ilOffset); + + // If that failed, mark the patchpoint as invalid. + // This is fatal, for partial compilation patchpoints + // + if (newMethodCode == (PCODE)NULL) + { + STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "JIT_PatchpointRequiredPolicy: patchpoint (0x%p) OSR method creation failed," + " marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, pMD, ilOffset); + InterlockedOr(&ppInfo->m_flags, (LONG)PerPatchpointInfo::patchpoint_invalid); + EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); + break; + } + + // We've successfully created the osr method; make it available. + _ASSERTE(ppInfo->m_osrMethodCode == (PCODE)NULL); + ppInfo->m_osrMethodCode = newMethodCode; + *pIsNewMethod = true; + } + } + + UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; + UNINSTALL_MANAGED_EXCEPTION_DISPATCHER; + + pFrame->Pop(CURRENT_THREAD); + + // If we get here, we have code to transition to... + PCODE osrMethodCode = ppInfo->m_osrMethodCode; + _ASSERTE(osrMethodCode != (PCODE)NULL); + + return osrMethodCode; +} + +// Jit helper invoked at a patchpoint. +// +// Checks to see if this is a known patchpoint, if not, +// an entry is added to the patchpoint table. +// +// When the patchpoint has been hit often enough to trigger +// a transition, create an OSR method. OR if the first argument +// is NULL, always create an OSR method and transition to it. +// +// Currently, counter(the first argument) is a pointer into the Tier0 method stack +// frame so we have exclusive access. + +extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransitionBlock) +{ + // BEGIN_PRESERVE_LAST_ERROR; + DWORD dwLastError = ::GetLastError(); + + // This method may not return normally + STATIC_CONTRACT_NOTHROW; + STATIC_CONTRACT_GC_TRIGGERS; + STATIC_CONTRACT_MODE_COOPERATIVE; + + PTR_PCODE pReturnAddress = (PTR_PCODE)(((BYTE*)pTransitionBlock) + TransitionBlock::GetOffsetOfReturnAddress()); + PCODE ip = *pReturnAddress; + int* counter = *(int**)GetFirstArgumentRegisterValuePtr(pTransitionBlock); + int ilOffset = *(int*)GetSecondArgumentRegisterValuePtr(pTransitionBlock); + int hitCount = 1; // This will stay at 1 for forced transition scenarios, but will be updated to the actual hit count for normal patch points + + // Patchpoint identity is the helper return address + + // Fetch or setup patchpoint info for this patchpoint. + EECodeInfo codeInfo(ip); + MethodDesc* pMD = codeInfo.GetMethodDesc(); + LoaderAllocator* allocator = pMD->GetLoaderAllocator(); + OnStackReplacementManager* manager = allocator->GetOnStackReplacementManager(); + PerPatchpointInfo * ppInfo = manager->GetPerPatchpointInfo(ip); + +#ifdef _DEBUG + const int ppId = ppInfo->m_patchpointId; +#endif + + bool isNewMethod = false; + PCODE osrMethodCode = (PCODE)NULL; + + + bool patchpointMustFindOptimizedCode = counter == NULL; + + if (patchpointMustFindOptimizedCode) + { + osrMethodCode = JIT_PatchpointRequiredPolicy(pTransitionBlock, counter, ilOffset, ppInfo, codeInfo, &isNewMethod); + } + else + { + osrMethodCode = JIT_PatchpointOptimizationPolicy(pTransitionBlock, counter, ilOffset, ppInfo, codeInfo, &isNewMethod); + } if (osrMethodCode == (PCODE)NULL) { - _ASSERTE(!forceTransition); + _ASSERTE(!patchpointMustFindOptimizedCode); goto DONE; } diff --git a/src/coreclr/vm/jitinterface.cpp b/src/coreclr/vm/jitinterface.cpp index e284b74639202c..35e06c4041e482 100644 --- a/src/coreclr/vm/jitinterface.cpp +++ b/src/coreclr/vm/jitinterface.cpp @@ -14537,7 +14537,7 @@ TADDR EECodeInfo::GetSavedMethodCode() return GetStartAddress(); } -TADDR EECodeInfo::GetStartAddress() +TADDR EECodeInfo::GetStartAddress() const { CONTRACTL { NOTHROW; @@ -14548,7 +14548,7 @@ TADDR EECodeInfo::GetStartAddress() return m_pJM->JitTokenToStartAddress(m_methodToken); } -NativeCodeVersion EECodeInfo::GetNativeCodeVersion() +NativeCodeVersion EECodeInfo::GetNativeCodeVersion() const { CONTRACTL { From f87e4b962b2660601d968bb3ecdf6538b3fec896 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Mon, 3 Feb 2025 11:35:46 -0800 Subject: [PATCH 06/10] Fix issue where we didn't update the m_osrMethodCode field in the ppInfo --- src/coreclr/vm/jithelpers.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 6e89323ad547ca..f18a16c822023a 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2511,8 +2511,11 @@ PCODE JIT_PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* c InterlockedOr(&ppInfo->m_flags, (LONG)PerPatchpointInfo::patchpoint_invalid); } - - *pIsNewMethod = true; + else + { + *pIsNewMethod = true; + ppInfo->m_osrMethodCode = osrMethodCode; + } } UNINSTALL_UNWIND_AND_CONTINUE_HANDLER; From f5208bf3daab448cfc960069be711cede74a3386 Mon Sep 17 00:00:00 2001 From: David Wrighton Date: Tue, 4 Feb 2025 17:00:42 -0800 Subject: [PATCH 07/10] Hopefully final round of code review feedback --- src/coreclr/vm/jithelpers.cpp | 39 ++++++++++++++++++----------------- src/coreclr/vm/jitinterface.h | 4 ++-- 2 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index f18a16c822023a..0a98a480c32df1 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2341,7 +2341,7 @@ static PCODE JitPatchpointWorker(MethodDesc* pMD, const EECodeInfo& codeInfo, in return osrVariant; } -PCODE JIT_PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* counter, int ilOffset, PerPatchpointInfo * ppInfo, const EECodeInfo& codeInfo, bool *pIsNewMethod) +PCODE PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* counter, int ilOffset, PerPatchpointInfo * ppInfo, const EECodeInfo& codeInfo, bool *pIsNewMethod) { STATIC_CONTRACT_NOTHROW; STATIC_CONTRACT_GC_TRIGGERS; @@ -2376,7 +2376,7 @@ PCODE JIT_PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* c if ((ppInfo->m_flags & PerPatchpointInfo::patchpoint_invalid) == PerPatchpointInfo::patchpoint_invalid) { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "JIT_PatchpointOptimizationPolicy: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "PatchpointOptimizationPolicy: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); goto DONE; @@ -2401,7 +2401,7 @@ PCODE JIT_PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* c if ((ppId < lowId) || (ppId > highId)) { - LOG((LF_TIEREDCOMPILATION, LL_INFO10, "JIT_PatchpointOptimizationPolicy: ignoring patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", + LOG((LF_TIEREDCOMPILATION, LL_INFO10, "PatchpointOptimizationPolicy: ignoring patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); goto DONE; } @@ -2438,7 +2438,7 @@ PCODE JIT_PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* c const int hitCount = InterlockedIncrement(&ppInfo->m_patchpointCount); const int hitLogLevel = (hitCount == 1) ? LL_INFO10 : LL_INFO1000; - LOG((LF_TIEREDCOMPILATION, hitLogLevel, "JIT_PatchpointOptimizationPolicy: patchpoint [%d] (0x%p) hit %d in Method=0x%pM (%s::%s) [il offset %d] (limit %d)\n", + LOG((LF_TIEREDCOMPILATION, hitLogLevel, "PatchpointOptimizationPolicy: patchpoint [%d] (0x%p) hit %d in Method=0x%pM (%s::%s) [il offset %d] (limit %d)\n", ppId, ip, hitCount, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset, hitLimit)); // Defer, if we haven't yet reached the limit @@ -2451,7 +2451,7 @@ PCODE JIT_PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* c LONG oldFlags = ppInfo->m_flags; if ((oldFlags & PerPatchpointInfo::patchpoint_triggered) == PerPatchpointInfo::patchpoint_triggered) { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "JIT_PatchpointOptimizationPolicy: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "PatchpointOptimizationPolicy: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); goto DONE; } @@ -2460,7 +2460,7 @@ PCODE JIT_PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* c if (!triggerTransition) { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "JIT_PatchpointOptimizationPolicy: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "PatchpointOptimizationPolicy: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); goto DONE; } @@ -2497,7 +2497,7 @@ PCODE JIT_PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* c // // We want to expose bugs in the jitted code // for OSR methods, so we stick with synchronous creation. - LOG((LF_TIEREDCOMPILATION, LL_INFO10, "JIT_PatchpointOptimizationPolicy: patchpoint [%d] (0x%p) TRIGGER at count %d\n", ppId, ip, hitCount)); + LOG((LF_TIEREDCOMPILATION, LL_INFO10, "PatchpointOptimizationPolicy: patchpoint [%d] (0x%p) TRIGGER at count %d\n", ppId, ip, hitCount)); // Invoke the helper to build the OSR method osrMethodCode = JitPatchpointWorker(pMD, codeInfo, ilOffset); @@ -2506,7 +2506,7 @@ PCODE JIT_PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* c if (osrMethodCode == (PCODE)NULL) { // Unexpected, but not fatal - STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "JIT_PatchpointOptimizationPolicy: patchpoint (0x%p) OSR method creation failed," + STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "PatchpointOptimizationPolicy: patchpoint (0x%p) OSR method creation failed," " marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, pMD, ilOffset); InterlockedOr(&ppInfo->m_flags, (LONG)PerPatchpointInfo::patchpoint_invalid); @@ -2529,7 +2529,7 @@ PCODE JIT_PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* c return (PCODE)NULL; } -PCODE JIT_PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* counter, int ilOffset, PerPatchpointInfo * ppInfo, const EECodeInfo& codeInfo, bool *pIsNewMethod) +PCODE PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* counter, int ilOffset, PerPatchpointInfo * ppInfo, const EECodeInfo& codeInfo, bool *pIsNewMethod) { STATIC_CONTRACT_NOTHROW; STATIC_CONTRACT_GC_TRIGGERS; @@ -2545,7 +2545,7 @@ PCODE JIT_PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* count if ((ppInfo->m_flags & PerPatchpointInfo::patchpoint_invalid) == PerPatchpointInfo::patchpoint_invalid) { - LOG((LF_TIEREDCOMPILATION, LL_FATALERROR, "JIT_PatchpointRequiredPolicy: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", + LOG((LF_TIEREDCOMPILATION, LL_FATALERROR, "PatchpointRequiredPolicy: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); } @@ -2574,7 +2574,7 @@ PCODE JIT_PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* count // if ((ppInfo->m_flags & PerPatchpointInfo::patchpoint_invalid) == PerPatchpointInfo::patchpoint_invalid) { - LOG((LF_TIEREDCOMPILATION, LL_FATALERROR, "JIT_PatchpointRequiredPolicy: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", + LOG((LF_TIEREDCOMPILATION, LL_FATALERROR, "PatchpointRequiredPolicy: invalid patchpoint [%d] (0x%p) in Method=0x%pM (%s::%s) at offset %d\n", ppId, ip, pMD, pMD->m_pszDebugClassName, pMD->m_pszDebugMethodName, ilOffset)); EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); } @@ -2584,7 +2584,7 @@ PCODE JIT_PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* count LONG oldFlags = ppInfo->m_flags; if ((oldFlags & PerPatchpointInfo::patchpoint_triggered) == PerPatchpointInfo::patchpoint_triggered) { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "JIT_PatchpointRequiredPolicy: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "PatchpointRequiredPolicy: AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); __SwitchToThread(0, backoffs++); continue; } @@ -2596,7 +2596,7 @@ PCODE JIT_PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* count if (!triggerTransition) { - LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "JIT_PatchpointRequiredPolicy: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); + LOG((LF_TIEREDCOMPILATION, LL_INFO1000, "PatchpointRequiredPolicy: (lost race) AWAITING OSR method for patchpoint [%d] (0x%p)\n", ppId, ip)); __SwitchToThread(0, backoffs++); continue; } @@ -2608,7 +2608,7 @@ PCODE JIT_PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* count // // (but consider: throw path in method with try/catch, OSR method will contain more than just the throw?) // - LOG((LF_TIEREDCOMPILATION, LL_INFO10, "JIT_PatchpointRequiredPolicy: patchpoint [%d] (0x%p) TRIGGER\n", ppId, ip)); + LOG((LF_TIEREDCOMPILATION, LL_INFO10, "PatchpointRequiredPolicy: patchpoint [%d] (0x%p) TRIGGER\n", ppId, ip)); PCODE newMethodCode = JitPatchpointWorker(pMD, codeInfo, ilOffset); // If that failed, mark the patchpoint as invalid. @@ -2616,7 +2616,7 @@ PCODE JIT_PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* count // if (newMethodCode == (PCODE)NULL) { - STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "JIT_PatchpointRequiredPolicy: patchpoint (0x%p) OSR method creation failed," + STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "PatchpointRequiredPolicy: patchpoint (0x%p) OSR method creation failed," " marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, pMD, ilOffset); InterlockedOr(&ppInfo->m_flags, (LONG)PerPatchpointInfo::patchpoint_invalid); EEPOLICY_HANDLE_FATAL_ERROR(COR_E_EXECUTIONENGINE); @@ -2652,7 +2652,7 @@ PCODE JIT_PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* count // is NULL, always create an OSR method and transition to it. // // Currently, counter(the first argument) is a pointer into the Tier0 method stack -// frame so we have exclusive access. +// frame if it exists so we have exclusive access. extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransitionBlock) { @@ -2691,11 +2691,11 @@ extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransiti if (patchpointMustFindOptimizedCode) { - osrMethodCode = JIT_PatchpointRequiredPolicy(pTransitionBlock, counter, ilOffset, ppInfo, codeInfo, &isNewMethod); + osrMethodCode = PatchpointRequiredPolicy(pTransitionBlock, counter, ilOffset, ppInfo, codeInfo, &isNewMethod); } else { - osrMethodCode = JIT_PatchpointOptimizationPolicy(pTransitionBlock, counter, ilOffset, ppInfo, codeInfo, &isNewMethod); + osrMethodCode = PatchpointOptimizationPolicy(pTransitionBlock, counter, ilOffset, ppInfo, codeInfo, &isNewMethod); } if (osrMethodCode == (PCODE)NULL) @@ -2837,7 +2837,7 @@ extern "C" void JIT_PatchpointWorkerWorkerWithPolicy(TransitionBlock * pTransiti #else -void JIT_Patchpoint(int* counter, int ilOffset) +HCIMPL2(void, JIT_Patchpoint, int* counter, int ilOffset) { // Stub version if OSR feature is disabled // @@ -2845,6 +2845,7 @@ void JIT_Patchpoint(int* counter, int ilOffset) UNREACHABLE(); } +HCIMPLEND HCIMPL1(VOID, JIT_PartialCompilationPatchpoint, int ilOffset) { diff --git a/src/coreclr/vm/jitinterface.h b/src/coreclr/vm/jitinterface.h index 47bd80c0ac5e6a..7f1835e458a53a 100644 --- a/src/coreclr/vm/jitinterface.h +++ b/src/coreclr/vm/jitinterface.h @@ -103,8 +103,8 @@ BOOL LoadDynamicInfoEntry(Module *currentModule, #endif // TARGET_X86 -// These must be implemented by the JIT in order to support the "jithelpers" functionality. -EXTERN_C void JIT_Patchpoint(int* counter, int ilOffset); +// These must be implemented in assembly and generate a TransitionBlock then calling JIT_PatchpointWorkerWithPolicy in order to actually be used. +EXTERN_C FCDECL2(void, JIT_Patchpoint, int* counter, int ilOffset); EXTERN_C FCDECL1(void, JIT_PartialCompilationPatchpoint, int ilOffset); // From 4f7e68c6dcd70dc262080273b5fa4ae8bc26f441 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 4 Feb 2025 17:06:25 -0800 Subject: [PATCH 08/10] Apply suggestions from code review Co-authored-by: Andy Ayers --- src/coreclr/vm/jithelpers.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 0a98a480c32df1..9a200987c49ee5 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2341,7 +2341,7 @@ static PCODE JitPatchpointWorker(MethodDesc* pMD, const EECodeInfo& codeInfo, in return osrVariant; } -PCODE PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* counter, int ilOffset, PerPatchpointInfo * ppInfo, const EECodeInfo& codeInfo, bool *pIsNewMethod) +static PCODE PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* counter, int ilOffset, PerPatchpointInfo * ppInfo, const EECodeInfo& codeInfo, bool *pIsNewMethod) { STATIC_CONTRACT_NOTHROW; STATIC_CONTRACT_GC_TRIGGERS; @@ -2505,7 +2505,7 @@ PCODE PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int* count // If that failed, mark the patchpoint as invalid. if (osrMethodCode == (PCODE)NULL) { - // Unexpected, but not fatal + // Unexpected, but not fatal, unless forced to transition STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "PatchpointOptimizationPolicy: patchpoint (0x%p) OSR method creation failed," " marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, pMD, ilOffset); From 36a8db0b0c8d4dd3c39e339af65e45a44bf5cd2a Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 4 Feb 2025 17:09:08 -0800 Subject: [PATCH 09/10] Update src/coreclr/vm/jithelpers.cpp Merged wrong suggestion earlier --- src/coreclr/vm/jithelpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 9a200987c49ee5..68cfacea3d73b3 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2505,7 +2505,7 @@ static PCODE PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int // If that failed, mark the patchpoint as invalid. if (osrMethodCode == (PCODE)NULL) { - // Unexpected, but not fatal, unless forced to transition + // Unexpected, but not fatal STRESS_LOG3(LF_TIEREDCOMPILATION, LL_WARNING, "PatchpointOptimizationPolicy: patchpoint (0x%p) OSR method creation failed," " marking patchpoint invalid for Method=0x%pM il offset %d\n", ip, pMD, ilOffset); From 034df2794493a5fa7528e216e7db661964047b2f Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Tue, 4 Feb 2025 17:12:23 -0800 Subject: [PATCH 10/10] Update src/coreclr/vm/jithelpers.cpp --- src/coreclr/vm/jithelpers.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/vm/jithelpers.cpp b/src/coreclr/vm/jithelpers.cpp index 68cfacea3d73b3..81d3481124ef1a 100644 --- a/src/coreclr/vm/jithelpers.cpp +++ b/src/coreclr/vm/jithelpers.cpp @@ -2529,7 +2529,7 @@ static PCODE PatchpointOptimizationPolicy(TransitionBlock* pTransitionBlock, int return (PCODE)NULL; } -PCODE PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* counter, int ilOffset, PerPatchpointInfo * ppInfo, const EECodeInfo& codeInfo, bool *pIsNewMethod) +static PCODE PatchpointRequiredPolicy(TransitionBlock* pTransitionBlock, int* counter, int ilOffset, PerPatchpointInfo * ppInfo, const EECodeInfo& codeInfo, bool *pIsNewMethod) { STATIC_CONTRACT_NOTHROW; STATIC_CONTRACT_GC_TRIGGERS;