Skip to content

Simplify EM_ASM handling #11972

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 11 commits into from
Aug 21, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
51 changes: 4 additions & 47 deletions emscripten.py
Original file line number Diff line number Diff line change
Expand Up @@ -1651,14 +1651,13 @@ def emscript(infile, outfile, memfile, temp_files, DEBUG):

report_missing_symbols(set([asmjs_mangle(f) for f in exports]), pre)

asm_consts, asm_const_funcs = create_asm_consts_wasm(forwarded_json, metadata)
asm_consts = create_asm_consts_wasm(forwarded_json, metadata)
em_js_funcs = create_em_js(forwarded_json, metadata)
asm_const_pairs = ['%s: %s' % (key, value) for key, value in asm_consts]
asm_const_map = 'var ASM_CONSTS = {\n ' + ', \n '.join(asm_const_pairs) + '\n};\n'
pre = pre.replace(
'// === Body ===',
('// === Body ===\n\n' + asm_const_map +
asstr('\n'.join(asm_const_funcs)) +
'\n'.join(em_js_funcs) + '\n'))
pre = apply_table(pre)
outfile.write(pre)
Expand Down Expand Up @@ -1743,6 +1742,7 @@ def finalize_wasm(temp_files, infile, outfile, memfile, DEBUG):
args.append('--pass-arg=legalize-js-interface-export-originals')
if shared.Settings.DEBUG_LEVEL >= 3:
args.append('--dwarf')
args.append('--minimize-wasm-changes')
stdout = building.run_binaryen_command('wasm-emscripten-finalize',
infile=base_wasm,
outfile=wasm,
Expand All @@ -1764,7 +1764,6 @@ def finalize_wasm(temp_files, infile, outfile, memfile, DEBUG):

def create_asm_consts_wasm(forwarded_json, metadata):
asm_consts = {}
all_sigs = []
for k, v in metadata['asmConsts'].items():
const, sigs, call_types = v
const = asstr(const)
Expand All @@ -1779,47 +1778,9 @@ def create_asm_consts_wasm(forwarded_json, metadata):
args.append('$' + str(i))
const = 'function(' + ', '.join(args) + ') {' + const + '}'
asm_consts[int(k)] = const
for sig, call_type in zip(sigs, call_types):
all_sigs.append((sig, call_type))

asm_const_funcs = []

for sig, call_type in set(all_sigs):
const_name = '_emscripten_asm_const_' + call_type + sig
forwarded_json['Functions']['libraryFunctions'][const_name] = 1

preamble = ''
if shared.Settings.USE_PTHREADS:
sync_proxy = call_type == 'sync_on_main_thread_'
async_proxy = call_type == 'async_on_main_thread_'
proxied = sync_proxy or async_proxy
if proxied:
# In proxied function calls, positive integers 1, 2, 3, ... denote pointers
# to regular C compiled functions. Negative integers -1, -2, -3, ... denote
# indices to EM_ASM() blocks, so remap the EM_ASM() indices from 0, 1, 2,
# ... over to the negative integers starting at -1.
preamble += ('\n if (ENVIRONMENT_IS_PTHREAD) { ' +
proxy_debug_print(sync_proxy) +
'return _emscripten_proxy_to_main_thread_js.apply(null, ' +
'[-1 - code, ' + str(int(sync_proxy)) + '].concat(args)) }')

if shared.Settings.RELOCATABLE:
preamble += '\n code -= %s;\n' % shared.Settings.GLOBAL_BASE

# EM_ASM functions are variadic, receiving the actual arguments as a buffer
# in memory. the last parameter (argBuf) points to that data. We need to
# alwayd un-variadify that, as in the async case this is a stack allocation
# that LLVM made, which may go away before the main thread gets the message.
# the readAsmConstArgs helper does so.
asm_const_funcs.append(r'''
function %s(code, sigPtr, argbuf) {
var args = readAsmConstArgs(sigPtr, argbuf);
%s
return ASM_CONSTS[code].apply(null, args);
}''' % (const_name, preamble))
asm_consts = [(key, value) for key, value in asm_consts.items()]
asm_consts.sort()
return asm_consts, asm_const_funcs
return asm_consts


def create_em_js(forwarded_json, metadata):
Expand Down Expand Up @@ -1973,13 +1934,9 @@ def create_sending_wasm(invoke_funcs, forwarded_json, metadata):
if shared.Settings.SAFE_HEAP:
basic_funcs += ['segfault', 'alignfault']

em_asm_sigs = [zip(sigs, call_types) for _, sigs, call_types in metadata['asmConsts'].values()]
# flatten em_asm_sigs
em_asm_sigs = [sig for sigs in em_asm_sigs for sig in sigs]
em_asm_funcs = ['_emscripten_asm_const_' + call_type + sig for sig, call_type in em_asm_sigs]
em_js_funcs = list(metadata['emJsFuncs'].keys())
declared_items = ['_' + item for item in metadata['declares']]
send_items = set(basic_funcs + invoke_funcs + em_asm_funcs + em_js_funcs + declared_items)
send_items = set(basic_funcs + invoke_funcs + em_js_funcs + declared_items)

def fix_import_name(g):
if g.startswith('Math_'):
Expand Down
49 changes: 43 additions & 6 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -3981,6 +3981,49 @@ LibraryManager.library = {
return readAsmConstArgsArray;
},

emscripten_asm_const_int__sig: 'iiii',
emscripten_asm_const_int: function(code, sigPtr, argbuf) {
#if RELOCATABLE
code -= {{{ GLOBAL_BASE }}};
#endif
var args = readAsmConstArgs(sigPtr, argbuf);
return ASM_CONSTS[code].apply(null, args);
},
emscripten_asm_const_double: 'emscripten_asm_const_int',
$mainThreadEM_ASM: function(code, sigPtr, argbuf, sync) {
#if RELOCATABLE
code -= {{{ GLOBAL_BASE }}};
#endif
var args = readAsmConstArgs(sigPtr, argbuf);
#if USE_PTHREADS
if (ENVIRONMENT_IS_PTHREAD) {
// EM_ASM functions are variadic, receiving the actual arguments as a buffer
// in memory. the last parameter (argBuf) points to that data. We need to
// always un-variadify that, *before proxying*, as in the async case this
// is a stack allocation that LLVM made, which may go away before the main
// thread gets the message. For that reason we handle proxying *after* the
// call to readAsmConstArgs, and therefore we do that manually here instead
// of using __proxy. (And dor simplicity, do the same in the sync
// case as well, even though it's not strictly necessary, to keep the two
// code paths as similar as possible on both sides.)
// -1 - code is the encoding of a proxied EM_ASM, as a negative number
// (positive numbers are non-EM_ASM calls).
return _emscripten_proxy_to_main_thread_js.apply(null, [-1 - code, sync].concat(args));
}
#endif
return ASM_CONSTS[code].apply(null, args);
},
emscripten_asm_const_int_sync_on_main_thread__deps: ['$mainThreadEM_ASM'],
emscripten_asm_const_int_sync_on_main_thread__sig: 'iiii',
emscripten_asm_const_int_sync_on_main_thread: function(code, sigPtr, argbuf) {
return mainThreadEM_ASM(code, sigPtr, argbuf, 1);
},
emscripten_asm_const_double_sync_on_main_thread: 'emscripten_asm_const_int_sync_on_main_thread',
emscripten_asm_const_async_on_main_thread__deps: ['$mainThreadEM_ASM'],
emscripten_asm_const_async_on_main_thread: function(code, sigPtr, argbuf) {
return mainThreadEM_ASM(code, sigPtr, argbuf, 0);
},

#if !DECLARE_ASM_MODULE_EXPORTS
// When DECLARE_ASM_MODULE_EXPORTS is not set we export native symbols
// at runtime rather than statically in JS code.
Expand Down Expand Up @@ -4186,12 +4229,6 @@ LibraryManager.library = {
llvm_dbg_value: function() {},
llvm_debugtrap: function() {},
llvm_ctlz_i32: function() {},
emscripten_asm_const: function() {},
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function no longer exists? I guess it was never needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it may have made sense in some old approach to EM_ASM in fastcomp perhaps.

emscripten_asm_const_int: function() {},
emscripten_asm_const_double: function() {},
emscripten_asm_const_int_sync_on_main_thread: function() {},
emscripten_asm_const_double_sync_on_main_thread: function() {},
emscripten_asm_const_async_on_main_thread: function() {},

__handle_stack_overflow: function() {
abort('stack overflow')
Expand Down
8 changes: 8 additions & 0 deletions tests/browser/test_em_asm_blocking.cpp
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#include <assert.h>
#include <emscripten.h>
#include <stdio.h>
#include <thread>
Expand All @@ -7,8 +8,15 @@ std::atomic<int> ret;
void foo() {
int len = MAIN_THREAD_EM_ASM_INT({
var elem = document.getElementById('elem');
window.almost_PI = 3.14159;
return elem.innerText.length;
});
double almost_PI = MAIN_THREAD_EM_ASM_DOUBLE({
// read a double from the main thread
return window.almost_PI;
});
printf("almost PI: %f\n", almost_PI);
assert(fabs(almost_PI - 3.14159) < 0.001);
Copy link
Collaborator

Choose a reason for hiding this comment

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

What the changes to the test code?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just some added testing. We tested MAIN_THREAD_EM_ASM_INT but not _DOUBLE.

atomic_store(&ret, len);
}

Expand Down