Skip to content

Enable WASM_BIGINT by default #19156

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

Closed
wants to merge 51 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
51 commits
Select commit Hold shift + click to select a range
ee34a80
yolo
kripken Apr 10, 2023
5b5e426
make it work
kripken Apr 10, 2023
520301b
fix
kripken Apr 10, 2023
85793ac
fix closure
kripken Apr 11, 2023
22393a4
Merge remote-tracking branch 'origin/main' into safari.bigint
kripken Apr 11, 2023
9fe22ab
fix wasm2js
kripken Apr 11, 2023
e255dad
fix test
kripken Apr 11, 2023
e1108d6
fix
kripken Apr 11, 2023
314cf8a
fix
kripken Apr 11, 2023
a52372d
cleanup
kripken Apr 11, 2023
c64adf2
more
kripken Apr 11, 2023
e7259ed
fix
kripken Apr 11, 2023
09ea912
fixflake8
kripken Apr 11, 2023
6699531
work
kripken Apr 11, 2023
d907b0e
fix
kripken Apr 12, 2023
af838e7
fix
kripken Apr 12, 2023
9aac743
fix
kripken Apr 12, 2023
5142300
work
kripken Apr 12, 2023
71ca03a
feedback
kripken Apr 12, 2023
d23687e
update test
kripken Apr 13, 2023
83165d1
fix
kripken Apr 13, 2023
0fb2458
try to fix test_dyncall_specific
kripken Apr 13, 2023
08c7170
Merge remote-tracking branch 'origin/main' into safari.bigint
kripken Apr 13, 2023
d1e4aec
fix old node version handling in common.py (maybe)
kripken Apr 13, 2023
2ae89c5
fix
kripken Apr 13, 2023
3f3c561
fix
kripken Apr 13, 2023
bf566aa
fix
kripken Apr 14, 2023
9459701
fix
kripken Apr 14, 2023
8dbb1ae
more test workarounds
kripken Apr 14, 2023
b107f47
Merge remote-tracking branch 'origin/main' into safari.bigint
kripken Apr 14, 2023
6b71f83
Merge remote-tracking branch 'origin/main' into safari.bigint
kripken Apr 17, 2023
6973fea
Merge remote-tracking branch 'origin/main' into safari.bigint
kripken Apr 17, 2023
560120a
fix
kripken Apr 17, 2023
84836b2
fix
kripken Apr 17, 2023
356a840
Explain safari-bigint issue and set version to 15
kripken Apr 17, 2023
00aab91
Merge remote-tracking branch 'origin/main' into safari.bigint
kripken Jun 21, 2023
0ca6930
Merge remote-tracking branch 'origin/main' into safari.bigint
kripken Jun 21, 2023
5985cdd
fix
kripken Jun 21, 2023
8566684
fix
kripken Jun 21, 2023
c35d241
Merge remote-tracking branch 'origin/main' into safari.bigint
kripken Jun 22, 2023
b537cc7
fix
kripken Jun 22, 2023
339b323
fix
kripken Jun 22, 2023
400f01e
fix
kripken Jun 22, 2023
62366ca
clean
kripken Jun 22, 2023
2f8f82c
waka
kripken Jun 22, 2023
8eab1f5
fix
kripken Jun 22, 2023
c93f72f
comment
kripken Jun 22, 2023
568ee40
fix
kripken Jun 22, 2023
fe12dc0
flip
kripken Jun 22, 2023
a42cb9e
remove =1s
kripken Jun 22, 2023
4998ef6
merge [ci skip]
kripken Jun 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ See docs/process.md for more on how version tagging works.

3.1.42 (in development)
-----------------------
- Enable WASM_BIGINT support by default. (#19156)
- The default minimum Node version of Emscripten output was bumped from 10.19 to
16.0. To run the output JS in an older version of node, you can use e.g.
`-sMIN_NODE_VERSION=101900` which will apply the previous minimum version of
Expand Down
5 changes: 5 additions & 0 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1895,6 +1895,11 @@ def phase_linker_setup(options, state, newargs):
if settings.LZ4:
settings.EXPORTED_RUNTIME_METHODS += ['LZ4']

# WASM_BIGINT defaults to 1 when VMs support it, and when not using wasm2js
default_setting('WASM_BIGINT',
feature_matrix.caniuse(feature_matrix.Feature.JS_BIGINT_INTEGRATION) and
not settings.WASM2JS)

if settings.PURE_WASI:
settings.STANDALONE_WASM = 1
settings.WASM_BIGINT = 1
Expand Down
7 changes: 7 additions & 0 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -3157,8 +3157,15 @@ mergeInto(LibraryManager.library, {
assert(('dynCall_' + sig) in Module, `bad function pointer type - dynCall function not found for sig '${sig}'`);
#endif
if (args && args.length) {
#if WASM_BIGINT
// j (64-bit integer) is fine, and is implemented as a BigInt. Without
// legalization, the number of parameters should match (j is not expanded
// into two i's).
assert(args.length === sig.length - 1);
#else
// j (64-bit integer) must be passed in as two numbers [low 32, high 32].
assert(args.length === sig.substring(1).replace(/j/g, '--').length);
#endif
} else {
assert(sig.length == 1);
}
Expand Down
6 changes: 6 additions & 0 deletions src/polyfill/bigint64array.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ if (typeof globalThis.BigInt64Array === "undefined") {
}

function createBigIntArrayShim(partsToBigInt) {
/**
* Closure thinks .set is not defined on Proxy objects for some reason. The
* error is on the line with proxy.set but we can only apply the
* suppression at the function level here.
* @suppress {missingProperties}
*/
function createBigInt64Array(array) {
if (typeof array === "number") {
array = new Uint32Array(2 * array);
Expand Down
5 changes: 2 additions & 3 deletions src/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -1381,10 +1381,9 @@ var DYNCALLS = false;

// WebAssembly integration with JavaScript BigInt. When enabled we don't need to
// legalize i64s into pairs of i32s, as the wasm VM will use a BigInt where an
// i64 is used. If WASM_BIGINT is present, the default minimum supported browser
// versions will be increased to the min version that supports BigInt.
// i64 is used.
// [link]
var WASM_BIGINT = false;
var WASM_BIGINT = true;

// WebAssembly defines a "producers section" which compilers and tools can
// annotate themselves in, and LLVM emits this by default.
Expand Down
13 changes: 6 additions & 7 deletions test/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,14 +290,13 @@ def metafunc(self, with_bigint):
self.skipTest('wasm2js does not support WASM_BIGINT')
if self.get_setting('WASM_BIGINT') is not None:
self.skipTest('redundant in bigint test config')
self.set_setting('WASM_BIGINT')
self.node_args += shared.node_bigint_flags()
f(self)
else:
self.set_setting('WASM_BIGINT', '0')
f(self)

metafunc._parameterize = {'': (False,),
'bigint': (True,)}
metafunc._parameterize = {'no_bigint': (False,),
'': (True,)}
return metafunc


Expand Down Expand Up @@ -951,9 +950,9 @@ def cleanup(line):
if len(line) > 2048:
# Sanity check that this is really the emscripten program/module on
# a single line.
assert line.startswith('var Module=typeof Module!="undefined"')
long_lines.append(line)
line = '<REPLACED ENTIRE PROGRAM ON SINGLE LINE>'
if line.startswith('var Module=typeof Module!="undefined"'):
long_lines.append(line)
line = '<REPLACED ENTIRE PROGRAM ON SINGLE LINE>'
return line

lines = [cleanup(l) for l in lines]
Expand Down
62 changes: 50 additions & 12 deletions test/core/dyncall_specific.c
Original file line number Diff line number Diff line change
Expand Up @@ -5,46 +5,84 @@
* found in the LICENSE file.
*/

#include <assert.h>
#include <emscripten.h>
#include <stdint.h>
#include <stdio.h>

int waka(int w, long long xy, int z) {
#ifdef WASM_BIGINT
// With WASM_BIGINT things are straightforward: the 64-bit value just arrives
// with the expected value of 4.
assert(w == 1);
assert(xy == 4);
assert(z == 9);
#else
// xy should be 0xffff_ffff_0000_0004
int x = (int) xy; // should be 4
int y = xy >> 32; // should be -1
EM_ASM({
out('received ' + [$0, $1, $2, $3] + '.');
}, w, x, y, z);
int x = (int) xy;
int y = xy >> 32;
assert(w == 1);
assert(x == 4);
assert(y == -1);
assert(z == 9);
#endif
return 42;
}

EM_JS_DEPS(main, "$dynCall");

int main() {
EM_ASM({
// Note that these would need to use BigInts if the file were built with
// -sWASM_BIGINT

#ifdef WASM_BIGINT

#if DIRECT
console.log('Received ' + dynCall_iiji($0, 1, BigInt(4), 9));
return;
#endif
#if DYNAMIC_SIG
console.log('Received ' + dynCall('iiji', $0, [1, BigInt(4), 9]));
return;
#endif
#if EXPORTED
console.log('Received ' + Module['dynCall_iiji']($0, 1, BigInt(4), 9));
return;
#endif
#if EXPORTED_DYNAMIC_SIG
console.log('Received ' + Module['dynCall']('iiji', $0, [1, BigInt(4), 9]));
return;
#endif
#if FROM_OUTSIDE
eval("console.log('Received ' + Module['dynCall_iiji'](" + $0 + ", 1, BigInt(4), 9))");
return;
#endif

#else // WASM_BIGINT

#if DIRECT
console.log('Received ' + dynCall_iiji($0, 1, 4, 0xffffffff, 9));
console.log('Received ' + dynCall_iiji($0, 1, BigInt(4), 9));
return;
#endif
#if DYNAMIC_SIG
console.log('Received ' + dynCall('iiji', $0, [1, 4, 0xffffffff, 9]));
console.log('Received ' + dynCall('iiji', $0, [1, BigInt(4), 9]));
return;
#endif
#if EXPORTED
console.log('Received ' + Module['dynCall_iiji']($0, 1, 4, 0xffffffff, 9));
console.log('Received ' + Module['dynCall_iiji']($0, 1, BigInt(4), 9));
return;
#endif
#if EXPORTED_DYNAMIC_SIG
console.log('Received ' + Module['dynCall']('iiji', $0, [1, 4, 0xffffffff, 9]));
console.log('Received ' + Module['dynCall']('iiji', $0, [1, BigInt(4), 9]));
return;
#endif
#if FROM_OUTSIDE
eval("console.log('Received ' + Module['dynCall_iiji'](" + $0 + ", 1, 4, 0xffffffff, 9))");
eval("console.log('Received ' + Module['dynCall_iiji'](" + $0 + ", 1, BigInt(4), 9))");
return;
#endif

#endif

// We should have run the test and returned before we get here.
throw "no test mode";
}, &waka);
}
Expand Down
3 changes: 1 addition & 2 deletions test/core/dyncall_specific.out
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
received 1,4,-1,9.
Received 42
Received 42
1 change: 0 additions & 1 deletion test/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@
'wasmfs',
'wasm64',
'wasm64l',
'bigint',
]


Expand Down
5 changes: 1 addition & 4 deletions test/test_browser.py
Original file line number Diff line number Diff line change
Expand Up @@ -4074,7 +4074,7 @@ def test_pthread_iostream(self):

@requires_threads
def test_pthread_unistd_io_bigint(self):
self.btest_exit(test_file('unistd/io.c'), args=['-pthread', '-sPROXY_TO_PTHREAD', '-sWASM_BIGINT'])
self.btest_exit(test_file('unistd/io.c'), args=['-pthread', '-sPROXY_TO_PTHREAD'])

# Test that the main thread is able to use pthread_set/getspecific.
@also_with_wasm2js
Expand Down Expand Up @@ -5456,9 +5456,6 @@ def test_dlmalloc_3GB(self):
'main_thread': (['-sPTHREAD_POOL_SIZE=5'],),
# using proxy_to_pthread also works, of course
'proxy_to_pthread': (['-sPROXY_TO_PTHREAD', '-sINITIAL_MEMORY=32MB', '-DPROXYING'],),
# using BigInt support affects the ABI, and should not break things. (this
# could be tested on either thread; do the main thread for simplicity)
'bigint': (['-sPTHREAD_POOL_SIZE=5', '-sWASM_BIGINT'],),
})
@requires_threads
def test_wasmfs_fetch_backend(self, args):
Expand Down
37 changes: 13 additions & 24 deletions test/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,15 +206,10 @@ def metafunc(self, standalone):
if not can_do_standalone(self, impure):
self.skipTest('Test configuration is not compatible with STANDALONE_WASM')
self.set_setting('STANDALONE_WASM')
# we will not legalize the JS ffi interface, so we must use BigInt
# support in order for JS to have a chance to run this without trapping
# when it sees an i64 on the ffi.
self.set_setting('WASM_BIGINT')
self.emcc_args.append('-Wno-unused-command-line-argument')
# if we are impure, disallow all wasm engines
if impure:
self.wasm_engines = []
self.node_args += shared.node_bigint_flags()
func(self)

metafunc._parameterize = {'': (False,),
Expand Down Expand Up @@ -536,9 +531,7 @@ def test_i64_varargs(self):
@no_wasm2js('wasm_bigint')
@requires_node
def test_i64_invoke_bigint(self):
self.set_setting('WASM_BIGINT')
self.emcc_args += ['-fexceptions']
self.node_args += shared.node_bigint_flags()
self.do_core_test('test_i64_invoke_bigint.cpp')

def test_vararg_copy(self):
Expand Down Expand Up @@ -2309,11 +2302,9 @@ def test_em_js(self, args, force_c):

@no_wasm2js('WASM_BIGINT is not compatible with wasm2js')
def test_em_js_i64(self):
err = self.expect_fail([EMCC, '-Werror', test_file('core/test_em_js_i64.c')])
err = self.expect_fail([EMCC, '-sWASM_BIGINT=0', '-Werror', test_file('core/test_em_js_i64.c')])
self.assertContained('emcc: error: using 64-bit arguments in EM_JS function without WASM_BIGINT is not yet fully supported: `foo`', err)

self.set_setting('WASM_BIGINT')
self.node_args += shared.node_bigint_flags()
self.do_core_test('test_em_js_i64.c')

def test_em_js_address_taken(self):
Expand Down Expand Up @@ -4251,7 +4242,6 @@ def test_dylink_basics_no_modify(self):
self.skipTest('no modify mode only works with non-optimizing builds')
if self.get_setting('MEMORY64') == 2:
self.skipTest('MEMORY64=2 always requires module re-writing')
self.set_setting('WASM_BIGINT')
self.set_setting('ERROR_ON_WASM_CHANGES_AFTER_LINK')
self.do_basic_dylink_test()

Expand Down Expand Up @@ -7204,30 +7194,37 @@ def test_EXPORTED_RUNTIME_METHODS(self):
self.set_setting('EXPORTED_RUNTIME_METHODS', ['dynCall', 'addFunction', 'lengthBytesUTF8', 'getTempRet0', 'setTempRet0'])
self.do_core_test('EXPORTED_RUNTIME_METHODS.c')

@no_wasm2js('depends on bigint support directly, which wasm2js disallows')
@parameterized({
'': [],
'minimal_runtime': ['-sMINIMAL_RUNTIME=1']
})
def test_dyncall_specific(self, *args):
if self.get_setting('WASM_BIGINT') or self.get_setting('MEMORY64'):
self.skipTest('not compatible with WASM_BIGINT')
if self.get_setting('MEMORY64'):
self.skipTest('not compatible with MEMORY64')
if self.get_setting('WASM_BIGINT'):
# define DYNCALLS because this test does test calling them directly, and
# in WASM_BIGINT mode we do not enable them by default (since we can do
# more without them - we don't need to legalize)
args = list(args) + ['-sDYNCALLS', '-DWASM_BIGINT']
cases = [
('DIRECT', []),
('DYNAMIC_SIG', ['-sDYNCALLS=1']),
('DYNAMIC_SIG', ['-sDYNCALLS']),
]
if '-sMINIMAL_RUNTIME=1' in args:
self.emcc_args += ['--pre-js', test_file('minimal_runtime_exit_handling.js')]
else:
cases += [
('EXPORTED', []),
('EXPORTED_DYNAMIC_SIG', ['-sDYNCALLS=1', '-sEXPORTED_RUNTIME_METHODS=dynCall']),
('EXPORTED_DYNAMIC_SIG', ['-sDYNCALLS', '-sEXPORTED_RUNTIME_METHODS=dynCall']),
('FROM_OUTSIDE', ['-sEXPORTED_RUNTIME_METHODS=dynCall_iiji'])
]

for which, extra_args in cases:
print(str(args) + ' ' + which)
self.do_core_test('dyncall_specific.c', emcc_args=['-D' + which] + list(args) + extra_args)

@no_wasm2js('depends on bigint support directly, which wasm2js disallows')
@also_with_wasm_bigint
def test_getValue_setValue(self):
# these used to be exported, but no longer are by default
Expand Down Expand Up @@ -7682,16 +7679,12 @@ def test_embind_dynamic_initialization(self):

@no_wasm2js('wasm_bigint')
def test_embind_i64_val(self):
self.set_setting('WASM_BIGINT')
self.emcc_args += ['-lembind']
self.node_args += shared.node_bigint_flags()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are there any remaining callers of node_bigint_flags?

Copy link
Member Author

Choose a reason for hiding this comment

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

This one is not needed, but there are still two uses after it, in emcc.py and gen_struct_info.py which I am not sure about.

Copy link
Member Author

Choose a reason for hiding this comment

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

To remove it in them, we might need to add node version checks...

self.do_run_in_out_file_test('embind/test_i64_val.cpp', assert_identical=True)

@no_wasm2js('wasm_bigint')
def test_embind_i64_binding(self):
self.set_setting('WASM_BIGINT')
self.emcc_args += ['-lembind']
self.node_args += shared.node_bigint_flags()
self.do_run_in_out_file_test('embind/test_i64_binding.cpp', assert_identical=True)

def test_embind_no_rtti(self):
Expand Down Expand Up @@ -9832,8 +9825,7 @@ def setUp(self):
settings={'MEMORY64': 1}, require_wasm64=True, require_v8=True)
# MEMORY64=2, or "lowered"
wasm64l = make_run('wasm64l', emcc_args=['-Wno-experimental', '--profiling-funcs'],
settings={'MEMORY64': 2},
node_args=shared.node_bigint_flags())
settings={'MEMORY64': 2})

lto0 = make_run('lto0', emcc_args=['-flto', '-O0'])
lto1 = make_run('lto1', emcc_args=['-flto', '-O1'])
Expand Down Expand Up @@ -9869,9 +9861,6 @@ def setUp(self):
core2s = make_run('core2s', emcc_args=['-O2'], settings={'SAFE_HEAP': 1})
core2ss = make_run('core2ss', emcc_args=['-O2'], settings={'STACK_OVERFLOW_CHECK': 2})

bigint = make_run('bigint', emcc_args=['--profiling-funcs'], settings={'WASM_BIGINT': 1},
node_args=shared.node_bigint_flags())

# Add DEFAULT_TO_CXX=0
strict = make_run('strict', emcc_args=[], settings={'STRICT': 1})

Expand Down
Loading