Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cranelift: Remove resumable traps #8809

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Jun 14, 2024

These were originally a SpiderMonkey-ism and have been unused ever since. It was introduced for GC integration, where the runtime could do something to make Cranelift code hit a trap and pause for a GC and then resume execution once GC completed. But it is unclear that, as implemented, this is actually a useful mechanism for doing that (compared to, say, loading from some Well Known page and the GC protecting that page and catching signals to interrupt the mutator, or simply branching and doing a libcall). And if someone has that particular use case in the future (Wasmtime and its GC integration doesn't need exactly this) then we can design something for what is actually needed at that time, instead of carrying this cruft forward forever.

These were originally a SpiderMonkey-ism and have been unused ever
since. It was introduced for GC integration, where the runtime could do
something to make Cranelift code hit a trap and pause for a GC and then resume
execution once GC completed. But it is unclear that, as implemented, this is
actually a useful mechanism for doing that (compared to, say, loading from some
Well Known page and the GC protecting that page and catching signals to
interrupt the mutator, or simply branching and doing a libcall). And if someone
has that particular use case in the future (Wasmtime and its GC integration
doesn't need exactly this) then we can design something for what is actually
needed at that time, instead of carrying this cruft forward forever.
@fitzgen fitzgen requested review from a team as code owners June 14, 2024 19:13
@fitzgen fitzgen requested review from alexcrichton and abrown and removed request for a team June 14, 2024 19:13
@fitzgen fitzgen enabled auto-merge June 14, 2024 19:25
@fitzgen fitzgen added this pull request to the merge queue Jun 14, 2024
Merged via the queue into bytecodealliance:main with commit 9ffc9e6 Jun 14, 2024
66 checks passed
@fitzgen fitzgen deleted the no-more-resumable-traps branch June 14, 2024 19:39
fitzgen added a commit to fitzgen/wasmtime that referenced this pull request Jun 14, 2024
After bytecodealliance#8809, the mutator cannot
resume from a trap so we don't need to consider them safepoints, as no
GC-managed references are live after the trap. The one exception being the
`debugtrap` CLIF instruction, which is technically still a resumable trap, but
which exists only for emitting the equivalent of an `int3` breakpoint
instruction for pausing in a debugger to inspect state, and should never be used
for mutator-collector interactions.
github-merge-queue bot pushed a commit that referenced this pull request Jun 14, 2024
After #8809, the mutator cannot
resume from a trap so we don't need to consider them safepoints, as no
GC-managed references are live after the trap. The one exception being the
`debugtrap` CLIF instruction, which is technically still a resumable trap, but
which exists only for emitting the equivalent of an `int3` breakpoint
instruction for pausing in a debugger to inspect state, and should never be used
for mutator-collector interactions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants