Skip to content

Part two of dynCall removal #12059

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 3 commits into from
Aug 31, 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
17 changes: 12 additions & 5 deletions emcc.py
Original file line number Diff line number Diff line change
Expand Up @@ -1267,6 +1267,12 @@ def check(input_file):
if shared.Settings.RELOCATABLE:
shared.Settings.ALLOW_TABLE_GROWTH = 1

if shared.Settings.ASYNCIFY:
# See: https://github.com/emscripten-core/emscripten/issues/12065
# See: https://github.com/emscripten-core/emscripten/issues/12066
shared.Settings.USE_LEGACY_DYNCALLS = 1
shared.Settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$getDynCaller']

# Reconfigure the cache now that settings have been applied. Some settings
# such as LTO and SIDE_MODULE/MAIN_MODULE effect which cache directory we use.
shared.reconfigure_cache()
Expand Down Expand Up @@ -1522,19 +1528,20 @@ def check(input_file):
'removeRunDependency',
]

if not shared.Settings.MINIMAL_RUNTIME or (shared.Settings.USE_PTHREADS or shared.Settings.EXIT_RUNTIME):
if not shared.Settings.MINIMAL_RUNTIME or shared.Settings.EXIT_RUNTIME:
# MINIMAL_RUNTIME only needs callRuntimeCallbacks in certain cases, but the normal runtime
# always does.
shared.Settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$callRuntimeCallbacks']
shared.Settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$callRuntimeCallbacks', '$dynCall']

if shared.Settings.USE_PTHREADS:
# memalign is used to ensure allocated thread stacks are aligned.
shared.Settings.EXPORTED_FUNCTIONS += ['_memalign', '_malloc']

# dynCall_ii is used to call pthread entry points in worker.js (as
# dynCall is used to call pthread entry points in worker.js (as
# metadce does not consider worker.js, which is external, we must
# consider it a user export, i.e., one which can never be removed).
building.user_requested_exports += ['dynCall_ii']
# consider it an export, i.e., one which can never be removed).
shared.Settings.DEFAULT_LIBRARY_FUNCS_TO_INCLUDE += ['$dynCall']
shared.Settings.EXPORTED_FUNCTIONS += ['dynCall']

if shared.Settings.MINIMAL_RUNTIME:
building.user_requested_exports += ['exit']
Expand Down
7 changes: 7 additions & 0 deletions emscripten.py
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,13 @@ def finalize_wasm(temp_files, infile, outfile, memfile, DEBUG):
args.append('-g')
if shared.Settings.WASM_BIGINT:
args.append('--bigint')

if not shared.Settings.USE_LEGACY_DYNCALLS:
if shared.Settings.WASM_BIGINT:
args.append('--no-dyncalls')
else:
args.append('--dyncalls-i64')

if shared.Settings.LEGALIZE_JS_FFI != 1:
args.append('--no-legalize-javascript-ffi')
if not shared.Settings.MEM_INIT_IN_WASM:
Expand Down
12 changes: 6 additions & 6 deletions src/Fetch.js
Original file line number Diff line number Diff line change
Expand Up @@ -454,28 +454,28 @@ function emscripten_start_fetch(fetch, successcb, errorcb, progresscb, readystat
#if FETCH_DEBUG
console.log('fetch: operation success. e: ' + e);
#endif
if (onsuccess) {{{ makeDynCall('vi') }}}(onsuccess, fetch);
if (onsuccess) {{{ makeDynCall('vi', 'onsuccess') }}}(fetch);
else if (successcb) successcb(fetch);
};

var reportProgress = function(fetch, xhr, e) {
if (onprogress) {{{ makeDynCall('vi') }}}(onprogress, fetch);
if (onprogress) {{{ makeDynCall('vi', 'onprogress') }}}(fetch);
else if (progresscb) progresscb(fetch);
};

var reportError = function(fetch, xhr, e) {
#if FETCH_DEBUG
console.error('fetch: operation failed: ' + e);
#endif
if (onerror) {{{ makeDynCall('vi') }}}(onerror, fetch);
if (onerror) {{{ makeDynCall('vi', 'onerror') }}}(fetch);
else if (errorcb) errorcb(fetch);
};

var reportReadyStateChange = function(fetch, xhr, e) {
#if FETCH_DEBUG
console.log('fetch: ready state change. e: ' + e);
#endif
if (onreadystatechange) {{{ makeDynCall('vi') }}}(onreadystatechange, fetch);
if (onreadystatechange) {{{ makeDynCall('vi', 'onreadystatechange') }}}(fetch);
else if (readystatechangecb) readystatechangecb(fetch);
};

Expand All @@ -495,14 +495,14 @@ function emscripten_start_fetch(fetch, successcb, errorcb, progresscb, readystat
#if FETCH_DEBUG
console.log('fetch: IndexedDB store succeeded.');
#endif
if (onsuccess) {{{ makeDynCall('vi') }}}(onsuccess, fetch);
if (onsuccess) {{{ makeDynCall('vi', 'onsuccess') }}}(fetch);
else if (successcb) successcb(fetch);
};
var storeError = function(fetch, xhr, e) {
#if FETCH_DEBUG
console.error('fetch: IndexedDB store failed.');
#endif
if (onsuccess) {{{ makeDynCall('vi') }}}(onsuccess, fetch);
if (onsuccess) {{{ makeDynCall('vi', 'onsuccess') }}}(fetch);
else if (successcb) successcb(fetch);
};
__emscripten_fetch_cache_data(Fetch.dbInstance, fetch, xhr.response, storeSuccess, storeError);
Expand Down
38 changes: 8 additions & 30 deletions src/embind/embind.js
Original file line number Diff line number Diff line change
Expand Up @@ -1062,42 +1062,20 @@ var LibraryEmbind = {
#endif
},

$embind__requireFunction__deps: ['$readLatin1String', '$throwBindingError'],
$embind__requireFunction__deps: ['$readLatin1String', '$throwBindingError', '$getDynCaller'],
$embind__requireFunction: function(signature, rawFunction) {
signature = readLatin1String(signature);

function makeDynCaller(dynCall) {
#if DYNAMIC_EXECUTION == 0
var argCache = [rawFunction];
return function() {
argCache.length = arguments.length + 1;
for (var i = 0; i < arguments.length; i++) {
argCache[i + 1] = arguments[i];
}
return dynCall.apply(null, argCache);
};
#else
var args = [];
for (var i = 1; i < signature.length; ++i) {
args.push('a' + i);
}

var name = 'dynCall_' + signature + '_' + rawFunction;
var body = 'return function ' + name + '(' + args.join(', ') + ') {\n';
body += ' return dynCall(rawFunction' + (args.length ? ', ' : '') + args.join(', ') + ');\n';
body += '};\n';

return (new Function('dynCall', 'rawFunction', body))(dynCall, rawFunction);
function makeDynCaller() {
#if !USE_LEGACY_DYNCALLS
if (signature.indexOf('j') == -1) {
return wasmTable.get(rawFunction);
}
#endif
return getDynCaller(signature, rawFunction);
}

#if MINIMAL_RUNTIME
var dc = asm['dynCall_' + signature];
#else
var dc = Module['dynCall_' + signature];
#endif
var fp = makeDynCaller(dc);

var fp = makeDynCaller();
if (typeof fp !== "function") {
throwBindingError("unknown function pointer with signature " + signature + ": " + rawFunction);
}
Expand Down
62 changes: 58 additions & 4 deletions src/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -3872,7 +3872,7 @@ LibraryManager.library = {
var trace = _emscripten_get_callstack_js();
var parts = trace.split('\n');
for (var i = 0; i < parts.length; i++) {
var ret = {{{ makeDynCall('iii') }}}(func, 0, arg);
var ret = {{{ makeDynCall('iii', 'func') }}}(0, arg);
if (ret !== 0) return;
}
},
Expand Down Expand Up @@ -3921,7 +3921,7 @@ LibraryManager.library = {
emscripten_scan_stack: function(func) {
var base = STACK_BASE; // TODO verify this is right on pthreads
var end = stackSave();
{{{ makeDynCall('vii') }}}(func, Math.min(base, end), Math.max(base, end));
{{{ makeDynCall('vii', 'func') }}}(Math.min(base, end), Math.max(base, end));
},

// misc definitions to avoid unnecessary unresolved symbols being reported
Expand Down Expand Up @@ -4004,6 +4004,60 @@ LibraryManager.library = {
});
},

#if USE_LEGACY_DYNCALLS || !WASM_BIGINT
$dynCallLegacy: function(sig, ptr, args) {
#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
if (args && args.length) {
return Module['dynCall_' + sig].apply(null, [ptr].concat(args));
}
return Module['dynCall_' + sig].call(null, ptr);
},
$dynCall__deps: ['$dynCallLegacy'],
Copy link
Member

Choose a reason for hiding this comment

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

it's odd to have the deps not near the function - why is this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because the dependency is conditional in the exact same way the this functions existence is conditions.

Copy link
Member

Choose a reason for hiding this comment

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

I guess I don't feel strongly but I believe this is the one place in the entire codebase not to have the two adjacent. Maybe it's worth the benefit though.


// Used in library code to get JS function from wasm function pointer.
// All callers should use direct table access where possible and only fall
// back to this function if needed.
$getDynCaller__deps: ['$dynCall'],
$getDynCaller: function(sig, ptr) {
#if !USE_LEGACY_DYNCALLS
assert(sig.indexOf('j') >= 0, 'getDynCaller should only be called with i64 sigs')
#endif
var argCache = [];
return function() {
argCache.length = arguments.length;
for (var i = 0; i < arguments.length; i++) {
argCache[i] = arguments[i];
}
return dynCall(sig, ptr, argCache);
};
},
#endif

$dynCall: function (sig, ptr, args) {
#if USE_LEGACY_DYNCALLS
return dynCallLegacy(sig, ptr, args);
#else
#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) {
return dynCallLegacy(sig, ptr, args);
}
#endif

return wasmTable.get(ptr).apply(null, args)
#endif
},

$callRuntimeCallbacks: function(callbacks) {
while(callbacks.length > 0) {
var callback = callbacks.shift();
Expand All @@ -4014,9 +4068,9 @@ LibraryManager.library = {
var func = callback.func;
if (typeof func === 'number') {
if (callback.arg === undefined) {
dynCall_v(func);
{{{ makeDynCall('v', 'func') }}}();
} else {
dynCall_vi(func, callback.arg);
{{{ makeDynCall('vi', 'func') }}}(callback.arg);
}
} else {
func(callback.arg === undefined ? null : callback.arg);
Expand Down
4 changes: 2 additions & 2 deletions src/library_async.js
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ mergeInto(LibraryManager.library, {
Asyncify.afterUnwind = function() {
var stackBegin = Asyncify.currData + {{{ C_STRUCTS.asyncify_data_s.__size__ }}};
var stackEnd = HEAP32[Asyncify.currData >> 2];
{{{ makeDynCall('vii') }}}(func, stackBegin, stackEnd);
{{{ makeDynCall('vii', 'func') }}}(stackBegin, stackEnd);
wakeUp();
};
});
Expand Down Expand Up @@ -408,7 +408,7 @@ mergeInto(LibraryManager.library, {
{{{ makeSetValue('newFiber', C_STRUCTS.emscripten_fiber_s.entry, 0, 'i32') }}};

var userData = {{{ makeGetValue('newFiber', C_STRUCTS.emscripten_fiber_s.user_data, 'i32') }}};
{{{ makeDynCall('vi') }}}(entryPoint, userData);
{{{ makeDynCall('vi', 'entryPoint') }}}(userData);
} else {
var asyncifyData = newFiber + {{{ C_STRUCTS.emscripten_fiber_s.asyncify_data }}};
Asyncify.currData = asyncifyData;
Expand Down
Loading