-
Notifications
You must be signed in to change notification settings - Fork 747
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
[Emscripten port] Improve emcc flags #6349
Conversation
I'm somewhat saddendend that SINGLE_FILE is so necessary/popular these days. I OK landing this but I wonder if we can come up with some of the higher level way to package these kind of things. Some kind of zipfile/container thing that could then be executable on the OS direclty (via #! on UNIX)? |
@@ -493,6 +509,7 @@ if(EMSCRIPTEN) | |||
target_link_libraries(binaryen_js "-sFILESYSTEM=1") | |||
endif() | |||
target_link_libraries(binaryen_js "-sNODERAWFS=0") | |||
target_link_libraries(binaryen_js "-sSINGLE_FILE") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need to specify this twice? i.e. do we not have a place to specify flag for both _js and _wasm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have some duplication here that we've never figured out a way to remove... I'm not sure if CMake has a way? We basically want to "inherit" one target from another.
We can perhaps remove SINGLE_FILE from |
No changes here to binaryen.js/wasm builds. 1. Add a flag to enable pthreads. 2. Use SINGLE_FILE on binaryen.js/.wasm as before, which is nice for library users as they want just a single file to distribute for Binaryen support. For other builds like wasm-opt.js etc. no longer set SINGLE_FILE, as that type of build wants to be a replacement for a normal wasm-opt build as much as possible, so avoid the overhead of SINGLE_FILE. (Previously we disabled SINGLE_FILE also in the case of BUILD_FOR_BROWSER but I don't think we need to special-case that any more.)
Add a flag to enable pthreads.
Use
SINGLE_FILE
onbinaryen.js/.wasm
as before, which is nice for library usersas they want just a single file to distribute for Binaryen support. For other builds
like
wasm-opt.js
etc. no longer setSINGLE_FILE
, as that type of build wants to bea replacement for a normal
wasm-opt
build as much as possible, so avoid theoverhead of
SINGLE_FILE
.(Previously we disabled
SINGLE_FILE
also in the case ofBUILD_FOR_BROWSER
but I don't think we need to special-case that any more.)
This is a step towards adding a
wasm-opt.js
build in our releases: We needpthreads there, and we don't want
SINGLE_FILE
.