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.

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

Binaryen's Emscripten bindings do not support WASM_BIGINT #7163

Open
dschuff opened this issue Dec 19, 2024 · 0 comments · May be fixed by #7167
Open

Binaryen's Emscripten bindings do not support WASM_BIGINT #7163

dschuff opened this issue Dec 19, 2024 · 0 comments · May be fixed by #7167

Comments

@dschuff
Copy link
Member

dschuff commented Dec 19, 2024

Currently the tests fail when BIGINT is enabled:
https://github.com/WebAssembly/binaryen/actions/runs/12406958589/job/34636211812?pr=7161

Emscripten enables WASM_BIGINT by default now and the feature is very widely supported, so if there is a benefit to enabling it, then we should fix the Binaryen bindings.

dschuff added a commit that referenced this issue Dec 19, 2024
…ff (#7162)

This handles the case where WASM_BIGINT is enabled by default by emcc.
Binaryen's JS bindings do not currently work with bigint (see #7163)
tlively added a commit that referenced this issue Dec 20, 2024
We previously had a build option for enabling BigInt support when
compiling with Emscripten, but the test suite did not actually pass with
it enabled. The problem was that the binaryen.js glue code assumed that
C API functions that took i64 parameters would be transformed to take a
pair of i32 parameters instead, but this assumption was incorrect when
BigInt support was enabled.

Emscripten has now enabled BigInt by default, so update binaryen.js to
use BigInt as well. Fix the JS API glue code to pass i64s as BigInts
rather than pairs of numbers and fix the tests accordingly. Also fix
some other small problems with the tests that only show up in debug
builds.

Resolves #7163.
@tlively tlively linked a pull request Dec 20, 2024 that will close this issue
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 a pull request may close this issue.

1 participant