Skip to content

[Wasm64] Initial work for get parts of embind working #17239

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

Merged
merged 1 commit into from
Jun 17, 2022
Merged

[Wasm64] Initial work for get parts of embind working #17239

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 embuilder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
'libdlmalloc-noerrno',
'libdlmalloc-tracing',
'libdlmalloc-debug',
'libembind-rtti',
'libemmalloc',
'libemmalloc-debug',
'libemmalloc-memvalidate',
Expand Down
8 changes: 7 additions & 1 deletion src/embind/embind.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,7 @@ var LibraryEmbind = {

_embind_register_bool__deps: [
'$getShiftFromSize', '$readLatin1String', '$registerType'],
_embind_register_bool__sig: 'vpppii',
Copy link
Member

Choose a reason for hiding this comment

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

Here's where I start asking dumb basic questions about JS libs.
what happens when there's no signature on a JS lib function? What does adding them here accomplish?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are a couple of different uses for these signatures.

In this case we need the signature so that we can generate automatic BigInt<->Number conversion wrapper at the boundary. The p here means pointer and is used for pointer/size_t things that we know assume won't exceed 53-bits..

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without these the JS code recieves BitInt values when running with MEMORY64.. which most of our JS code is not setup to receive.

_embind_register_bool: function(rawType, name, size, trueValue, falseValue) {
var shift = getShiftFromSize(size);

Expand Down Expand Up @@ -546,9 +547,12 @@ var LibraryEmbind = {
_embind_register_integer__deps: [
'embind_repr', '$getShiftFromSize', '$integerReadValueFromPointer',
'$readLatin1String', '$registerType'],
_embind_register_integer__sig: 'vpppii',
_embind_register_integer: function(primitiveType, name, size, minRange, maxRange) {
name = readLatin1String(name);
if (maxRange === -1) { // LLVM doesn't have signed and unsigned 32-bit types, so u32 literals come out as 'i32 -1'. Always treat those as max u32.
// LLVM doesn't have signed and unsigned 32-bit types, so u32 literals come
// out as 'i32 -1'. Always treat those as max u32.
if (maxRange === -1) {
maxRange = 4294967295;
}

Expand Down Expand Up @@ -599,6 +603,7 @@ var LibraryEmbind = {
#if WASM_BIGINT
_embind_register_bigint__deps: [
'embind_repr', '$readLatin1String', '$registerType', '$integerReadValueFromPointer'],
_embind_register_bigint__sig: 'vpppjj',
_embind_register_bigint: function(primitiveType, name, size, minRange, maxRange) {
name = readLatin1String(name);

Expand Down Expand Up @@ -639,6 +644,7 @@ var LibraryEmbind = {
_embind_register_float__deps: [
'embind_repr', '$floatReadValueFromPointer', '$getShiftFromSize',
'$readLatin1String', '$registerType'],
_embind_register_float__sig: 'vppp',
_embind_register_float: function(rawType, name, size) {
var shift = getShiftFromSize(size);
name = readLatin1String(name);
Expand Down
60 changes: 37 additions & 23 deletions src/embind/emval.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

// -- jshint doesn't understand library syntax, so we need to mark the symbols exposed here
/*global getStringOrSymbol, emval_handle_array, Emval, __emval_unregister, count_emval_handles, emval_symbols, emval_free_list, get_first_emval, __emval_decref, emval_newers*/
/*global craftEmvalAllocator, __emval_addMethodCaller, emval_methodCallers, LibraryManager, mergeInto, __emval_allocateDestructors, global, __emval_lookupTypes, makeLegalFunctionName*/
/*global craftEmvalAllocator, emval_addMethodCaller, emval_methodCallers, LibraryManager, mergeInto, emval_allocateDestructors, global, emval_lookupTypes, makeLegalFunctionName*/
/*global emval_get_global*/

var LibraryEmVal = {
Expand Down Expand Up @@ -261,7 +261,7 @@ var LibraryEmVal = {
})()('return this')();
},
#endif
_emval_get_global__sig: 'ii',
_emval_get_global__sig: 'pp',
_emval_get_global__deps: ['$Emval', '$getStringOrSymbol', '$emval_get_global'],
_emval_get_global: function(name) {
if (name===0) {
Expand All @@ -279,7 +279,7 @@ var LibraryEmVal = {
return Emval.toHandle(Module[name]);
},

_emval_get_property__sig: 'iii',
_emval_get_property__sig: 'ppp',
_emval_get_property__deps: ['$Emval'],
_emval_get_property: function(handle, key) {
handle = Emval.toValue(handle);
Expand All @@ -296,7 +296,7 @@ var LibraryEmVal = {
handle[key] = value;
},

_emval_as__sig: 'iiii',
_emval_as__sig: 'dppp',
_emval_as__deps: ['$Emval', '$requireRegisteredType'],
_emval_as: function(handle, returnType, destructorsRef) {
handle = Emval.toValue(handle);
Expand All @@ -308,58 +308,65 @@ var LibraryEmVal = {
},

_emval_as_int64__deps: ['$Emval', '$requireRegisteredType'],
_emval_as_int64__sig: 'dppp',
_emval_as_int64: function(handle, returnType, destructorsRef) {
handle = Emval.toValue(handle);
returnType = requireRegisteredType(returnType, 'emval::as');
return returnType['toWireType'](null, handle);
},

_emval_as_uint64__deps: ['$Emval', '$requireRegisteredType'],
_emval_as_uint64__sig: 'dppp',
_emval_as_uint64: function(handle, returnType, destructorsRef) {
handle = Emval.toValue(handle);
returnType = requireRegisteredType(returnType, 'emval::as');
return returnType['toWireType'](null, handle);
},

_emval_equals__deps: ['$Emval'],
_emval_equals__sig: 'ipp',
_emval_equals: function(first, second) {
first = Emval.toValue(first);
second = Emval.toValue(second);
return first == second;
},

_emval_strictly_equals__deps: ['$Emval'],
_emval_strictly_equals__sig: 'ipp',
_emval_strictly_equals: function(first, second) {
first = Emval.toValue(first);
second = Emval.toValue(second);
return first === second;
},

_emval_greater_than__deps: ['$Emval'],
_emval_greater_than__sig: 'ipp',
_emval_greater_than: function(first, second) {
first = Emval.toValue(first);
second = Emval.toValue(second);
return first > second;
},

_emval_less_than__deps: ['$Emval'],
_emval_less_than__sig: 'ipp',
_emval_less_than: function(first, second) {
first = Emval.toValue(first);
second = Emval.toValue(second);
return first < second;
},

_emval_not__deps: ['$Emval'],
_emval_not__sig: 'ip',
_emval_not: function(object) {
object = Emval.toValue(object);
return !object;
},

_emval_call__sig: 'iiiii',
_emval_call__deps: ['_emval_lookupTypes', '$Emval'],
_emval_call__sig: 'ppppp',
_emval_call__deps: ['$emval_lookupTypes', '$Emval'],
_emval_call: function(handle, argCount, argTypes, argv) {
handle = Emval.toValue(handle);
var types = __emval_lookupTypes(argCount, argTypes);
var types = emval_lookupTypes(argCount, argTypes);

var args = new Array(argCount);
for (var i = 0; i < argCount; ++i) {
Expand All @@ -372,18 +379,18 @@ var LibraryEmVal = {
return Emval.toHandle(rv);
},

_emval_lookupTypes__deps: ['$requireRegisteredType'],
_emval_lookupTypes: function(argCount, argTypes) {
$emval_lookupTypes__deps: ['$requireRegisteredType'],
Copy link
Member

Choose a reason for hiding this comment

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

what's the difference between the $-prefixed and the _-prefixed function names?

Copy link
Collaborator Author

@sbc100 sbc100 Jun 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

$-prefixed functions are JS-only, and not visible to the native code/linker. When we have a function like this that take a JS object, my policy is always to $-prefix it.

$emval_lookupTypes: function(argCount, argTypes) {
var a = new Array(argCount);
for (var i = 0; i < argCount; ++i) {
a[i] = requireRegisteredType(HEAP32[(argTypes >> 2) + i],
a[i] = requireRegisteredType({{{ makeGetValue('argTypes', 'i * POINTER_SIZE', '*') }}},
"parameter " + i);
}
return a;
},

_emval_allocateDestructors__deps: ['$Emval'],
_emval_allocateDestructors: function(destructorsRef) {
$emval_allocateDestructors__deps: ['$Emval'],
$emval_allocateDestructors: function(destructorsRef) {
var destructors = [];
HEAP32[destructorsRef >> 2] = Emval.toHandle(destructors);
return destructors;
Expand All @@ -393,18 +400,18 @@ var LibraryEmVal = {
// to have null be a valid method caller.
$emval_methodCallers: [undefined],

_emval_addMethodCaller__deps: ['$emval_methodCallers'],
_emval_addMethodCaller: function(caller) {
$emval_addMethodCaller__deps: ['$emval_methodCallers'],
$emval_addMethodCaller: function(caller) {
var id = emval_methodCallers.length;
emval_methodCallers.push(caller);
return id;
},

$emval_registeredMethods: [],
_emval_get_method_caller__sig: 'iii',
_emval_get_method_caller__deps: ['_emval_addMethodCaller', '_emval_lookupTypes', '$new_', '$makeLegalFunctionName', '$emval_registeredMethods'],
_emval_get_method_caller__sig: 'ppp',
_emval_get_method_caller__deps: ['$emval_addMethodCaller', '$emval_lookupTypes', '$new_', '$makeLegalFunctionName', '$emval_registeredMethods'],
_emval_get_method_caller: function(argCount, argTypes) {
var types = __emval_lookupTypes(argCount, argTypes);
var types = emval_lookupTypes(argCount, argTypes);
var retType = types[0];
var signatureName = retType.name + "_$" + types.slice(1).map(function (t) { return t.name; }).join("_") + "$";
var returnId = emval_registeredMethods[signatureName];
Expand Down Expand Up @@ -469,69 +476,76 @@ var LibraryEmVal = {
params.push(functionBody);
var invokerFunction = new_(Function, params).apply(null, args);
#endif
returnId = __emval_addMethodCaller(invokerFunction);
returnId = emval_addMethodCaller(invokerFunction);
emval_registeredMethods[signatureName] = returnId;
return returnId;
},

_emval_call_method__deps: ['_emval_allocateDestructors', '$getStringOrSymbol', '$emval_methodCallers', '$Emval'],
_emval_call_method__deps: ['$emval_allocateDestructors', '$getStringOrSymbol', '$emval_methodCallers', '$Emval'],
_emval_call_method__sig: 'dppppp',
_emval_call_method: function(caller, handle, methodName, destructorsRef, args) {
caller = emval_methodCallers[caller];
handle = Emval.toValue(handle);
methodName = getStringOrSymbol(methodName);
return caller(handle, methodName, __emval_allocateDestructors(destructorsRef), args);
return caller(handle, methodName, emval_allocateDestructors(destructorsRef), args);
},

_emval_call_void_method__sig: 'viiii',
_emval_call_void_method__deps: ['_emval_allocateDestructors', '$getStringOrSymbol', '$emval_methodCallers', '$Emval'],
_emval_call_void_method__sig: 'vpppp',
_emval_call_void_method__deps: ['$emval_allocateDestructors', '$getStringOrSymbol', '$emval_methodCallers', '$Emval'],
_emval_call_void_method: function(caller, handle, methodName, args) {
caller = emval_methodCallers[caller];
handle = Emval.toValue(handle);
methodName = getStringOrSymbol(methodName);
caller(handle, methodName, null, args);
},

_emval_typeof__sig: 'ii',
_emval_typeof__sig: 'pp',
_emval_typeof__deps: ['$Emval'],
_emval_typeof: function(handle) {
handle = Emval.toValue(handle);
return Emval.toHandle(typeof handle);
},

_emval_instanceof__deps: ['$Emval'],
_emval_instanceof__sig: 'ipp',
_emval_instanceof: function(object, constructor) {
object = Emval.toValue(object);
constructor = Emval.toValue(constructor);
return object instanceof constructor;
},

_emval_is_number__deps: ['$Emval'],
_emval_is_number__sig: 'ip',
_emval_is_number: function(handle) {
handle = Emval.toValue(handle);
return typeof handle == 'number';
},

_emval_is_string__deps: ['$Emval'],
_emval_is_string__sig: 'ip',
_emval_is_string: function(handle) {
handle = Emval.toValue(handle);
return typeof handle == 'string';
},

_emval_in__deps: ['$Emval'],
_emval_in__sig: 'ipp',
_emval_in: function(item, object) {
item = Emval.toValue(item);
object = Emval.toValue(object);
return item in object;
},

_emval_delete__deps: ['$Emval'],
_emval_delete__sig: 'ipp',
_emval_delete: function(object, property) {
object = Emval.toValue(object);
property = Emval.toValue(property);
return delete object[property];
},

_emval_throw__deps: ['$Emval'],
_emval_throw__sig: 'vp',
_emval_throw: function(object) {
object = Emval.toValue(object);
throw object;
Expand Down
8 changes: 4 additions & 4 deletions system/include/emscripten/bind.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,15 @@ void _embind_register_integer(
TYPEID integerType,
const char* name,
size_t size,
long minRange,
unsigned long maxRange);
int32_t minRange,
uint32_t maxRange);

void _embind_register_bigint(
TYPEID integerType,
const char* name,
size_t size,
long long minRange,
unsigned long long maxRange);
int64_t minRange,
uint64_t maxRange);

void _embind_register_float(
TYPEID floatType,
Expand Down
35 changes: 12 additions & 23 deletions system/include/emscripten/val.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#error Including <emscripten/val.h> requires building with -std=c++11 or newer!
#endif

#include <cassert>
#include <array>
#include <climits>
#include <emscripten/wire.h>
Expand Down Expand Up @@ -190,15 +191,14 @@ union GenericWireType {
union {
unsigned u;
float f;
#if __ILP32__
const void* p;
#endif
// Use uint32_t for pointer values. This limits us, for now, to 32-bit
// address ranges even on wasm64. This is enforced by assertions below.
// TODO(sbc): Allow full 64-bit address range here under wasm64, most
Copy link
Member

Choose a reason for hiding this comment

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

I assume if we updated this to 64-bit, we'd still want to use 53-bit ranges on the JS side though?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think they are separate concerns.

// likely by increasing the size of GenericWireType on wasm64.
uint32_t p;
} w[2];
double d;
uint64_t u;
#if __LP64__
const void* p;
#endif
};
static_assert(sizeof(GenericWireType) == 8, "GenericWireType must be 8 bytes");
static_assert(alignof(GenericWireType) == 8, "GenericWireType must be 8-byte-aligned");
Expand All @@ -225,29 +225,18 @@ inline void writeGenericWireType(GenericWireType*& cursor, uint64_t wt) {

template<typename T>
void writeGenericWireType(GenericWireType*& cursor, T* wt) {
#if __ILP32__
cursor->w[0].p = wt;
#else
cursor->p = wt;
// FIXME: This requires the JS reading code to be audited to be compatible with it.
assert(false);
abort();
#endif
uintptr_t short_ptr = reinterpret_cast<uintptr_t>(wt);
assert(short_ptr <= UINT32_MAX);
Copy link
Member

Choose a reason for hiding this comment

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

is this assert going to get compiled out in release builds? Should we maybe make it a "real" error, or is there some other code that will help the user if they happen to end up with a pointer outside this range?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Its only going to fire for users of MEMORY64 + embind... which is hyper experiential config anyway, so I'm not too worried.

cursor->w[0].p = short_ptr;
++cursor;
}

template<typename ElementType>
inline void writeGenericWireType(GenericWireType*& cursor, const memory_view<ElementType>& wt) {
uintptr_t short_ptr = reinterpret_cast<uintptr_t>(wt.data);
assert(short_ptr <= UINT32_MAX);
cursor->w[0].u = wt.size;
#if __ILP32__
cursor->w[1].p = wt.data;
#else
// FIXME: need to change GenericWireType such that it can store a 64-bit pointer?
// This requires the JS reading code to be audited to be compatible with it.
cursor->w[1].u = 0;
assert(false);
abort();
#endif
cursor->w[1].p = short_ptr;
++cursor;
}

Expand Down
5 changes: 5 additions & 0 deletions system/lib/embind/bind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,13 @@ void __embind_register_native_and_builtin_types() {
register_integer<unsigned short>("unsigned short");
register_integer<signed int>("int");
register_integer<unsigned int>("unsigned int");
#if __wasm64__
register_bigint<signed long>("long");
register_bigint<unsigned long>("unsigned long");
#else
register_integer<signed long>("long");
register_integer<unsigned long>("unsigned long");
#endif

register_bigint<int64_t>("int64_t");
register_bigint<uint64_t>("uint64_t");
Expand Down
1 change: 0 additions & 1 deletion tests/test_core.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7332,7 +7332,6 @@ def test2():

do_test(test2, level=2, prefix='hello_libcxx')

@no_wasm64('embind does not yet support MEMORY64')
def test_embind(self):
# Verify that both the old `--bind` arg and the new `-lembind` arg work
for args in [['-lembind'], ['--bind']]:
Expand Down