Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
winch: Overhaul the internal ABI (bytecodealliance#7974)
* winch: Overhaul the internal ABI This change overhauls Winch's ABI. This means that as part of this change, the default ABI now closely resembles Cranelift's ABI, particularly on the treatment of the VMContext. This change also fixes many wrong assumptions about trampolines, which are tied to how the previous ABI operated. The main motivation behind this change is: * To make it easier to integrate Winch-generated functions with Wasmtime * Fix fuzz bugs related to imports * Solidify the implementation regarding the usage of a pinned register to hold the VMContext value throughout the lifetime of a function. The previous implementation had the following characteristics, and wrong assumptions): * Assumed that nternal functions don't receive a caller or callee VMContexts as parameters. * Worked correctly in the following scenarios: * `Wasm -> Native`: since we can explicitly load the caller and callee `VMContext`, because we're calling a native import. * `(Native, Array) -> Wasm`: because the native signatures define a tuple of `VMContext` as arguments. * It didn't work in the following scenario: * `Wasm->Wasm`: When calling imports from another WebAssembly instance (via direct call or `call_indirect`. The previous implementation wrongly assumes that there should be a trampoline in this case, but there isn't. The code was generated by the same compiler, so the same ABI should be used in both functions, but it doesn't. This change introduces the following changes, which fix the previous assumptions and bugs: * All internal functions declare a two extra pointer-sized parameters, which will hold the callee and caller `VMContext`s * Use a pinned register that will be considered live through the lifetime of the function instead of pinning it at the trampoline level. The pinning explicitlly happens when entering the function body and no other assumptions are made from there on. * Introduce the concept of special `ContextArgs` for function calls. This enum holds metadata about which context arguments are needed depending on the callee. The previous implementation of introducing register values at arbitrary locations in the value stack conflicts with the stack ordering principle which states that older values must *always* precede newer values. So we can't insert a register, because if a spill happens the order of the values will be wrong. Finally, given that this change also enables the `imports.wast` test suite, it also includes a fix to `global.{get, set}` instructions which didn't account entirely for imported globals. Resolved conflicts Update Winch filetests * Fix typos * Use `get_wasm_local` and `get_frame_local` instead of `get_local` and `get_local_unchecked` * Introduce `MAX_CONTEXT_ARGS` and use it in the trampoline to skip context arguments.
- Loading branch information