Bug 215275 – Support WebAssembly tail calls

• Status: REOPENED
• Component: JavaScriptCore

[Alessandro Pignotti]

Hi,

we have a use case that requires WebAssembly tail calls. You can find more information in this thread: https://github.com/WebAssembly/tail-call/issues/12

From what I can see by inspecting the repo, no work has been done so far to support WebAssembly tail calls in JavaScriptCore/Safari. I want to explicitly ask if there are plans to implement this feature and what is the expected timeline, if any.

Regards,
Alessandro

[Radar WebKit Bug Importer]

<rdar://problem/67081070>

[Tom Tartarin]

Created attachment 457113 [details]
Patch

[EWS Watchlist]

This patch modifies one of the wasm.json files. Please ensure that any changes in one have been mirrored to the other. You can find the wasm.json files at "Source/JavaScriptCore/wasm/wasm.json" and "JSTests/wasm/wasm.json".

[Tom Tartarin]

As a follow up:

This is a WIP patch to gather some feedback before potentially going further.

Known limitations:
- B3 and AIR are stubbed
- probably some portability issues (only tested on x86_64)
- tests could be more extensive (multiple return values, recursive calls etc..)
- some thinking could be put into formatting the code in slowPathForWasmReturnCall

From the caller's point of view, this consists of reusing the previous frame with the callee's stack arguments/return values.


Right now, a slowPathForWasmTailCall is defined in WebAssembly.asm and will be only invoked for tail variants of call and call_indirect.
Conceptually it is a merge between slowPathForWasmCall and prepareForTailCall, that deals with passing arguments via registers.
The numberOfCallerStackArguments is also encoded in the corresponding bytecode.


Beside formatting and naming in the patch I'd like to discuss the viability of the implementation in the interpreter. In the context of LLIntPlan I didn't find a satisfactory way to JIT a specific entry thunk, and adding a dynamic check ( similar to what happens in JS) made less sense than creating a slowPath for tail variant of calls in the interpreter.

I also included some tests that cover basic cases.

For reference:
https://github.com/WebAssembly/tail-call

[Yusuke Suzuki]

Comment on attachment 457113 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=457113&action=review

Cool, will look later today.

> Source/JavaScriptCore/ChangeLog:3
> +        Support for WebAssembly tail calls

Let's rename bug and patch's title to say, Support WebAssembly tail calls in LLInt tier

[Alessandro Pignotti]

Updated the title as suggested

[Yusuke Suzuki]

Hmm, seems that EWS gets red. Can you take a look?

[Tom Tartarin]

Yes, I'm on it.

[Tom Tartarin]

Created attachment 458500 [details]
Patch

[Tom Tartarin]

Created attachment 458515 [details]
with Changelogs

[Tom Tartarin]

To my knowledge, the limitations mentioned in comment 4 have been addressed. B3 and Air still redirect to a normal call as the bug focuses on the LLInt tier for now.

[Tom Tartarin]

Created attachment 460584 [details]
Change returns offsets in wasm convention

[Tom Tartarin]

Created attachment 460585 [details]
Implement wasm tail calls

[Tom Tartarin]

I went on and implemented it in the other tiers, hence we should change the bug title.

I think the convention would need to change the locations of the stack return values if we were to follow the reasoning of unconditionally reusing the previous frame when tail-calling: 
With the current convention, the callee stack values are written from stackPointer + headerSize towards framePointer (caller's POV). When a frame is reused, the stack size for argument/returns can grow depending on the callee, which means that when at the return location of the original frame, given that the stack pointer is restored, the caller might not found the values at the right offsets, or they may even be lost.
In the first patch, we always align the stack size (max(stackArgs, stackReturns)) in our calculation, and start writing the return values at offset header + stackSize - numberOfReturnValues (still caller's POV). This way, return values are at the same locations, even if the frame is recycled in between.
In the generators, I am not sure how much refactoring is allowed since most of the code for ReturnCall is similar to Call.

[Tom Tartarin]

Created attachment 460587 [details]
Implement wasm tail calls

[Tom Tartarin]

Created attachment 460626 [details]
Implement wasm tail calls

Attempt #1 to fix arm build

[Yusuke Suzuki]

@Tom BTW, WebKit dev has slack workspace for discussions, where anyone can join.
https://webkit.org/getting-started/#staying-in-touch
And there are #jsc channel, where JSC developers are talking about things.
So, you can join, request reviews, and discuss the design etc. :)

[Tom Tartarin]

Pull request: https://github.com/WebKit/WebKit/pull/2065

[Tom Tartarin]

*** Bug 242322 has been marked as a duplicate of this bug. ***

[Tom Tartarin]

Thanks! I joined

[EWS]

Committed 258008@main (5ee0680f6669): <https://commits.webkit.org/258008@main>

Reviewed commits have been landed. Closing PR #2065 and removing active labels.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 249492

[Sanjay Kumar]

Why is this still open if blocker is indeed resolved? WASM in Safari still does not have tail calls.

[Sergey Rubanov]

Wasm tail calls have been enabled in https://github.com/WebKit/WebKit/commit/c3a1babc1e75481763edce5a9e46cb66c15639b8