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 wasm-opt MVP features only #891

Merged
merged 1 commit into from
Jan 5, 2023
Merged

Conversation

ascjones
Copy link
Collaborator

@ascjones ascjones commented Jan 5, 2023

As @brson pointed out in #888, this upgrade brings new Wasm features enabled by default. @Robbepop says these are supported by wasmi but still need to be enabled in pallet-contracts (/cc @athei).

Originally I intended just to revert to wasm-opt 0.110, but noticed in the CHANGELOG that there is the option of .mvp_features_only(), which I have added as an alternative in this PR.

@Robbepop
Copy link
Contributor

Robbepop commented Jan 5, 2023

Since LLVM enables those Wasm proposals by default, too I wonder where we deny those proposals for Rust/LLVM? 🤔

@ascjones ascjones merged commit a22f49a into master Jan 5, 2023
@ascjones ascjones deleted the aj/wasm-opt-mvp-features branch January 5, 2023 11:57
@ascjones
Copy link
Collaborator Author

ascjones commented Jan 5, 2023

Since LLVM enables those Wasm proposals by default, too I wonder where we deny those proposals for Rust/LLVM? thinking

@brson do you have any insight into this?

@brson
Copy link
Contributor

brson commented Jan 5, 2023

@ascjones @Robbepop I looked a little deeper, and here is the LLVM change referenced by the binaryen release notes on this subject:

https://reviews.llvm.org/D125728

It appears to actually be a change to clang's default wasm configuration, not LLVM's. It is also scheduled for the LLVM 16 release, while rustc nightly is on 15. So as far as I can tell, rustc should not enable this features unless it does so explicitly.

@athei
Copy link
Contributor

athei commented Jan 10, 2023

We will only enable any proposals if we can proof that they benefit us. Because all they do is make life more complicated. Look at the multi value proposal which would make returning from functions a non constant instruction. If you overlook this fact you created yourself a security vulnerability. So there needs to be a very strong case to allow any proposals on pallet-contracts.

@ascjones ascjones mentioned this pull request Feb 14, 2023
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