-
Notifications
You must be signed in to change notification settings - Fork 5.2k
[browser][coreCLR] enable bigint #121025
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
base: main
Are you sure you want to change the base?
[browser][coreCLR] enable bigint #121025
Conversation
|
Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov |
- detect max memory size - use bulk memory - fix zero size calls to memcpy
|
There is also an option to fix the underlying bug in our emscripten fork for our current version. That would take some effort, depends on how much we need this now. @pavelsavara does enabling bigint improve the linking times on your system? |
The fix is in emscripten-core/emscripten#22873, we could do partial backport into our fork. But I much prefer full upgrade. Yes it does make a large difference. Yes -sMIN_CHROME_VERSION=74 does the trick in our version of emscripten. We would loose NON_TRAPPING_FPTOINT and PROMISE_ANY why is that a big deal @SingleAccretion ? |
It is not a big deal. What I was worried about is this https://github.com/emscripten-core/emscripten/blob/8ac7ef7de5b9dc555d83db0f17bbc22346914dba/tools/link.py#L1284-L1299. Looks like it can be turned off via |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR enables WebAssembly BigInt support and bulk memory operations for the CoreCLR browser runtime. The changes improve WASM compatibility and prevent runtime errors when working with zero-size memory operations.
Key changes:
- Enable
-sWASM_BIGINT=1and-mbulk-memorycompiler flags across browser-related build configurations - Add guards to prevent
memcpycalls with zero-size buffers, which can cause issues in WASM - Skip cgroup file system operations on WASM platform and implement WASM-specific memory size detection
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/tests/CMakeLists.txt | Adds -mbulk-memory flag to test build configuration |
| src/native/corehost/browserhost/CMakeLists.txt | Enables bulk memory and BigInt flags for browser host |
| src/coreclr/vm/methodtablebuilder.cpp | Guards ZeroMemory call when field count is zero |
| src/coreclr/pal/src/misc/cgroup.cpp | Prevents cgroup operations on WASM and guards cgroup path finding |
| src/coreclr/pal/CMakeLists.txt | Adds -mbulk-memory flag to PAL build |
| src/coreclr/interpreter/compiler.cpp | Guards multiple memcpy calls against zero-size operations |
| src/coreclr/hosts/corerun/CMakeLists.txt | Enables bulk memory and BigInt flags for corerun host |
| src/coreclr/gc/unix/gcenv.unix.cpp | Implements WASM-specific memory size detection using emscripten_get_heap_max() |
| src/coreclr/gc/unix/cgroup.cpp | Prevents cgroup operations on WASM and guards cgroup path finding |
|
Note that recent emscripten has bulk memory as default AFAIK, so if we ever update emscripten this will happen anyway. I think it's still worthwhile to explicitly turn bulk memory on now though even if it will become the default later. |
Yes, and with it we also closer to current mono, which has bulk memory enabled too. |
Sure, we want to do that anyway. |
-sWASM_BIGINT=1-mbulk-memorymemcpyon zero size buffersFixes #119639
Fixes #119997
Related #120298 (comment)