Migrate libcalls to checking for traps #9710
Migrate libcalls to checking for traps #9710alexcrichton merged 1 commit intobytecodealliance:mainfrom
Conversation
| self.raise_if_host_trapped(); | ||
| // Under fuel limits branch. | ||
| self.masm.bind(continuation); | ||
| self.context.free_reg(fuel_var); |
There was a problem hiding this comment.
Similarly @saulecabrera I had to move this up since fuel_var was assigned rax but rax was needed as the return value of the hostcall, so this was needed to fix some panics during compilation.
Subscribe to Label Actioncc @fitzgen DetailsThis issue or pull request has been labeled: "wasmtime:api", "wasmtime:ref-types"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
065edd4 to
c10f692
Compare
|
Does this significantly change overall code size of Cranelift-generated code, due to needing inline trap check code at each libcall callsite? |
|
Yes as-is all libcalls that can trap have a check after them, so it's inflating code by at least that much. In thinking though this can be pretty easily optimized. We already generate a trampoline-per-libcall so I'll move the check-for-trap logic to the trampoline instead of the callsite of the trampoline. |
|
Ok I've updated this where the trampoline itself now checks for a trap, so the code size impact on wasm itself should be minimal. |
fitzgen
left a comment
There was a problem hiding this comment.
LGTM modulo one comment below
This commit is the equivalent of bytecodealliance#9675 for libcalls used in core wasm. All libcalls now communicate whether or not they trapped through their return value instead of implicitly calling `longjmp` to exit from the libcall. This is to make integration with Pulley easier to avoid the need to `longjmp` over Pulley execution. Libcall definitions have changed where appropriate and the `catch_unwind_and_record_trap` function introduced in bytecodealliance#9675 was refactored to better support multiple types of values being returned from libcalls (instead of just `Result<()>`). Note that changes have been made to both the Cranelift translation layer and the Winch translation layer for this as the ABI of various libcalls are all changing.
5e10e06 to
6470dfd
Compare
This PR is the equivalent of #9675 for libcalls used in core wasm and components.
All libcalls now communicate whether or not they trapped through their
return value instead of implicitly calling
longjmpto exit from thelibcall. This is to make integration with Pulley easier to avoid the
need to
longjmpover Pulley execution.Libcall definitions have changed where appropriate and the
catch_unwind_and_record_trapfunction introduced in #9675 wasrefactored to better support multiple types of values being returned
from libcalls (instead of just
Result<()>).Note that changes have been made to both the Cranelift translation layer
and the Winch translation layer for this as the ABI of various libcalls
are all changing.