Wasmtime GitHub notifications bot (Apr 02 2025 at 20:47): Wasmtime GitHub notifications bot (Apr 02 2025 at 20:47):Mrmaxmeier opened PR #10515 from Mrmaxmeier:cranelift-trapcode-extend-u16 to bytecodealliance:main:
This allows users of Cranelift to have ~65k unique trap codes again.
The range ofTrapCodewas reduced in #9338 in order to be able to fit arbitrary trap codes inside ofMemFlags.I'm keeping MemFlags a u16 with this change and chose to only allow a _low subset_ of trap codes to be represented instead. Explicit traps can still use the full TrapCode range.
The implementation is a bit awkward but I've added exhaustive tests for the encoding part. Feel free to close this PR if the cons (panic in
MemFlags::with_trap_code) outweigh the pros here (more flexibility with dynamically-assigned user traps). :slightly_smiling_face:Thanks!
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Wasmtime GitHub notifications bot (Apr 02 2025 at 20:47):Mrmaxmeier requested wasmtime-compiler-reviewers for a review on PR #10515.
Wasmtime GitHub notifications bot (Apr 02 2025 at 20:47):Mrmaxmeier requested cfallin for a review on PR #10515.
Wasmtime GitHub notifications bot (Apr 02 2025 at 20:47):Mrmaxmeier requested pchickey for a review on PR #10515.
Wasmtime GitHub notifications bot (Apr 02 2025 at 20:47):Mrmaxmeier requested wasmtime-core-reviewers for a review on PR #10515.
Wasmtime GitHub notifications bot (Apr 02 2025 at 20:54):cfallin commented on PR #10515:
Thanks for the PR!
Looking it over, it indeed does what you say, but it's not clear to me that it is a clean, usable, improvement to the API to have only a subset of trap codes usable in some places. E.g., we may want the ability to turn a
trapifinto a virtual memory-based trap (which would use a load withMemFlags); that transform/optimization would no longer be possible with this distinction.If the change were to make trap codes, say, 32 or 64 bits, I could understand the utility better, but u16 is still small enough that one has to worry about running out if one is allocating these codes dynamically. Could you clarify your use-case and requirements? Maybe there's another way to encode the information that you're encoding in the trap code?
Wasmtime GitHub notifications bot (Apr 02 2025 at 21:00):Mrmaxmeier updated PR #10515.
Wasmtime GitHub notifications bot (Apr 02 2025 at 21:09):Mrmaxmeier commented on PR #10515:
Ah yes, it doesn't seem worth it considering the possible optimization you mentioned.
In my specific use case I'm using Cranelift in a custom JIT that generates code with a fair amount of rare "error" cases. In my case the amount of unique errors roughly matches the amount of basic blocks in a source function.
I'm handling these through signals and Cranelift's ability to resolve the trapping instruction back to an IR-level trap code is quite useful here. Of course I could also call into a function that raises a passed code from the generated assembly, but I'm not sure if there's a way that is as _code_-space efficient.
Wasmtime GitHub notifications bot (Apr 02 2025 at 21:30):cfallin closed without merge PR #10515.
Wasmtime GitHub notifications bot (Apr 02 2025 at 21:30):cfallin commented on PR #10515:
OK, yeah, in that case I suspect that a call with an error code is a reasonable option; or if you structure the IR to have an unconditional jump to a shared basic block with the error callsite, with one block arg for a unique ID, you might get it down to ~12 bytes or so on x86-64 (a move-immediate into a register and a jump). Given the niche use-case and the. impact to clean/generic IR design I'm going to go ahead and close this PR; thanks again for the suggestion, though.
Last updated: Apr 17 2025 at 08:04 UTC