Skip to content

Binaryen stack branch support #4528

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

Merged
merged 7 commits into from
Sep 7, 2016
Merged

Binaryen stack branch support #4528

merged 7 commits into from
Sep 7, 2016

Conversation

kripken
Copy link
Member

@kripken kripken commented Aug 29, 2016

This branch works together with binaryen's stack branch in WebAssembly/binaryen#678. We should merge them only together.

Nothing major here, just mem init file improvements (as binaryen can now import it into asm2wasm modules) and some fuzzing improvements.

if shared.Settings.BINARYEN_IMPRECISE:
cmd += ['--imprecise']
if opt_level == 0:
cmd += ['--no-opts']
# import mem init file if it exists, and if we will not be using asm.js as a binaryen method (as it needs the mem init file, of course)
import_mem_init = memory_init_file and 'asmjs' not in shared.Settings.BINARYEN_METHOD and 'interpret-asm2wasm' not in shared.Settings.BINARYEN_METHOD
if import_mem_init and os.path.exists(memfile):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just checking, is the semantic of import_mem_init at this point intended to be import_mem_init_if_it_exists or import_mem_init_and_it_should_exist? This line feels odd since above we unconditionally did set memory_init_file = True?

@kripken
Copy link
Member Author

kripken commented Sep 7, 2016

stack branch landed in binaryen master, so this could land.

I added a commit to update the binaryen port to current master. This is needed since we have some flag changes for emcc to pass to binaryen (importing memory, no need to mappedGlobals). This would only affect users that did not set BINARYEN_ROOT of course, but for people using binaryen in the simple way (through ports, automatically), it updates them to latest master. @dschuff, any concerns with landing this to incoming?

@dschuff
Copy link
Member

dschuff commented Sep 7, 2016

No, this looks fine.

@kripken kripken merged commit b405077 into incoming Sep 7, 2016
@kripken
Copy link
Member Author

kripken commented Sep 7, 2016

Cool, thanks.

@kripken kripken deleted the binaryen-stack branch September 7, 2016 21:03
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