[Wasm RyuJit] emit virtual IP ranges in the unwind info#128382
Conversation
Extend the per-funclet unwind info to record the starting Virtual IP for the funclet and the length of the virtual IP range. The unwind info previously just recorded the size of the fixed part of the frame. Virtual IP ranges for funclets (and main method) are disjoint. All data is encoded as ULEB128.
|
@davidwrighton PTAL |
|
Sample output (from jit dump), for a method with a simple try/finally: Also note you currently must pass |
There was a problem hiding this comment.
Pull request overview
This PR updates the Wasm RyuJIT unwind-info payload to include a per-method/per-funclet Virtual IP (VIP) range alongside the existing frame-size data, and records those VIP ranges during the Wasm virtual-IP phase.
Changes:
- Extend
FuncInfoDsc(Wasm-only) withstartVirtualIP/endVirtualIPfields. - Record
startVirtualIP/endVirtualIPfor each function/funclet duringfgWasmVirtualIP(). - Emit unwind info as ULEB128-encoded
{ frameSize, startVirtualIP, (endVirtualIP - startVirtualIP) }inunwindEmitFunc().
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/coreclr/jit/unwindwasm.cpp | Emits Wasm unwind payload including VIP range (start + delta) encoded as ULEB128. |
| src/coreclr/jit/fgwasm.cpp | Captures per-funclet VIP range boundaries while assigning VIPs. |
| src/coreclr/jit/compiler.h | Stores per-funclet VIP range endpoints in FuncInfoDsc under TARGET_WASM. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
src/coreclr/jit/codegenwasm.cpp:3472
- This changes the GC header's code length to
maxVirtualIP, but the subsequentgcMakeRegPtrTablecalls still passcodeSize/prologSize(native code byte offsets) to define interruptible ranges and call sites. Mixing VIP and byte-offset coordinate systems risks producing invalid GC info (e.g., offsets beyond the reported code length). Make the GC header and all reported offsets use the same units.
unsigned callCnt = 0;
// First we figure out the encoder ID's for the stack slots and registers.
gcInfo.gcMakeRegPtrTable(gcInfoEncoder, codeSize, prologSize, GCInfo::MAKE_REG_PTR_MODE_ASSIGN_SLOTS, &callCnt);
kg
left a comment
There was a problem hiding this comment.
LGTM aside from copilot and davidw's concerns
|
Looks like there is some other length field in GC info that needs fixing. |
This is going to be more complicated than I thought. We need to track the Virtual IPs per instruction group so we can properly describe no-GC regions, and we need to ensure we have appropriate Virtual IP updates for these regions. Or else we need to ensure we don't create any no-GC regions. Or else convince ourselves that these regions cannot span calls and so we can just ignore them for GC reporting purposes on Wasm, at least for now, since the only viable GC safepoints are at calls. |
I think this is valid simplifying assumption to make for wasm. We should be able to omit reporting fully interruptible GC information (including no-GC regions) on any platform that requires explicit GC polls and does not support suspension via execution "redirection". |
|
I started down the path of disabling fully interruptible GC for Wasm, but there are some missing parts that will require more work. Perhaps we can defer it a bit to unblock consumption of the data we're preoducing here
|
|
Locally I hit an assert in debug SPC which I thought we had fixed: working around this (and any subsequent NYIs) via SPC finishes crossgen cleanly. |
We may be able to get by without explicit gc polls for single threaded wasm, for MVP at least. The explicit gc polls should not be require for single-threaded runtime to work. The gc can be only triggered from the one thread and there are no other threads to suspend for the gc. The explicit gc polls should be only required for scenarios like managed debugger attach to a process that is stuck inside a long running loop in AOT compiled code. I think it is P2 - I believe that it does not work with Mono either. |
I'll look into this bitcast issue. |
adamperlin
left a comment
There was a problem hiding this comment.
This LGTM from what I understand. I don't have enough context to comment on the GC-related discussion here.
|
edit: wrong tab |
|
Going to wait for @davidwrighton to confirm this actually works before merging. |
davidwrighton
left a comment
There was a problem hiding this comment.
I won't be able to actually test this work before next Wednesday, but it now looks good to me (modulo the multiply by two issue on gc code size). I think it's in good enough state to merge.
|
Tagging subscribers to 'arch-wasm': @lewing, @pavelsavara |
Extend the per-funclet unwind info to record the length of the virtual IP range. The lowest Virtual IP for a funclet can be found by summing the lengths of all the prior funclets.
The unwind info previously just recorded the size of the fixed part of the frame.
Virtual IP ranges for funclets (and main method) are disjoint.
Data is encoded as ULEB128.