Skip to content

winch: support tail calls with callee cleanup - #14349

Open
macovedj wants to merge 4 commits into
bytecodealliance:mainfrom
macovedj:winch-tailcall-callee-clean
Open

macovedj wants to merge 4 commits into
bytecodealliance:mainfrom
macovedj:winch-tailcall-callee-clean

Conversation

@macovedj

Copy link
Copy Markdown
Contributor

Adds support for return_call and return_call_indirect to Winch on x86-64 and AArch64.
The implementation changes Winch’s internal calling convention so callees reclaim their aligned incoming stack-argument area before returning. This applies to ordinary calls as well as tail calls. Tail calls can then replace the current frame and resize the argument area without requiring the original caller to recover SP afterward. Cranelift-generated trampolines use the existing callee_pop_size machinery to follow the same convention.
Includes two performance optimizations:

  • Reclaim alignment padding and consumed operand spills together when no stack-return area needs to be preserved.
  • On x86-64, relocate the return address and perform explicit cleanup before a plain ret, using a compact SP-relative sequence for small frames and an FP-relative fallback for larger frames.

Implement direct and indirect tail calls on x86-64 and AArch64 using the Winch callee-pop convention. Include semantic, trampoline, trap, stack-result, ABI, and disassembly coverage with the implementation.
Reclaim alignment padding and consumed operand spills together when no stack return area must be preserved. Cover live operands, consumed results, and direct, imported, and indirect calls on both supported architectures.
Use a short SP-relative return sequence for compact stack-argument frames and retain the FP-relative fallback for larger frames. Include runtime, disassembly, and exact imm8-boundary encoding tests.
@macovedj
macovedj requested review from a team as code owners September 16, 2026 19:43
@macovedj
macovedj requested review from cfallin and removed request for a team September 16, 2026 19:43
@github-actions github-actions Bot added cranelift Issues related to the Cranelift code generator cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:x64 Issues related to x64 codegen wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:config Issues related to the configuration of Wasmtime labels Sep 16, 2026
@github-actions

Copy link
Copy Markdown

Label Messager: wasmtime:config

It looks like you are changing Wasmtime's configuration options. Make sure to
complete this check list:

  • If you added a new Config method, you wrote extensive documentation for
    it.

    Details

    Our documentation should be of the following form:

    Short, simple summary sentence.
    
    More details. These details can be multiple paragraphs. There should be
    information about not just the method, but its parameters and results as
    well.
    
    Is this method fallible? If so, when can it return an error?
    
    Can this method panic? If so, when does it panic?
    
    # Example
    
    Optional example here.
    
  • If you added a new Config method, or modified an existing one, you
    ensured that this configuration is exercised by the fuzz targets.

    Details

    For example, if you expose a new strategy for allocating the next instance
    slot inside the pooling allocator, you should ensure that at least one of our
    fuzz targets exercises that new strategy.

    Often, all that is required of you is to ensure that there is a knob for this
    configuration option in wasmtime_fuzzing::Config (or one
    of its nested structs).

    Rarely, this may require authoring a new fuzz target to specifically test this
    configuration. See our docs on fuzzing for more details.

  • If you are enabling a configuration option by default, make sure that it
    has been fuzzed for at least two weeks before turning it on by default.


Details

To modify this label's message, edit the .github/label-messager/wasmtime-config.md file.

To add new label messages or remove existing label messages, edit the
.github/label-messager.json configuration file.

Learn more.

@cfallin

cfallin commented Sep 16, 2026

Copy link
Copy Markdown
Member

Thanks for this -- I will be able to review next week (currently on a work trip with limited availability).

@saulecabrera saulecabrera left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a first pass; thanks for all the changes here. FWIW, @macovedj and myself discussed some of the trade-offs here between a caller and callee pop approach.


The following programs fail to compile:

(module
  (type $t (func (result i32)))
  (table 1 funcref)
  (func (export "f") (param i32) (result i32)
    (if (local.get 0) (then (return_call_indirect (type $t) (i32.const 0))))
    (if (local.get 0) (then (return_call_indirect (type $t) (i32.const 0))))
    (if (local.get 0) (then (return_call_indirect (type $t) (i32.const 0))))
    (if (local.get 0) (then (return_call_indirect (type $t) (i32.const 0))))
    (if (local.get 0) (then (return_call_indirect (type $t) (i32.const 0))))
    (i32.const 0)
  )
)

When invoked via

target/release/wasmtime -C compiler=winch -W tail-call <wasm>

It fails with

Winch internal error: Expected register to be available.

I think we are failing to free the register in emit_return?

(module
  (func $f
    (return_call $f)
    (loop)
  )
)

When invoked using epoch or using fuel interruption, like:

  • wasmtime compile -C compiler=winch -W tail-call,fuel=1000 <wasm>
  • wasmtime compile -C compiler=winch -W tail-call,epoch-interruption=y <wasm>

It fails with:

Winch internal error: Invalid local offset

Is it possible that we are not handling SP correctly at emit_return?

Aside from the issues above, I think we'd also want to:

  • Enable tail calls in the fuzzer configuration
  • Update the doc comment in Config::wasm_tail_call

@macovedj
macovedj requested a review from a team as a code owner September 18, 2026 15:40
@macovedj
macovedj requested review from fitzgen and removed request for a team September 18, 2026 15:40
@macovedj

macovedj commented Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @saulecabrera!
I’ve pushed an update addressing the feedback. It updates the documentation and fuzz configuration, releases the temporary callee registers after tail-call lowering, and skips fuel/epoch checks for unreachable loops while preserving the control-stack bookkeeping. I also added regressions for the register cleanup and unreachable-loop cases.

@github-actions github-actions Bot added the fuzzing Issues related to our fuzzing infrastructure label Sep 18, 2026
@github-actions

Copy link
Copy Markdown

Subscribe to Label Action

cc @fitzgen

Details This issue or pull request has been labeled: "cranelift", "cranelift:area:aarch64", "cranelift:area:machinst", "cranelift:area:x64", "fuzzing", "wasmtime:api", "wasmtime:config"

Thus the following users have been cc'd because of the following labels:

  • fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift:area:aarch64 Issues related to AArch64 backend. cranelift:area:machinst Issues related to instruction selection and the new MachInst backend. cranelift:area:x64 Issues related to x64 codegen cranelift Issues related to the Cranelift code generator fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:config Issues related to the configuration of Wasmtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants