Skip to content

Conversation

@sbc100
Copy link
Collaborator

@sbc100 sbc100 commented May 4, 2022

This change introduces and type to the __sig scheme that we using for
library functions: 'p'. This letter signifies that the paramater or
return value is the width of a pointer which means it will differ
between wasm32 and wasm64.

We use this whenever a JS function receives or returns pointer type or a
size_t type.

For wasm32, we don't need to do anything since the incoming value is and
i32. For wasm64 we covert the incoming BigInt value to an int53/double
at the JS/Wasm boundary. At the moment no checking is done on the
values, which means values that don't pricely convert to JS double can
be currupted/truncated. As a followup, we could consider using the
checking coversion functions, or asserting in debug builds if the values
don't fit within this range.

@sbc100 sbc100 requested review from aardappel and kripken May 4, 2022 16:14
@sbc100
Copy link
Collaborator Author

sbc100 commented May 4, 2022

This is just the initial change that introduces this concept. More widespread usage, and increased test coverage is coming in a followup.

@sbc100 sbc100 force-pushed the automatic_signature_conversions branch 2 times, most recently from d5cb46c to 32a18ed Compare May 4, 2022 16:23
@kripken
Copy link
Member

kripken commented May 4, 2022

For wasm64 we covert the incoming BigInt value to an int53/double at the JS/Wasm boundary.

Do we assume WASM_BIGINT when MEMORY64? It seems like we should if we don't already.

@aardappel
Copy link
Collaborator

Do we assume WASM_BIGINT when MEMORY64? It seems like we should if we don't already.

Yup that's already been hard-coded in since the start.

@aardappel
Copy link
Collaborator

Nice, this seems a more solid way to do it!

@kripken
Copy link
Member

kripken commented May 4, 2022

Yup that's already been hard-coded in since the start.

Great, thanks!

@sbc100 sbc100 force-pushed the automatic_signature_conversions branch from 32a18ed to 5525cf8 Compare May 4, 2022 18:09
@sbc100 sbc100 changed the title Convert BigInt<->Number based on library function __sig attributes [wasm64] convert BigInt<->Number based on library function __sig attributes May 4, 2022
@sbc100 sbc100 force-pushed the automatic_signature_conversions branch 4 times, most recently from 12d3cfd to c298297 Compare May 4, 2022 20:42
@sbc100 sbc100 enabled auto-merge (squash) May 4, 2022 20:42
@sbc100 sbc100 force-pushed the automatic_signature_conversions branch from c298297 to f058030 Compare May 4, 2022 20:43
@sbc100 sbc100 disabled auto-merge May 4, 2022 20:43
@sbc100 sbc100 enabled auto-merge (squash) May 4, 2022 20:43
…tributes

This change introduces and type to the `__sig` scheme that we using for
library functions: 'p'.  This letter signifies that the parameter or
return value is the width of a pointer which means it will differ
between wasm32 and wasm64.

We use this whenever a JS function receives or returns pointer type or a
size_t type.

For wasm32, we don't need to do anything since the incoming value is and
i32.  For wasm64 we convert the incoming BigInt value to an int53/double
at the JS/Wasm boundary.  At the moment no checking is done on the
values, which means values that don't precisely convert to JS double can
be corrupted/truncated.  As a followup, we could consider using the
checking conversion functions, or asserting in debug builds if the values
don't fit within this range.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants