Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug in wasm2c's tail call optimization codegen #2491

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

squk
Copy link
Contributor

@squk squk commented Oct 18, 2024

The default wasm-rt implementation defines wasm_rt_memcpy as memcpy which expects void*

Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

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

Does the old output code even compile?

@keithw
Copy link
Member

keithw commented Oct 18, 2024

I'm a little confused -- we pass the tail-call tests, so... this suggests we need some additional testing here. How is this code-path not hit by the current tests?

@keithw
Copy link
Member

keithw commented Nov 7, 2024

I guess there are no spec tests for tail-calls that use multi-value? If so that seems like an omission we should fix. @squk , are you up for either:

  • adding a test to this repo (of using tail-call with multi-value) that this PR will make green, and/or
  • submitting one upstream to the tail-call proposal repo?

If you don't have cycles for this or don't want to, I think we could probably take it from here too, but I'd love to have a test somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants