JIT: Restore arm64, LA64 and RISCV64 OSR callee saves from tier0 frame#126880
JIT: Restore arm64, LA64 and RISCV64 OSR callee saves from tier0 frame#126880jakobbotsch wants to merge 31 commits intodotnet:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates CoreCLR JIT OSR support on ARM64 so OSR methods can correctly “inherit” callee-saved registers from the Tier0 frame and restore them on exit, instead of saving/restoring the already-mutated register state from Tier0.
Changes:
- Record and consume Tier0 callee-save information for ARM64 OSR, and update ARM64 prolog/epilog code to avoid re-saving inherited callee-saves and to restore them from the Tier0 frame at return.
- Extend patchpoint/callee-save bookkeeping (including SuperPMI replay handling for older collections).
- Update
genPushCalleeSavedRegisterssignature across targets and wire OSR prolog handling for AMD64/ARM64.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/coreclr/vm/jitinterface.cpp | Adds debug-only AltJit patchpoint info tracking so AltJit OSR compilations can retrieve the right patchpoint info. |
| src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp | Adjusts replayed OSR patchpoint info on ARM64 to include FP/LR in older collections. |
| src/coreclr/jit/lclvars.cpp | Ensures ARM64 Tier0 patchpoint methods save FP/LR with all callee-saves (needed for OSR interactions). |
| src/coreclr/jit/compiler.cpp | Updates patchpoint info generation for callee-save recording (incl. ARM64 LR) and updates debug dumping. |
| src/coreclr/jit/codegenxarch.cpp | Updates genPushCalleeSavedRegisters signature to match common prolog flow. |
| src/coreclr/jit/codegenwasm.cpp | Updates genPushCalleeSavedRegisters signature (no-op target). |
| src/coreclr/jit/codegencommon.cpp | Adds ARM64 OSR handling to the “inherit tier0” prolog path and avoids double-saving inherited callee-saves. |
| src/coreclr/jit/codegenarmarch.cpp | ARM64 prolog logic now skips saving FP/LR (and other inherited callee-saves) when OSR inherits them from Tier0. |
| src/coreclr/jit/codegenarm64.cpp | ARM64 epilog now restores inherited callee-saves from the Tier0 frame and then pops the Tier0 frame. |
| src/coreclr/jit/codegen.h | Updates CodeGen APIs to support unwind-only prolog save helpers and unified genPushCalleeSavedRegisters signature. |
|
/azp run runtime-jit-experimental |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-jit-experimental |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| newInfo->Next = s_altJitPatchpointInfoList; | ||
| newInfo->Method = m_pMethodBeingCompiled; | ||
| newInfo->Info = newPpi; | ||
| s_altJitPatchpointInfoList = newInfo; |
There was a problem hiding this comment.
@jkotas do we have something similar to the method table auxiliary data for MethodDesc where I could store this in debug builds? Or perhaps I should just use a hash table with a mutex.
There was a problem hiding this comment.
We have MethodDescCodeData that should work for what you trying to do here.
| for (AltJitPatchpointInfo* altJitPpi = s_altJitPatchpointInfoList; altJitPpi != NULL; altJitPpi = altJitPpi->Next) | ||
| { | ||
| if (altJitPpi->Method == m_pMethodBeingCompiled) | ||
| { | ||
| result = altJitPpi->Info; | ||
| break; | ||
| } |
There was a problem hiding this comment.
This ALT_JIT debug-only cache grows monotonically and is never cleaned up. Beyond the memory leak, keeping MethodDesc* keys for collectible methods past unload can lead to false matches if addresses get reused, returning the wrong PatchpointInfo. Consider scoping the cache to a reclaimable lifetime (e.g., tied to a LoaderAllocator cleanup) or using a non-colliding key.
| for (AltJitPatchpointInfo* altJitPpi = s_altJitPatchpointInfoList; altJitPpi != NULL; altJitPpi = altJitPpi->Next) | |
| { | |
| if (altJitPpi->Method == m_pMethodBeingCompiled) | |
| { | |
| result = altJitPpi->Info; | |
| break; | |
| } | |
| AltJitPatchpointInfo* previousAltJitPpi = NULL; | |
| for (AltJitPatchpointInfo* altJitPpi = s_altJitPatchpointInfoList; altJitPpi != NULL; altJitPpi = altJitPpi->Next) | |
| { | |
| if (altJitPpi->Method == m_pMethodBeingCompiled) | |
| { | |
| uint32_t ppiSize = altJitPpi->Info->PatchpointInfoSize(); | |
| PatchpointInfo* copiedPpi = new (new uint8_t[ppiSize]) PatchpointInfo; | |
| copiedPpi->Initialize(altJitPpi->Info->NumberOfLocals(), altJitPpi->Info->TotalFrameSize()); | |
| copiedPpi->Copy(altJitPpi->Info); | |
| if (previousAltJitPpi == NULL) | |
| { | |
| s_altJitPatchpointInfoList = altJitPpi->Next; | |
| } | |
| else | |
| { | |
| previousAltJitPpi->Next = altJitPpi->Next; | |
| } | |
| delete[] reinterpret_cast<uint8_t*>(altJitPpi->Info); | |
| delete altJitPpi; | |
| result = copiedPpi; | |
| break; | |
| } | |
| previousAltJitPpi = altJitPpi; |
|
cc @dotnet/jit-contrib PTAL @AndyAyersMS. Left a few comments above that I should address, but I think this is ready for review. Diffs. Larger OSR method prologs. It will be understated a little bit since the collections do not have any callee saves in the patchpoint information, but using callee saves in tier0 functions is rare, so probably it isn't understated by that much. After some thinking I was able to remove the tier0 frame type regression – we now put FP/LR into the callee save registers in the patchpoint info only if it was saved with the rest of the callee saves, and then we use that in OSR to determine the kind of frame. In the end we don't need to care about all 5 types of frames, only where fp/lr ends up being saved which turns out not to be too bad. |
AndyAyersMS
left a comment
There was a problem hiding this comment.
Once we're done with this cycle of OSR changes we should reconcile the various bits of documentation like OSR Details and Debuggging: OSR Prolog and OSR x64 Epilog Redesign.
And make a pass through #33658
Change OSR functions for arm64, loongarch64 and riscv64 to start their prolog out by restoring the values of all callee saves that were saved by the tier0 function. This should make it possible to transition from tier0 to OSR without requiring any unwinding – instead it can be done with a simple jump. The end goal is to fix #120865 which is showing up in ASP.NET benchmarks.
Also support saving patchpoint info produced by an altjit in the VM.