Skip to content

Commit 5ce6b39

Browse files
committed
[emscripten] Drop -sWASM=0 variant build
There is now just one emscripten target called `binaryen_js`. The settings for these two targets were almost identical except for. 1. Some JS_OF_OCAML specific stuff which is kept for the new unified target. 2. `-sASSERTIONS` was being forced (See #2507). I dropped this in favor of doing a debug build in testing where this is enabled by default.
1 parent 16f7f8d commit 5ce6b39

File tree

7 files changed

+37
-89
lines changed

7 files changed

+37
-89
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ CMakeFiles
2929
/dist/
3030
/config.h
3131
/emcc-build
32+
/emcc-build-dbg
3233
compile_commands.json
3334
test/lit/lit.site.cfg.py
3435
CMakeUserPresets.json

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,11 @@ full changeset diff at the end of each section.
1414

1515
Current Trunk
1616
-------------
17+
- The emscripten build of binaryen no longer targets pure JS (via wasm2js) by
18+
default. In the long run this will allow us to enable WASM_BIGINT and other
19+
features that wasm2js does not support. There is now just a single
20+
binaryen_js target. It should still be possible to inject `-sWASM=0` as a
21+
linker flag but this is not officially supported. (#7995)
1722

1823
v125
1924
----

CMakeLists.txt

Lines changed: 14 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -537,90 +537,41 @@ endif()
537537
# binaryen.js
538538
#
539539
# Note that we can't emit binaryen.js directly, as there is libbinaryen already
540-
# declared earlier, so we create binaryen_wasm/js.js, which must then be copied.
540+
# declared earlier, so we create binaryen_js.js, which must then be copied.
541541
if(EMSCRIPTEN)
542-
# binaryen.js WebAssembly variant
543-
add_executable(binaryen_wasm ${binaryen_HEADERS})
542+
add_executable(binaryen_js ${binaryen_HEADERS})
544543
# For the emscripten build we link against libbinaryen wih `--whole-archive`.
545544
# Without this, the EMSCRIPTEN_KEEPALIVE symbols within the library would
546545
# be ignored.
547-
target_link_libraries(binaryen_wasm PRIVATE binaryen)
548-
target_link_libraries(binaryen_wasm PRIVATE "-sFILESYSTEM")
549-
target_link_libraries(binaryen_wasm PRIVATE "-sEXPORT_NAME=Binaryen")
550-
target_link_libraries(binaryen_wasm PRIVATE "-sNODERAWFS=0")
551-
# Do not error on the repeated NODERAWFS argument
552-
target_link_libraries(binaryen_wasm PRIVATE "-Wno-unused-command-line-argument")
553-
# Emit a single file for convenience of people using binaryen.js as a library,
554-
# so they only need to distribute a single file.
555-
if(EMSCRIPTEN_ENABLE_SINGLE_FILE)
556-
target_link_libraries(binaryen_wasm PRIVATE "-sSINGLE_FILE")
557-
endif()
558-
target_link_libraries(binaryen_wasm PRIVATE "-sEXPORT_ES6")
559-
target_link_libraries(binaryen_wasm PRIVATE "-sEXPORTED_RUNTIME_METHODS=stringToUTF8OnStack,stringToAscii")
560-
# Explictly export _i32_load to ensure the binaryen-c.cpp is linked in.
561-
# This is the file that contains all the EMSCRIPTEN_KEEPALIVE symbols which
562-
# we want to exports, but enless we explictly reference a symbol from it
563-
# the linker will ignore the whole library.
564-
# TODO(sbc): Replace with `LINK_LIBRARY:WHOLE_ARCHIVE` once we bump the min
565-
# cmake version 3.24.
566-
target_link_libraries(binaryen_wasm PRIVATE "-sEXPORTED_FUNCTIONS=_malloc,_free,__i32_load")
567-
target_link_libraries(binaryen_wasm PRIVATE "--post-js=${CMAKE_CURRENT_SOURCE_DIR}/src/js/binaryen.js-post.js")
568-
target_link_libraries(binaryen_wasm PRIVATE optimized "--closure=1")
569-
# TODO: Fix closure warnings! (#5062)
570-
target_link_libraries(binaryen_wasm PRIVATE optimized "-Wno-error=closure")
571-
target_link_libraries(binaryen_wasm PRIVATE debug "--profiling")
572-
# Avoid catching exit as that can confuse error reporting in Node,
573-
# see https://github.com/emscripten-core/emscripten/issues/17228
574-
target_link_libraries(binaryen_wasm PRIVATE "-sNODEJS_CATCH_EXIT=0")
575-
install(TARGETS binaryen_wasm DESTINATION ${CMAKE_INSTALL_BINDIR})
576-
577-
# binaryen.js JavaScript variant
578-
add_executable(binaryen_js ${binaryen_HEADERS})
579546
target_link_libraries(binaryen_js PRIVATE binaryen)
580-
target_link_libraries(binaryen_js PRIVATE "-sWASM=0")
581-
target_link_libraries(binaryen_js PRIVATE "-sWASM_ASYNC_COMPILATION=0")
582-
583-
if(${CMAKE_CXX_COMPILER_VERSION} STREQUAL "6.0.1")
584-
# only valid with fastcomp and WASM=0
585-
target_link_libraries(binaryen_js PRIVATE "-sELIMINATE_DUPLICATE_FUNCTIONS")
586-
endif()
587-
# Disabling filesystem and setting web environment for js_of_ocaml
588-
# so it doesn't try to detect the "node" environment
589-
if(JS_OF_OCAML)
590-
target_link_libraries(binaryen_js PRIVATE "-sFILESYSTEM=0")
591-
target_link_libraries(binaryen_js PRIVATE "-sENVIRONMENT=web,worker")
592-
else()
593-
target_link_libraries(binaryen_js PRIVATE "-sFILESYSTEM=1")
594-
endif()
547+
target_link_libraries(binaryen_js PRIVATE "-sFILESYSTEM")
548+
target_link_libraries(binaryen_js PRIVATE "-sEXPORT_NAME=Binaryen")
595549
target_link_libraries(binaryen_js PRIVATE "-sNODERAWFS=0")
596550
# Do not error on the repeated NODERAWFS argument
597551
target_link_libraries(binaryen_js PRIVATE "-Wno-unused-command-line-argument")
552+
# Emit a single file for convenience of people using binaryen.js as a library,
553+
# so they only need to distribute a single file.
598554
if(EMSCRIPTEN_ENABLE_SINGLE_FILE)
599555
target_link_libraries(binaryen_js PRIVATE "-sSINGLE_FILE")
600556
endif()
601-
target_link_libraries(binaryen_js PRIVATE "-sEXPORT_NAME=Binaryen")
602-
# Currently, js_of_ocaml can only process ES5 code
603557
if(JS_OF_OCAML)
604-
target_link_libraries(binaryen_js PRIVATE "-sEXPORT_ES6=0")
558+
# js_of_ocaml needs a specified variable with special comment to provide the library to consumer
559+
target_link_libraries(binaryen_js PRIVATE "--extern-pre-js=${CMAKE_CURRENT_SOURCE_DIR}/src/js/binaryen.jsoo-extern-pre.js")
560+
# Currently, js_of_ocaml can only process ES5 code
561+
target_link_libraries(binaryen_js PRIVATE optimized "--closure-args=\"--language_out=ECMASCRIPT5\"")
605562
else()
606-
target_link_libraries(binaryen_js PRIVATE "-sEXPORT_ES6=1")
563+
# Currently, js_of_ocaml can only process ES5 code
564+
target_link_libraries(binaryen_js PRIVATE "-sEXPORT_ES6")
607565
endif()
608566
target_link_libraries(binaryen_js PRIVATE "-sEXPORTED_RUNTIME_METHODS=stringToUTF8OnStack,stringToAscii")
609567
target_link_libraries(binaryen_js PRIVATE "-sEXPORTED_FUNCTIONS=_malloc,_free,__i32_load")
610568
target_link_libraries(binaryen_js PRIVATE "--post-js=${CMAKE_CURRENT_SOURCE_DIR}/src/js/binaryen.js-post.js")
611-
# js_of_ocaml needs a specified variable with special comment to provide the library to consumers
612-
if(JS_OF_OCAML)
613-
target_link_libraries(binaryen_js PRIVATE "--extern-pre-js=${CMAKE_CURRENT_SOURCE_DIR}/src/js/binaryen.jsoo-extern-pre.js")
614-
endif()
569+
target_link_libraries(binaryen_js PRIVATE "-msign-ext")
570+
target_link_libraries(binaryen_js PRIVATE "-mbulk-memory")
615571
target_link_libraries(binaryen_js PRIVATE optimized "--closure=1")
616-
# Currently, js_of_ocaml can only process ES5 code
617-
if(JS_OF_OCAML)
618-
target_link_libraries(binaryen_js PRIVATE optimized "--closure-args=\"--language_out=ECMASCRIPT5\"")
619-
endif()
620572
# TODO: Fix closure warnings! (#5062)
621573
target_link_libraries(binaryen_js PRIVATE optimized "-Wno-error=closure")
622574
target_link_libraries(binaryen_js PRIVATE debug "--profiling")
623-
target_link_libraries(binaryen_js PRIVATE debug "-sASSERTIONS")
624575
# Avoid catching exit as that can confuse error reporting in Node,
625576
# see https://github.com/emscripten-core/emscripten/issues/17228
626577
target_link_libraries(binaryen_js PRIVATE "-sNODEJS_CATCH_EXIT=0")

check.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,6 @@ def run():
419419
('example', run_example_tests),
420420
('unit', run_unittest),
421421
('binaryenjs', binaryenjs.test_binaryen_js),
422-
('binaryenjs_wasm', binaryenjs.test_binaryen_wasm),
423422
('lit', run_lit),
424423
('gtest', run_gtest),
425424
])
@@ -428,7 +427,7 @@ def run():
428427
# Run all the tests
429428
def main():
430429
all_suites = TEST_SUITES.keys()
431-
skip_by_default = ['binaryenjs', 'binaryenjs_wasm']
430+
skip_by_default = ['binaryenjs']
432431

433432
if shared.options.list_suites:
434433
for suite in all_suites:

scripts/emcc-tests.sh

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,17 @@ set -o pipefail
55

66
SRCDIR="$(dirname $(dirname ${BASH_SOURCE[0]}))"
77

8+
mkdir -p emcc-build-dbg
9+
echo "emcc-tests: build dbg"
10+
emcmake cmake -S $SRCDIR -B emcc-build-dbg -DCMAKE_BUILD_TYPE=Debug -G Ninja
11+
ninja -C emcc-build-dbg binaryen_js
12+
echo "emcc-tests: test dbg"
13+
$SRCDIR/check.py --binaryen-bin=emcc-build-dbg/bin binaryenjs
14+
815
mkdir -p emcc-build
9-
echo "emcc-tests: build:wasm"
16+
echo "emcc-tests: build"
1017
emcmake cmake -S $SRCDIR -B emcc-build -DCMAKE_BUILD_TYPE=Release -G Ninja
11-
ninja -C emcc-build binaryen_wasm
12-
echo "emcc-tests: test:wasm"
13-
$SRCDIR/check.py --binaryen-bin=emcc-build/bin binaryenjs_wasm
14-
echo "emcc-tests: done:wasm"
15-
16-
echo "emcc-tests: build:js"
17-
ninja -C emcc-build binaryen_js
18-
echo "emcc-tests: test:js"
18+
ninja -C emcc-build binaryen_js
19+
echo "emcc-tests: test"
1920
$SRCDIR/check.py --binaryen-bin=emcc-build/bin binaryenjs
20-
echo "emcc-tests: done:js"
21+
echo "emcc-tests: done"

scripts/test/binaryenjs.py

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -45,18 +45,18 @@ def make_js_test(input_js_file, binaryen_js):
4545
return outname
4646

4747

48-
def do_test_binaryen_js_with(which):
48+
def test_binaryen_js():
4949
if not (shared.MOZJS or shared.NODEJS):
5050
shared.fail_with_error('no vm to run binaryen.js tests')
5151

5252
node_has_wasm = shared.NODEJS and support.node_has_webassembly(shared.NODEJS)
53-
if not os.path.exists(which):
54-
shared.fail_with_error('no ' + which + ' build to test')
53+
if not os.path.exists(shared.BINARYEN_JS):
54+
shared.fail_with_error('no ' + shared.BINARYEN_JS + ' build to test')
5555

56-
print('\n[ checking binaryen.js testcases (' + which + ')... ]\n')
56+
print('\n[ checking binaryen.js testcases (' + shared.BINARYEN_JS + ')... ]\n')
5757

5858
for s in shared.get_tests(shared.get_test_dir('binaryen.js'), ['.js']):
59-
outname = make_js_test(s, which)
59+
outname = make_js_test(s, shared.BINARYEN_JS)
6060

6161
def test(cmd):
6262
if 'fatal' not in s:
@@ -110,11 +110,3 @@ def update(cmd):
110110
update([shared.NODEJS, outname])
111111
else:
112112
print('Skipping ' + s + ' because WebAssembly might not be supported')
113-
114-
115-
def test_binaryen_js():
116-
do_test_binaryen_js_with(shared.BINARYEN_JS)
117-
118-
119-
def test_binaryen_wasm():
120-
do_test_binaryen_js_with(shared.BINARYEN_WASM)

scripts/test/shared.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,6 @@ def is_exe(fpath):
212212
WASM_EMSCRIPTEN_FINALIZE = [os.path.join(options.binaryen_bin,
213213
'wasm-emscripten-finalize')]
214214
BINARYEN_JS = os.path.join(options.binaryen_bin, 'binaryen_js.js')
215-
BINARYEN_WASM = os.path.join(options.binaryen_bin, 'binaryen_wasm.js')
216215

217216

218217
def wrap_with_valgrind(cmd):

0 commit comments

Comments
 (0)