Skip to content

Switch InterpreterFrame to deterministic unwinding#125182

Closed
am11 wants to merge 6 commits into
dotnet:mainfrom
am11:feature/deterministic-unwinding4
Closed

Switch InterpreterFrame to deterministic unwinding#125182
am11 wants to merge 6 commits into
dotnet:mainfrom
am11:feature/deterministic-unwinding4

Conversation

@am11
Copy link
Copy Markdown
Member

@am11 am11 commented Mar 4, 2026

No description provided.

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Mar 4, 2026
@dotnet-policy-service
Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @BrzVlad, @janvorli, @kg
See info in area-owners.md if you want to be subscribed.

@janvorli
Copy link
Copy Markdown
Member

janvorli commented Mar 4, 2026

I don't think this is going to work. We need to know the callee saved registers value in the InterpExecMethod function. The ones we capture in the IntepreterFrame are not necessarily related. Both the ExecuteInterpretedMethod and InterpExecMethod can modify these registers.

@am11 am11 force-pushed the feature/deterministic-unwinding4 branch from 7903973 to 508674c Compare March 4, 2026 18:48
@am11
Copy link
Copy Markdown
Member Author

am11 commented Mar 4, 2026

@janvorli, good point. The TransitionBlock values are from InterpreterStub and can be stale by the time we're in InterpExecMethod. I've reworked this to call ClrCaptureContext from within InterpExecMethod's try block instead, so we get the correct callee-saved register values and an IP that falls within the try range.

Comment thread src/coreclr/vm/interpexec.cpp Outdated
@am11 am11 force-pushed the feature/deterministic-unwinding4 branch from fde94ba to 9f2788f Compare March 5, 2026 06:27
Comment thread src/coreclr/vm/eetwain.cpp
@am11 am11 force-pushed the feature/deterministic-unwinding4 branch from 8b0852b to 64305cd Compare March 5, 2026 19:05
@am11 am11 marked this pull request as ready for review March 7, 2026 09:56
@am11 am11 requested a review from kg as a code owner March 7, 2026 09:56
@am11 am11 requested a review from janvorli March 7, 2026 09:56
@janvorli
Copy link
Copy Markdown
Member

janvorli commented Apr 1, 2026

@am11, I still feel like capturing the non-volatile registers on the non-exceptional path is a perf hit that we would not like to take. Especially since it is done on every instruction interpreted where every added instruction counts.
When I've measured the impact of adding capturing just the nonvolatile floating point registers in the InterpreterStub / CallInterpreterFunclet, it was about 5% for the case of calling an interpreted method that contained just "ret". But for that change, this gets amortized the more the longer the callee is. With this change, we would add larger hit due to capturing also general purpose non-volatile registers and it would not get amortized.

As an alternative, I've been recently experimenting with the WASM-like resume after catch, it is much more tricky than I've thought it would be due to the need of adding personality routine to more helpers than just the InterpreterStub, having to manually unwind the frame in that personality routine (as it gets the context in the function the personality routine is attached to and we need the context of the caller so that we can rethrow the exception from there) and there might be more. I didn't have a chance to progress on that experiment recently, but I'd like to finish that.

I still feel like keeping using OS libunwind for Apple OSes wouldn't hurt, as it is always present and it doesn't add any burden over there. And iOS is actually the scenario that needs interpreter and that we care about the most (besides WASM).

@am11
Copy link
Copy Markdown
Member Author

am11 commented Apr 2, 2026

@janvorli, your alternative approach sounds like a worthwhile direction. I haven’t done performance testing myself yet to understand the impact.

Although we will ship only for iOS at first, we already build this code on all desktop platforms. i.e. we can’t discount it from “last six usages of in-tree libunwind” equation due to linux, FreeBSD and others.

@JulieLeeMSFT
Copy link
Copy Markdown
Member

@janvorli, this is awaiting your review.

@janvorli
Copy link
Copy Markdown
Member

@janvorli, this is awaiting your review.

@JulieLeeMSFT in the comments above, we have agreed to do it differently.

@jkotas jkotas marked this pull request as draft April 20, 2026 15:08
@JulieLeeMSFT JulieLeeMSFT added the needs-author-action An issue or pull request that requires more info or actions from the author. label Apr 20, 2026
@am11
Copy link
Copy Markdown
Member Author

am11 commented May 28, 2026

Superseded by #128728.

@am11 am11 closed this May 28, 2026
@am11 am11 deleted the feature/deterministic-unwinding4 branch May 28, 2026 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-CodeGen-Interpreter-coreclr community-contribution Indicates that the PR has been added by a community member needs-author-action An issue or pull request that requires more info or actions from the author.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants