Skip to content
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.

Sign up for GitHub

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

Implement macros {{{ ptrToIdx() }}}, {{{ convertPtrToIdx() }}}, {{{ idxToPtr() }}} #19289

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Implement macros {{{ ptrToIdx() }}}, {{{ convertPtrToIdx() }}}, {{{ idxToPtr() }}} #19289

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b5f597f
Implement macros {{{ ptrToIdx() }}}, {{{ convertPtrToIdx() }}}, {{{ i…
juj May 11, 2023
d417758
More WebGL updates
juj Aug 29, 2023
3593b67
Fix merge issues
juj Aug 29, 2023
64d50cd
Work around Firefox and Chrome bugs
juj Aug 29, 2023
7d48ac1
Remove uses of CAN_ADDRESS_2GB
juj Aug 29, 2023
e97af5a
Rebaseline
juj Aug 29, 2023
cc8d42f
Fix conflict with closure and export wrapper generation in webgl
juj Aug 29, 2023
6a2efe2
Fix bad check
juj Aug 29, 2023
1805e3a
Rebaseline
juj Aug 29, 2023
ce19bdb
Use idxToPtr
juj Aug 30, 2023
ac9fce5
Merge remote-tracking branch 'origin/main' into ptr_to_idx_and_back
juj Aug 30, 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
24 changes: 12 additions & 12 deletions src/library.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@ addToLibrary({
#if ASSERTIONS
assert(typeof ptr === 'number');
#endif
#if !CAN_ADDRESS_2GB && !MEMORY64
// With CAN_ADDRESS_2GB or MEMORY64, pointers are already unsigned.
ptr >>>= 0;
#endif
{{{ convertPtrToIdx('ptr') }}};
return '0x' + ptr.toString(16).padStart(8, '0');
},

Expand Down Expand Up @@ -201,10 +198,7 @@ addToLibrary({
emscripten_resize_heap: 'ip',
emscripten_resize_heap: (requestedSize) => {
var oldSize = HEAPU8.length;
#if !MEMORY64 && !CAN_ADDRESS_2GB
// With CAN_ADDRESS_2GB or MEMORY64, pointers are already unsigned.
requestedSize >>>= 0;
#endif
{{{ convertPtrToIdx('requestedSize') }}};
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about just dropping the convert and just calling this ptrToIdx? I'm also not very clear that "Index" is the right thing to be calling this. I don't love either of these but what about ptrToNumber or wasmToJSPtr ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Because emscripten_resize_heap has its argument marked as p I don't think we need the full conversion here do we? i.e. we didn't need from64 here before.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Having a separate convertPtrToIdx('variable') function has the benefit that it avoids a redundant assignment to self in wasm2gb builds. I.e. if one writes

variable = {{{ ptrToIdx('variable') }}};

Then it will generate variable = variable; in <2GB builds, and one will need to use Closure to fix that up.

#if ALLOW_MEMORY_GROWTH == 0
#if ABORTING_MALLOC
abortOnCannotGrowMemory(requestedSize);
Expand Down Expand Up @@ -3215,6 +3209,9 @@ addToLibrary({
// Function pointers are 64-bit, but wasmTable.get() requires a Number.
// https://github.com/emscripten-core/emscripten/issues/18200
funcPtr = Number(funcPtr);
#endif
#if ASSERTIONS
assert(funcPtr >= 0, "Function pointers must be nonnegative!");
#endif
var func = wasmTableMirror[funcPtr];
if (!func) {
Expand Down Expand Up @@ -3243,6 +3240,9 @@ addToLibrary({
// Function pointers are 64-bit, but wasmTable.get() requires a Number.
// https://github.com/emscripten-core/emscripten/issues/18200
funcPtr = Number(funcPtr);
#endif
#if ASSERTIONS
assert(funcPtr >= 0, "Function pointers must be nonnegative!");
#endif
// In -Os and -Oz builds, do not implement a JS side wasm table mirror for small
// code size, but directly access wasmTable, which is a bit slower as uncached.
Expand Down Expand Up @@ -3516,11 +3516,11 @@ addToLibrary({
#if RELOCATABLE
// Globals that are normally exported from the wasm module but in relocatable
// mode are created here and imported by the module.
__stack_pointer: "new WebAssembly.Global({'value': '{{{ POINTER_WASM_TYPE }}}', 'mutable': true}, {{{ to64(STACK_HIGH) }}})",
__stack_pointer: "new WebAssembly.Global({'value': '{{{ POINTER_WASM_TYPE }}}', 'mutable': true}, {{{ idxToPtr(STACK_HIGH) }}})",
// tell the memory segments where to place themselves
__memory_base: "new WebAssembly.Global({'value': '{{{ POINTER_WASM_TYPE }}}', 'mutable': false}, {{{ to64(GLOBAL_BASE) }}})",
__memory_base: "new WebAssembly.Global({'value': '{{{ POINTER_WASM_TYPE }}}', 'mutable': false}, {{{ idxToPtr(GLOBAL_BASE) }}})",
// the wasm backend reserves slot 0 for the NULL function pointer
__table_base: "new WebAssembly.Global({'value': '{{{ POINTER_WASM_TYPE }}}', 'mutable': false}, {{{ to64(TABLE_BASE) }}})",
__table_base: "new WebAssembly.Global({'value': '{{{ POINTER_WASM_TYPE }}}', 'mutable': false}, {{{ idxToPtr(TABLE_BASE) }}})",
#if MEMORY64 == 2
__memory_base32: "new WebAssembly.Global({'value': 'i32', 'mutable': false}, {{{ GLOBAL_BASE }}})",
#endif
Expand All @@ -3545,7 +3545,7 @@ addToLibrary({
#endif
#if ASYNCIFY == 1
__asyncify_state: "new WebAssembly.Global({'value': 'i32', 'mutable': true}, 0)",
__asyncify_data: "new WebAssembly.Global({'value': '{{{ POINTER_WASM_TYPE }}}', 'mutable': true}, {{{ to64(0) }}})",
__asyncify_data: "new WebAssembly.Global({'value': '{{{ POINTER_WASM_TYPE }}}', 'mutable': true}, {{{ idxToPtr(0) }}})",
#endif
#endif

Expand Down
11 changes: 5 additions & 6 deletions src/library_ccall.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,30 +27,29 @@ addToLibrary({
// For fast lookup of conversion functions
var toC = {
#if MEMORY64
'pointer': (p) => {{{ to64('p') }}},
'pointer': (p) => {{{ idxToPtr('p') }}},
#endif
'string': (str) => {
var ret = 0;
if (str !== null && str !== undefined && str !== 0) { // null string
// at most 4 bytes per UTF-8 code point, +1 for the trailing '\0'
ret = stringToUTF8OnStack(str);
}
return {{{ to64('ret') }}};
return {{{ idxToPtr('ret') }}};
},
'array': (arr) => {
var ret = stackAlloc(arr.length);
writeArrayToMemory(arr, ret);
return {{{ to64('ret') }}};
return {{{ idxToPtr('ret') }}};
}
};

function convertReturnValue(ret) {
if (returnType === 'string') {
{{{ from64('ret') }}}
return UTF8ToString(ret);
return UTF8ToString({{{ ptrToIdx('ret') }}});
}
#if MEMORY64
if (returnType === 'pointer') return Number(ret);
if (returnType === 'pointer') return {{{ ptrToIdx('ret') }}};
#endif
if (returnType === 'boolean') return Boolean(ret);
return ret;
Expand Down
30 changes: 13 additions & 17 deletions src/library_dylink.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ var LibraryDylink = {
args = args.map(Number);
#endif
var rtn = findMatchingCatch(args);
return {{{ to64('rtn') }}};
return {{{ idxToPtr('rtn') }}};
}
}
#endif
Expand Down Expand Up @@ -226,7 +226,7 @@ var LibraryDylink = {
dbg(`updateGOT: before: ${symName} : ${GOT[symName].value}`);
#endif
if (typeof value == 'function') {
GOT[symName].value = {{{ to64('addFunction(value)') }}};
GOT[symName].value = {{{ idxToPtr('addFunction(value)') }}};
#if DYLINK_DEBUG == 2
dbg(`updateGOT: FUNC: ${symName} : ${GOT[symName].value}`);
#endif
Expand Down Expand Up @@ -272,7 +272,7 @@ var LibraryDylink = {
value = value.value;
}
if (typeof value == {{{ POINTER_JS_TYPE }}}) {
value += {{{ to64('memoryBase') }}};
value += {{{ idxToPtr('memoryBase') }}};
}
relocated[e] = value;
}
Expand Down Expand Up @@ -304,12 +304,12 @@ var LibraryDylink = {
#endif
if (typeof value == 'function') {
/** @suppress {checkTypes} */
GOT[symName].value = {{{ to64('addFunction(value, value.sig)') }}};
GOT[symName].value = {{{ idxToPtr('addFunction(value, value.sig)') }}};
#if DYLINK_DEBUG == 2
dbg(`assigning table entry for : ${symName} -> ${GOT[symName].value}`);
#endif
} else if (typeof value == 'number') {
GOT[symName].value = {{{ to64('value') }}};
GOT[symName].value = {{{ idxToPtr('value') }}};
#if MEMORY64
} else if (typeof value == 'bigint') {
GOT[symName].value = value;
Expand Down Expand Up @@ -381,7 +381,7 @@ var LibraryDylink = {
assert(end <= HEAP8.length, 'failure to getMemory - memory growth etc. is not supported there, call malloc/sbrk directly or increase INITIAL_MEMORY');
#endif
___heap_base = end;
GOT['__heap_base'].value = {{{ to64('end') }}};
GOT['__heap_base'].value = {{{ idxToPtr('end') }}};
return ret;
},

Expand Down Expand Up @@ -692,9 +692,9 @@ var LibraryDylink = {
// symbols that should be local to this module
switch (prop) {
case '__memory_base':
return {{{ to64('memoryBase') }}};
return {{{ idxToPtr('memoryBase') }}};
case '__table_base':
return {{{ to64('tableBase') }}};
return {{{ idxToPtr('tableBase') }}};
#if MEMORY64
#if MEMORY64 == 2
case '__memory_base32':
Expand Down Expand Up @@ -757,7 +757,7 @@ var LibraryDylink = {
// now. Otherwise this is delayed until `setDylinkStackLimits` is
// called after initialization.
if (moduleExports['__set_stack_limits'] && runtimeInitialized) {
moduleExports['__set_stack_limits']({{{ to64('_emscripten_stack_get_base()') }}}, {{{ to64('_emscripten_stack_get_end()') }}});
moduleExports['__set_stack_limits']({{{ idxToPtr('_emscripten_stack_get_base()') }}}, {{{ idxToPtr('_emscripten_stack_get_end()') }}});
}
#endif

Expand All @@ -782,10 +782,8 @@ var LibraryDylink = {

// Add any EM_ASM function that exist in the side module
if ('__start_em_asm' in moduleExports) {
var start = moduleExports['__start_em_asm'];
var stop = moduleExports['__stop_em_asm'];
{{{ from64('start') }}}
{{{ from64('stop') }}}
var start = {{{ ptrToIdx('moduleExports["__start_em_asm"]') }}};
var stop = {{{ ptrToIdx('moduleExports["__stop_em_asm"]') }}};
while (start < stop) {
var jsString = UTF8ToString(start);
addEmAsm(start, jsString);
Expand Down Expand Up @@ -815,9 +813,7 @@ var LibraryDylink = {

for (var name in moduleExports) {
if (name.startsWith('__em_js__')) {
var start = moduleExports[name]
{{{ from64('start') }}}
var jsString = UTF8ToString(start);
var jsString = UTF8ToString(moduleExports[name]);
// EM_JS strings are stored in the data section in the form
// SIG<::>BODY.
var parts = jsString.split('<::>');
Expand Down Expand Up @@ -899,7 +895,7 @@ var LibraryDylink = {
#endif
var lib = LDSO.loadedLibsByName[name];
if (lib.exports['__set_stack_limits']) {
lib.exports['__set_stack_limits']({{{ to64("stackTop") }}}, {{{ to64("stackMax") }}});
lib.exports['__set_stack_limits']({{{ idxToPtr("stackTop") }}}, {{{ idxToPtr("stackMax") }}});
}
}
},
Expand Down
2 changes: 1 addition & 1 deletion src/library_glemu.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,7 @@ var LibraryGLEmulation = {
(GL.currentContext.compressionExt ? ' GL_ARB_texture_compression GL_EXT_texture_compression_s3tc' : '') +
(GL.currentContext.anisotropicExt ? ' GL_EXT_texture_filter_anisotropic' : '')
);
return GL.stringCache[name_] = {{{ to64('ret') }}};
return GL.stringCache[name_] = {{{ idxToPtr('ret') }}};
}
return glGetString(name_);
};
Expand Down
Loading