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

Feat/wasmer.wasm #2835

Closed
wants to merge 21 commits into from
Closed

Feat/wasmer.wasm #2835

wants to merge 21 commits into from

Conversation

ptitSeb
Copy link
Contributor

@ptitSeb ptitSeb commented Apr 4, 2022

Description

Allow WASMER to be built as WASM, with compile ability (only singlepass compiler and universal engine for now)

@ptitSeb ptitSeb marked this pull request as ready for review April 4, 2022 15:23
@ptitSeb ptitSeb requested a review from syrusakbary as a code owner April 4, 2022 15:23
@syrusakbary
Copy link
Member

Side question: what is the diff between api/sys and api/wasm ? (in code, what are the things unused? Both look pretty similar to me but I might be missing something!)

@ptitSeb
Copy link
Contributor Author

ptitSeb commented Apr 5, 2022

Side question: what is the diff between api/sys and api/wasm ? (in code, what are the things unused? Both look pretty similar to me but I might be missing something!)

Many "runtime"' only parts are just empty, like native.rs, but also many files from externals, like globals. It's actualy a mix between js and sys, but with some parts removed and left as "unimplemented".

@ptitSeb ptitSeb requested a review from Amanieu April 6, 2022 10:56
@syrusakbary
Copy link
Member

Cranelift is now updated in master. Can you update the PR and also see if we can get cranelift working?

@ptitSeb
Copy link
Contributor Author

ptitSeb commented Apr 7, 2022

Cranelift is now updated in master. Can you update the PR and also see if we can get cranelift working?

You can merge this PR and I'll work on Cranelift next week.

@Amanieu
Copy link
Contributor

Amanieu commented Apr 7, 2022

I'm concerned about the amount of code duplication in this PR. It introduces a whole new copy of the vm and api crates which are full of unimplemented stubs. I would prefer a cleaner structure where wasmer.wasm is a separate tool that only depends on the compiler crates without depending on the rest of the engine.

@ptitSeb
Copy link
Contributor Author

ptitSeb commented Apr 7, 2022

I'm concerned about the amount of code duplication in this PR. It introduces a whole new copy of the vm and api crates which are full of unimplemented stubs. I would prefer a cleaner structure where wasmer.wasm is a separate tool that only depends on the compiler crates without depending on the rest of the engine.

Note that they are unimplemented because they will not work in a wasm context (not because I'm lazy).

@ptitSeb
Copy link
Contributor Author

ptitSeb commented Apr 7, 2022

I'm concerned about the amount of code duplication in this PR. It introduces a whole new copy of the vm and api crates which are full of unimplemented stubs. I would prefer a cleaner structure where wasmer.wasm is a separate tool that only depends on the compiler crates without depending on the rest of the engine.

I'm not sure how that would reduce code duplication. Both engine & compiler are needed, and that's already mostly how wasmer.wasm is built, with as minimum as possible dependancies, but while trying to limit the amount of #cfg conditionnal build.

@syrusakbary
Copy link
Member

Closing this PR as it was done and merged in #2851

@syrusakbary syrusakbary closed this May 6, 2022
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.

3 participants