-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add ILStrip+wasm_opt pass when using WasmSingleFileBundle #95478
Comments
Tagging subscribers to this area: @agocke, @vitek-karas, @VSadov Issue Detailsnull
|
Tagging subscribers to 'arch-wasm': @lewing Issue Detailsnull
|
For browser-wasm the SingleFileBundle support is missing - #95480 . |
We can run ilstrip by hand to test first but we should add it to the build anyway |
I am not sure how to do |
This is covered by some other changes that I'm working on. |
@radical If you are hitting any issues using it, feel free to let me know. |
@radical what is the state of this, ILStrip is in, what is the state of wasm-opt? |
It's not a separate step. I can extract that into one though. runtime/src/mono/wasm/build/WasmApp.targets Lines 507 to 511 in f83838b
What arguments do we want to run that with? |
The discussion in https://github.com/dotnet/runtime/issues/94804 is relevant here. And https://github.com/emscripten-core/emscripten/blob/c54ebd73e383742e1e94729d863f97f23cfa8ff0/tools/link.py#L369 and if the section is indeed missing in the wasi build I assume clang is doing something similar in the final link step |
WIP - #95900 |
The build has support for this now, but not all of it is working for example - browser-wasm+singlefilebundle . |
Note that because we now produce WASI components, wasm-opt can't be used on those until WebAssembly/binaryen#6728 But I think we il-strip on publish ? |
When using WasmSingleFileBundle+AOT we should be able to run ILStrip + wasm-opt which we think would optimize out the zero'd out memory. We should test this hypothesis on browser-wasm and implement it on wasi-wasm if it is true
cc @fanyang-mono @SamMonoRT
The text was updated successfully, but these errors were encountered: