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

Support emcc 'upstream' branch #10

Merged
merged 2 commits into from Sep 16, 2019

Conversation

@Beuc
Copy link
Contributor

commented Sep 13, 2019

No description provided.

@@ -17,7 +17,7 @@
# make install prefix=$HOME

# -s WASM=0 -s SINGLE_FILE=1: allow loading zee.js synchronously
EMCC=emcc -O2 -s WASM=0 -s SINGLE_FILE=1 -s EXPORTED_FUNCTIONS="['_gzopen', '_gzread', '_gzwrite', '_gzclose', '_gzerror', '_gzeof']" --pre-js pre.js --post-js post.js
EMCC=emcc -O2 -s WASM=0 -s WASM_ASYNC_COMPILATION=0 -s SINGLE_FILE=1 -s EXPORTED_FUNCTIONS="['_gzopen', '_gzread', '_gzwrite', '_gzclose', '_gzerror', '_gzeof']" --pre-js pre.js --post-js post.js

This comment has been minimized.

Copy link
@kripken

kripken Sep 14, 2019

Owner

is the output file small enough to work in chrome?

This comment has been minimized.

Copy link
@Beuc

Beuc Sep 14, 2019

Author Contributor

Isn't WASM=0 meant to deal with this?

This comment has been minimized.

Copy link
@kripken

kripken Sep 16, 2019

Owner

Oh, I missed that. Yeah, without wasm there isn't an issue with code size. However, then we can remove WASM_ASYNC_COMPILATION as it has no effect.

This comment has been minimized.

Copy link
@Beuc

Beuc Sep 16, 2019

Author Contributor

nodejs test.js strongly disagrees :)
Assertion failed: you need to wait for the runtime to be ready (e.g. wait for main() to be called)

This comment has been minimized.

Copy link
@kripken

kripken Sep 16, 2019

Owner

Ah, yes, I forgot - in upstream, wasm=0 tracks the wasm=1 path very closely, to make it easier to swap between the two. In particular, it looks like wasm all through the instantiation, all using a fake WebAssembly object. And that includes async startup. So yes, it is necessary there. Sorry for the mixup!

This comment has been minimized.

Copy link
@Beuc

Beuc Sep 16, 2019

Author Contributor

Thanks -- it's somewhat reassuring to see I'm not the only one struggling to keep all this in mind ;)

@kripken kripken merged commit 4324d2c into kripken:master Sep 16, 2019
@Beuc Beuc deleted the Beuc:patch-2 branch Sep 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.