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

Enable ref types by default #3894

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

Conversation

Zzzabiyaka
Copy link
Contributor

@Zzzabiyaka Zzzabiyaka commented Nov 4, 2024

After Rust 1.82.0 was released enabling reftypes by default for rust wasm target https://blog.rust-lang.org/2024/09/24/webassembly-targets-change-in-default-target-features.html#enabling-reference-types-by-default I think this would be helpful for users to have this enabled by default in runtime as well, so that rust file compiled to wasm with the default flags can be run on wamr compiled with the default flags

@loganek
Copy link
Collaborator

loganek commented Nov 4, 2024

I'm not against this change, however, I think by default WAMR tries to be as lean as possible. Not sure if @wenyongh and others have any thoughts on that.

@Zzzabiyaka
Copy link
Contributor Author

I'm not against this change, however, I think by default WAMR tries to be as lean as possible. Not sure if @wenyongh and others have any thoughts on that.

Thanks, yeah I am happy to discuss that.

I just find the flow to be easier if anyone who just compiled wasm from rust can run it on our runtime without issues.

If wamr needs to be used in prod producing minimal binary possible this can be set overriding the default value

@lum1n0us
Copy link
Collaborator

lum1n0us commented Nov 5, 2024

I'm also struggling with this. On one side, ref-types are now part of the specification and should be supported by default by any WebAssembly runtime. On the other side, we need to keep the binary size as small as possible.

Here's a suggestion:

  • We always release binaries of iwasm and wamrc that are fully compatible with the specification and offer better performance. This way, users can easily run WebAssembly code from Rust.
  • (Optional) We provide separate compilation groups for full specification compatibility and for minimal binary size through something like CMakePresets.json. This allows developers to switch quickly.
  • By default, we enable full specification compatibility. I mean enable ref-types by default.

Let's make a vote ?

@wenyongh
Copy link
Contributor

wenyongh commented Nov 5, 2024

It is good to me if it brings convenience and has little impact on footprint and performance. And yes, not sure what others' opinions are, maybe a vote is better.

@wenyongh
Copy link
Contributor

wenyongh commented Nov 5, 2024

BTW, if to enable ref-types by default, had better also change the CMakeLists.txt under product-mini/platforms/windows and product-mini/platforms/cosmopolitan.

@Zzzabiyaka
Copy link
Contributor Author

@wenyongh, how do we run the vote?

should I just ask all TSC members to 👍 or 👎 the initial pr comment?

Copy link
Collaborator

@lum1n0us lum1n0us left a comment

Choose a reason for hiding this comment

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

LGTM

@lum1n0us
Copy link
Collaborator

#3913 (comment)

The recent version of the rust toolchain will emit ref types opcodes, need to enable this feature in the iwasm build.

It is a strong reason we need this one now.

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