wasmtime / PR #14349 winch: support tail calls with calle... · git-wasmtime

Stream: git-wasmtime

Topic: wasmtime / PR #14349 winch: support tail calls with calle...

Wasmtime GitHub notifications bot (Sep 16 2026 at 19:43):

macovedj opened PR #14349 from macovedj:winch-tailcall-callee-clean to bytecodealliance:main:

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.

Wasmtime GitHub notifications bot (Sep 16 2026 at 19:43):

macovedj requested cfallin for a review on PR #14349.

Wasmtime GitHub notifications bot (Sep 16 2026 at 19:43):

macovedj requested wasmtime-compiler-reviewers for a review on PR #14349.

Wasmtime GitHub notifications bot (Sep 16 2026 at 19:43):

macovedj requested wasmtime-core-reviewers for a review on PR #14349.

Wasmtime GitHub notifications bot (Sep 16 2026 at 20:45):

github-actions[bot] added the label cranelift on PR #14349.

Wasmtime GitHub notifications bot (Sep 16 2026 at 20:45):

github-actions[bot] added the label cranelift:area:machinst on PR #14349.

Wasmtime GitHub notifications bot (Sep 16 2026 at 20:45):

github-actions[bot] added the label cranelift:area:aarch64 on PR #14349.

Wasmtime GitHub notifications bot (Sep 16 2026 at 20:45):

github-actions[bot] added the label cranelift:area:x64 on PR #14349.

Wasmtime GitHub notifications bot (Sep 16 2026 at 20:45):

github-actions[bot] added the label wasmtime:api on PR #14349.

Wasmtime GitHub notifications bot (Sep 16 2026 at 20:45):

github-actions[bot] added the label wasmtime:config on PR #14349.

Wasmtime GitHub notifications bot (Sep 16 2026 at 20:46):

github-actions[bot] commented on PR #14349:

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:

```text
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.
```

</details>

[ ] 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][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][fuzzing-docs] for more details.

</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.

[fuzzing-config]: https://github.com/bytecodealliance/wasmtime/blob/ca0e8d0a1d8cefc0496dba2f77a670571d8fdcab/crates/fuzzing/src/generators.rs#L182-L194
[fuzzing-docs]: https://docs.wasmtime.dev/contributing-fuzzing.html

<details>

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

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

Learn more.

</details>

Wasmtime GitHub notifications bot (Sep 16 2026 at 21:02):

cfallin commented on PR #14349:

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

Wasmtime GitHub notifications bot (Sep 17 2026 at 20:25):

:memo: saulecabrera submitted PR review:

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 issue_a.wat

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

Wasmtime GitHub notifications bot (Sep 17 2026 at 23:40):

:memo: saulecabrera submitted PR review:

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

Wasmtime GitHub notifications bot (Sep 18 2026 at 15:40):

macovedj updated PR #14349.

Wasmtime GitHub notifications bot (Sep 18 2026 at 15:40):

macovedj requested fitzgen for a review on PR #14349.

Wasmtime GitHub notifications bot (Sep 18 2026 at 15:40):

macovedj requested wasmtime-fuzz-reviewers for a review on PR #14349.

Wasmtime GitHub notifications bot (Sep 18 2026 at 15:46):

macovedj commented on PR #14349:

Thanks Saul!
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.

Wasmtime GitHub notifications bot (Sep 18 2026 at 15:58):

macovedj edited a comment on PR #14349:

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.

Wasmtime GitHub notifications bot (Sep 18 2026 at 17:44):

github-actions[bot] added the label fuzzing on PR #14349.

Wasmtime GitHub notifications bot (Sep 18 2026 at 17:44):

github-actions[bot] commented on PR #14349:

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 <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Sep 21 2026 at 17:53):

:memo: cfallin submitted PR review:

Looks generally fine, thanks! A number of nits below but nothing fundamental about the approach in Winch itself.

Wasmtime GitHub notifications bot (Sep 21 2026 at 17:53):

:speech_balloon: cfallin created PR review comment:

Rather than winch_callee_pop (which is an implementation detail) maybe winch_tail_calls ?

Wasmtime GitHub notifications bot (Sep 21 2026 at 17:53):

:speech_balloon: cfallin created PR review comment:

Is this unit-test necessary? We don't have any other tests specifically for the fuzzing config; we see it more as configuration that itself directs the testing. I also find the test_n_times(10, ...) a bit strange -- is this trying to patch over some nondeterminism or something?

Wasmtime GitHub notifications bot (Sep 21 2026 at 17:53):

:speech_balloon: cfallin created PR review comment:

Is there any reason that this test is Winch-only? It seems to be testing accurate backtracing in general; we should check that Cranelift can get this right too.

(I vaguely recall some sort of callstack model in our fuzzing -- is there anything we can unify this with?)

Wasmtime GitHub notifications bot (Sep 21 2026 at 17:53):

:speech_balloon: cfallin created PR review comment:

No narrative comments (LLMs love these for some reason) -- "the previously failing 64/80-argument tails" tells a reader of the current tree nothing at all, because no one has seen the path that this implementation took. Just describe the case if it's relevant, or avoid the comment otherwise.

Wasmtime GitHub notifications bot (Sep 21 2026 at 17:53):

:speech_balloon: cfallin created PR review comment:

Let's not create a brittle tie between one test and another like this -- include the WAT directly here as a literal string (raw r#"..."# if you want).

Wasmtime GitHub notifications bot (Sep 21 2026 at 17:53):

:speech_balloon: cfallin created PR review comment:

For this and other tests below that are pure "instantiate this module and invoke this function, expecting this result", is there a reason we can't write a WAST test instead? In general we should only add actual integration tests in Rust like this when we need ad-hoc access to the Wasmtime API (e.g. to check the backtrace from inside a call or cause an interruption or whatever).

Wasmtime GitHub notifications bot (Sep 21 2026 at 17:53):

:speech_balloon: cfallin created PR review comment:

As with above, I don't think we need to start a whole unit-test section here; this code gets test coverage via the layers above (including especially disas tests as well as full execution tests).

Wasmtime GitHub notifications bot (Sep 21 2026 at 19:09):

:speech_balloon: saulecabrera created PR review comment:

I would avoid putting any ISA-specific comments here, since there's a low-guarantee that we will keep them up to date. I think it's fine to delegate this to the ISA-specific pieces in Winch (the current state).

Wasmtime GitHub notifications bot (Sep 21 2026 at 19:09):

:memo: saulecabrera submitted PR review.

Wasmtime GitHub notifications bot (Sep 21 2026 at 20:16):

macovedj requested wasmtime-default-reviewers for a review on PR #14349.

Wasmtime GitHub notifications bot (Sep 21 2026 at 20:16):

macovedj updated PR #14349.

Wasmtime GitHub notifications bot (Sep 21 2026 at 20:17):

:memo: macovedj submitted PR review.

Wasmtime GitHub notifications bot (Sep 21 2026 at 20:17):

:speech_balloon: macovedj created PR review comment:

Removed this test. The ten iterations were sampling generated configurations, not retrying failures or working around nondeterminism.

Wasmtime GitHub notifications bot (Sep 21 2026 at 20:17):

:memo: macovedj submitted PR review.

Wasmtime GitHub notifications bot (Sep 21 2026 at 20:17):

:speech_balloon: macovedj created PR review comment:

Ah sorry I should have caught this

Wasmtime GitHub notifications bot (Sep 21 2026 at 20:17):

:memo: macovedj submitted PR review.

Wasmtime GitHub notifications bot (Sep 21 2026 at 20:17):

:speech_balloon: macovedj created PR review comment:

Made this test compiler-generic. There’s some overlap, but this test adds focused coverage for callback backtrace capture, host errors/panics, indirect tail calls, and large mixed-result signatures. My inclination is to keep those checks here rather than fold them into the generator.

Wasmtime GitHub notifications bot (Sep 21 2026 at 20:18):

macovedj commented on PR #14349:

Thanks both for the review! I believe I've addressed all of your feedback. I also updated the compiler-support tables in stability-tiers.md to mark tail calls as supported by Winch on x86-64 and AArch64.

Wasmtime GitHub notifications bot (Sep 21 2026 at 21:44):

github-actions[bot] added the label wasmtime:docs on PR #14349.

Wasmtime GitHub notifications bot (Sep 22 2026 at 14:04):

:thumbs_up: saulecabrera submitted PR review:

LGTM, thanks for iterating on this.

By the way, while fuzzing this PR, I came across https://github.com/bytecodealliance/wasmtime/issues/14377. That one is unrelated to this PR.

Wasmtime GitHub notifications bot (Sep 22 2026 at 14:06):

:thumbs_up: saulecabrera submitted PR review:

LGTM, thanks for iterating on this.

By the way, while fuzzing this PR, I came across https://github.com/bytecodealliance/wasmtime/issues/14377 (unrelated to this change).

Wasmtime GitHub notifications bot (Sep 22 2026 at 15:46):

:memo: cfallin submitted PR review.

Wasmtime GitHub notifications bot (Sep 22 2026 at 15:46):

:speech_balloon: cfallin created PR review comment:

Cool, that sounds good then -- thanks for checking.

Wasmtime GitHub notifications bot (Sep 22 2026 at 15:46):

:thumbs_up: cfallin submitted PR review:

Looks good to me as well -- thanks!

Wasmtime GitHub notifications bot (Sep 22 2026 at 15:46):

cfallin added PR #14349 winch: support tail calls with callee cleanup to the merge queue.

Wasmtime GitHub notifications bot (Sep 22 2026 at 16:14):

:check: cfallin merged PR #14349.

Wasmtime GitHub notifications bot (Sep 22 2026 at 16:14):

cfallin removed PR #14349 winch: support tail calls with callee cleanup from the merge queue.

Last updated: Oct 11 2026 at 04:10 UTC