Skip to content
This repository was archived by the owner on Apr 25, 2025. It is now read-only.

[interpreter] Cherry-pick tail call support #274

Merged
merged 2 commits into from
Apr 23, 2023

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Apr 21, 2023

This cherry-picks interpreter changes from
https://github.com/WebAssembly/tail-call to run spec tests that mix trys with return_call(_indirect)s. (#275) When tail-call is merged to the main spec repo, we can revert this change and merge the upstream spec.

This also adds missing handling for ReturningInvoke in the new Catch/Caught/Delegate instructions in the evaluator.

This cherry-picks interpreter changes from
https://github.com/WebAssembly/tail-call to run spec tests that mix
`try`s with `return_call(_indirect)`s (WebAssembly#275). When tail-call is merged
to the main spec repo, we can revert this change and merge the upstream
spec.
@aheejin aheejin force-pushed the tail_call_interpreter branch from 3bb7799 to c2cf4e8 Compare April 21, 2023 09:18
aheejin added a commit to aheejin/exception-handling that referenced this pull request Apr 21, 2023
This adds tests of `return_call(_indirect)`s within `try`s. Because this
repo's interpreter doesn't have the tail call support, this requires
 WebAssembly#274 to be merged before merging. But currently even with #??? the
tests here don't run. When a `return_call(_indirect)` run within a
`try`, it errors out with
```
runtime crash: undefined frame
```
I think we need to fix the interpreter first before merging this.

Closes WebAssembly#249.
aheejin added a commit to aheejin/exception-handling that referenced this pull request Apr 21, 2023
This adds tests of `return_call(_indirect)`s within `try`s. Because this
repo's interpreter doesn't have the tail call support, this requires
 WebAssembly#274 to be merged before merging. But currently even with WebAssembly#275 the
tests here don't run. When a `return_call(_indirect)` run within a
`try`, it errors out with
```
runtime crash: undefined frame
```
I think we need to fix the interpreter first before merging this.

Closes WebAssembly#249.
Copy link
Member

@rossberg rossberg left a comment

Choose a reason for hiding this comment

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

The evaluator will also need to define the interaction between ReturningInvoke and the new Catch/Caught/Delegate instructions. I believe that should merely consist of adding ReturningInvoke to the enumerated cases in lines 721, 746, and 762.

@aheejin
Copy link
Member Author

aheejin commented Apr 23, 2023

Thanks!

@aheejin aheejin merged commit e15eb41 into WebAssembly:main Apr 23, 2023
@aheejin aheejin deleted the tail_call_interpreter branch April 23, 2023 07:37
aheejin added a commit that referenced this pull request Apr 23, 2023
This adds tests of `return_call(_indirect)`s within `try`s. Because this
repo's interpreter doesn't have the tail call support, this requires
 #274 to be merged before merging.

These test also needs `--experimental-wasm-return_call` to `node`
argument in the CI to pass.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants