From 9684867d7289f82c98d5394f34a4327d362fdbaf Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Sat, 22 Aug 2020 10:24:44 -0700 Subject: [PATCH 1/2] Use direct table access over dynCall where possible This change starts the transition away from dynCall and towards direct use of the wasm table. In particular this change replaces the use of dynCall in invoke_xx functions where possible. dynCall is still needed for functions with i64 in thier signature when WASM_BIGINT is not enabled. When WASM_BIGING is enabled this change removes all use of dynCall by invoke_xx functions. --- src/preamble.js | 1 + src/support.js | 35 +++++++++++-------- ...bcxxabi_message_O3_STANDALONE_WASM.exports | 1 + ...LLOW_MEMORY_GROWTH_STANDALONE_WASM.exports | 1 + .../metadce/mem_O3_STANDALONE_WASM.exports | 1 + .../mem_no_argv_O3_STANDALONE_WASM.exports | 1 + ...em_no_argv_O3_STANDALONE_WASM_flto.exports | 1 + ...no_main_O3_STANDALONE_WASM_noentry.exports | 1 + tests/test_other.py | 2 +- tools/building.py | 13 ++++++- tools/shared.py | 29 +++++++++------ 11 files changed, 58 insertions(+), 28 deletions(-) diff --git a/src/preamble.js b/src/preamble.js index ed1d23a2b3b67..2066a2068dfb0 100644 --- a/src/preamble.js +++ b/src/preamble.js @@ -896,6 +896,7 @@ function createWasm() { // then exported. // TODO: do not create a Memory earlier in JS wasmMemory = exports['memory']; + wasmTable = exports['__indirect_function_table']; updateGlobalBufferAndViews(wasmMemory.buffer); #if ASSERTIONS writeStackCookie(); diff --git a/src/support.js b/src/support.js index c7c3365204807..6d2f517b12c8f 100644 --- a/src/support.js +++ b/src/support.js @@ -301,7 +301,7 @@ function createInvokeFunction(sig) { return function() { var sp = stackSave(); try { - return Module['dynCall_' + sig].apply(null, arguments); + return dynCall(sig, arguments[0], Array.prototype.slice.call(arguments, 1)); } catch(e) { stackRestore(sp); if (e !== e+0 && e !== 'longjmp') throw e; @@ -602,24 +602,29 @@ function makeBigInt(low, high, unsigned) { /** @param {Array=} args */ function dynCall(sig, ptr, args) { - if (args && args.length) { -#if ASSERTIONS - // 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 -#if ASSERTIONS - assert(('dynCall_' + sig) in Module, 'bad function pointer type - no table for sig \'' + sig + '\''); -#endif - return Module['dynCall_' + sig].apply(null, [ptr].concat(args)); - } else { -#if ASSERTIONS - assert(sig.length == 1); -#endif +#if !WASM_BIGINT + // Without WASM_BIGINT support we cannot directly call function with i64 as + // part of thier signature, so we rely the dynCall functions generated by + // wasm-emscripten-finalize + if (sig.indexOf('j') != -1) { #if ASSERTIONS assert(('dynCall_' + sig) in Module, 'bad function pointer type - no table for sig \'' + sig + '\''); + if (args && args.length) { + // 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); + } else { + assert(sig.length == 1); + } #endif - return Module['dynCall_' + sig].call(null, ptr); + if (args && args.length) { + return Module['dynCall_' + sig].apply(null, [ptr].concat(args)); + } else { + return Module['dynCall_' + sig].call(null, ptr); + } } +#endif + + return wasmTable.get(ptr).apply(null, args) } var tempRet0 = 0; diff --git a/tests/other/metadce/libcxxabi_message_O3_STANDALONE_WASM.exports b/tests/other/metadce/libcxxabi_message_O3_STANDALONE_WASM.exports index 5890ab2140468..92d336c44efa9 100644 --- a/tests/other/metadce/libcxxabi_message_O3_STANDALONE_WASM.exports +++ b/tests/other/metadce/libcxxabi_message_O3_STANDALONE_WASM.exports @@ -1,2 +1,3 @@ +__indirect_function_table _start memory diff --git a/tests/other/metadce/mem_O3_ALLOW_MEMORY_GROWTH_STANDALONE_WASM.exports b/tests/other/metadce/mem_O3_ALLOW_MEMORY_GROWTH_STANDALONE_WASM.exports index 5890ab2140468..92d336c44efa9 100644 --- a/tests/other/metadce/mem_O3_ALLOW_MEMORY_GROWTH_STANDALONE_WASM.exports +++ b/tests/other/metadce/mem_O3_ALLOW_MEMORY_GROWTH_STANDALONE_WASM.exports @@ -1,2 +1,3 @@ +__indirect_function_table _start memory diff --git a/tests/other/metadce/mem_O3_STANDALONE_WASM.exports b/tests/other/metadce/mem_O3_STANDALONE_WASM.exports index 5890ab2140468..92d336c44efa9 100644 --- a/tests/other/metadce/mem_O3_STANDALONE_WASM.exports +++ b/tests/other/metadce/mem_O3_STANDALONE_WASM.exports @@ -1,2 +1,3 @@ +__indirect_function_table _start memory diff --git a/tests/other/metadce/mem_no_argv_O3_STANDALONE_WASM.exports b/tests/other/metadce/mem_no_argv_O3_STANDALONE_WASM.exports index 5890ab2140468..92d336c44efa9 100644 --- a/tests/other/metadce/mem_no_argv_O3_STANDALONE_WASM.exports +++ b/tests/other/metadce/mem_no_argv_O3_STANDALONE_WASM.exports @@ -1,2 +1,3 @@ +__indirect_function_table _start memory diff --git a/tests/other/metadce/mem_no_argv_O3_STANDALONE_WASM_flto.exports b/tests/other/metadce/mem_no_argv_O3_STANDALONE_WASM_flto.exports index 5890ab2140468..92d336c44efa9 100644 --- a/tests/other/metadce/mem_no_argv_O3_STANDALONE_WASM_flto.exports +++ b/tests/other/metadce/mem_no_argv_O3_STANDALONE_WASM_flto.exports @@ -1,2 +1,3 @@ +__indirect_function_table _start memory diff --git a/tests/other/metadce/mem_no_main_O3_STANDALONE_WASM_noentry.exports b/tests/other/metadce/mem_no_main_O3_STANDALONE_WASM_noentry.exports index fd0ebab924f79..f6bd3d591ceba 100644 --- a/tests/other/metadce/mem_no_main_O3_STANDALONE_WASM_noentry.exports +++ b/tests/other/metadce/mem_no_main_O3_STANDALONE_WASM_noentry.exports @@ -1,3 +1,4 @@ +__indirect_function_table _initialize foo memory diff --git a/tests/test_other.py b/tests/test_other.py index 871dc553edb2e..f47a9d70269da 100644 --- a/tests/test_other.py +++ b/tests/test_other.py @@ -7111,7 +7111,7 @@ def test_metadce_mem(self, filename, *args): [], [], 128), # noqa # argc/argv support code etc. is in the wasm 'O3_standalone': ('libcxxabi_message.cpp', ['-O3', '-s', 'STANDALONE_WASM'], - [], [], 198), # noqa + [], [], 242), # noqa }) def test_metadce_libcxxabi_message(self, filename, *args): self.run_metadce_test(filename, *args) diff --git a/tools/building.py b/tools/building.py index b2f2d6dc50200..04162b71094a5 100644 --- a/tools/building.py +++ b/tools/building.py @@ -484,6 +484,8 @@ def lld_flags_for_executable(external_symbol_list): if not Settings.STANDALONE_WASM: cmd.append('--import-memory') cmd.append('--import-table') + else: + cmd.append('--export-table') if Settings.USE_PTHREADS: cmd.append('--shared-memory') @@ -1136,7 +1138,10 @@ def add_to_path(dirname): logger.error(proc.stderr) # print list of errors (possibly long wall of text if input was minified) # Exit and print final hint to get clearer output - exit_with_error('closure compiler failed (rc: %d.%s)', proc.returncode, '' if pretty else ' the error message may be clearer with -g1 and EMCC_DEBUG=2 set') + msg = 'closure compiler failed (rc: %d): %s' % (proc.returncode, shared.shlex_join(cmd)) + if not pretty: + msg += ' the error message may be clearer with -g1 and EMCC_DEBUG=2 set' + exit_with_error(msg) if len(proc.stderr.strip()) > 0 and Settings.CLOSURE_WARNINGS != 'quiet': # print list of warnings (possibly long wall of text if input was minified) @@ -1227,6 +1232,12 @@ def metadce(js_file, wasm_file, minify_whitespace, debug_info): 'reaches': [], 'root': True }) + graph.append({ + 'export': '__indirect_function_table', + 'name': 'emcc$export$__indirect_function_table', + 'reaches': [], + 'root': True + }) # fix wasi imports TODO: support wasm stable with an option? WASI_IMPORTS = set([ 'environ_get', diff --git a/tools/shared.py b/tools/shared.py index b1ecf38aff9f5..6fcee4a70e26b 100644 --- a/tools/shared.py +++ b/tools/shared.py @@ -1209,28 +1209,34 @@ def make_jscall(sig): return ret @staticmethod - def make_dynCall(sig): - # Optimize dynCall accesses in the case when not building with dynamic - # linking enabled. - if not Settings.MAIN_MODULE and not Settings.SIDE_MODULE: - return 'dynCall_' + sig + def make_dynCall(sig, args): + # wasm2c and asyncify are yet compatible with direct wasm table calls + if Settings.ASYNCIFY or Settings.WASM2C or not JS.is_legal_sig(sig): + args = ','.join(args) + if not Settings.MAIN_MODULE and not Settings.SIDE_MODULE: + # Optimize dynCall accesses in the case when not building with dynamic + # linking enabled. + return 'dynCall_%s(%s)' % (sig, args) + else: + return 'Module["dynCall_%s"](%s)' % (sig, args) else: - return 'Module["dynCall_' + sig + '"]' + return 'wasmTable.get(%s)(%s)' % (args[0], ','.join(args[1:])) @staticmethod def make_invoke(sig, named=True): legal_sig = JS.legalize_sig(sig) # TODO: do this in extcall, jscall? - args = ','.join(['a' + str(i) for i in range(1, len(legal_sig))]) - args = 'index' + (',' if args else '') + args + args = ['index'] + ['a' + str(i) for i in range(1, len(legal_sig))] ret = 'return ' if sig[0] != 'v' else '' - body = '%s%s(%s);' % (ret, JS.make_dynCall(sig), args) + body = '%s%s;' % (ret, JS.make_dynCall(sig, args)) # C++ exceptions are numbers, and longjmp is a string 'longjmp' if Settings.SUPPORT_LONGJMP: rethrow = "if (e !== e+0 && e !== 'longjmp') throw e;" else: rethrow = "if (e !== e+0) throw e;" - ret = '''function%s(%s) { + name = (' invoke_' + sig) if named else '' + ret = '''\ +function%s(%s) { var sp = stackSave(); try { %s @@ -1239,7 +1245,8 @@ def make_invoke(sig, named=True): %s _setThrew(1, 0); } -}''' % ((' invoke_' + sig) if named else '', args, body, rethrow) +}''' % (name, ','.join(args), body, rethrow) + return ret @staticmethod From 7369845ed6c8e6edcb80278eff59611651561d12 Mon Sep 17 00:00:00 2001 From: Sam Clegg Date: Tue, 25 Aug 2020 03:59:23 -0700 Subject: [PATCH 2/2] feedback --- src/support.js | 2 +- tools/shared.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/support.js b/src/support.js index 6d2f517b12c8f..267eb91abafee 100644 --- a/src/support.js +++ b/src/support.js @@ -604,7 +604,7 @@ function makeBigInt(low, high, unsigned) { function dynCall(sig, ptr, args) { #if !WASM_BIGINT // Without WASM_BIGINT support we cannot directly call function with i64 as - // part of thier signature, so we rely the dynCall functions generated by + // part of their signature, so we rely on the dynCall functions generated by // wasm-emscripten-finalize if (sig.indexOf('j') != -1) { #if ASSERTIONS diff --git a/tools/shared.py b/tools/shared.py index 6fcee4a70e26b..51373bdaf3c90 100644 --- a/tools/shared.py +++ b/tools/shared.py @@ -1210,7 +1210,7 @@ def make_jscall(sig): @staticmethod def make_dynCall(sig, args): - # wasm2c and asyncify are yet compatible with direct wasm table calls + # wasm2c and asyncify are not yet compatible with direct wasm table calls if Settings.ASYNCIFY or Settings.WASM2C or not JS.is_legal_sig(sig): args = ','.join(args) if not Settings.MAIN_MODULE and not Settings.SIDE_MODULE: