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

[mono][wasi] Optional wasm-opt pass #94804

Open
Tracked by #94803
lewing opened this issue Nov 15, 2023 · 5 comments
Open
Tracked by #94803

[mono][wasi] Optional wasm-opt pass #94804

lewing opened this issue Nov 15, 2023 · 5 comments
Labels
arch-wasm WebAssembly architecture area-VM-meta-mono os-wasi Related to WASI variant of arch-wasm
Milestone

Comments

@lewing
Copy link
Member

lewing commented Nov 15, 2023

wasm-opt which we already ship as part of the wasm-tools workload can be used to optimize wasi builds as well. We should have an optional step to run it as part of the build.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 15, 2023
@lewing lewing added os-wasi Related to WASI variant of arch-wasm arch-wasm WebAssembly architecture labels Nov 15, 2023
@ghost
Copy link

ghost commented Nov 15, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

wasm-opt which we already ship as part of the wasm-tools workload can be used to optimize wasi builds as well. We should have an optional step to run it as part of the build.

Author: lewing
Assignees: -
Labels:

arch-wasm, untriaged, area-VM-meta-mono, os-wasi

Milestone: -

@lewing lewing removed the untriaged New issue has not been triaged by the area owner label Nov 15, 2023
@lewing lewing added this to the 9.0.0 milestone Nov 15, 2023
@RReverser
Copy link

RReverser commented Nov 20, 2023

Note that you (.NET) are currently using Clang for linking, which already implicitly runs wasm-opt if 1) it's detected on the PATH and 2) linker optimisation is enabled.

It can be enabled via e.g. this in .csproj:

<_WasiSdkClangArgs Include="-O2" />

The problem is that currently somewhere in the pipeline .NET either doesn't emit correct target_features custom section, or maybe strips it. That secion is important for any WebAssembly tools that want to modify the binary as otherwise they don't know which features they can legally use without breaking compatibility. Correspondingly, when enabling link-time optimisation like above, wasm-opt fails validation with errors like:

  Performing WASI SDK build: "C:\Users\me\.wasi-sdk\wasi-sdk-20\bin\clang.exe" "@..."
  [wasm-validator error in function 5099] unexpected false: Bulk memory operations require bulk memory [--enable-bulk-memory], on 
  (memory.copy
   (local.get $0)
   (local.get $1)
   (local.get $2)
  )
  [wasm-validator error in function 5100] unexpected false: Bulk memory operations require bulk memory [--enable-bulk-memory], on
  (memory.copy
   (local.get $0)
   (local.get $1)
   (local.get $2)
  )
  [wasm-validator error in function 5101] unexpected false: Bulk memory operations require bulk memory [--enable-bulk-memory], on
  (memory.fill
   (local.get $0)
   (local.get $1)
   (local.get $2)
  )
  Fatal: error validating input
clang : error : linker command failed with exit code 1 (use -v to see invocation)

That is, it complains because Wasm uses bulk memory operations but doesn't have a target_features section that allows them.

I guess I could report this as a separate issue, but it seems most relevant to this one as it's pretty much the only blocker to using wasm-opt and other similar tools.

It's possible to work around that by doing something like wasm-opt -all for now but that's potentially dangerous because it allows wasm-opt to use arbitrary features regardless of whether our target engines support them.

@lewing
Copy link
Member Author

lewing commented Dec 11, 2023

I'm not sure why the section isn't being generated will take a look. At the moment --enable-bulk-memory and --enable-simd (If you pass WasmEnableSIMD) are the only post MVP features in the wasi build.

@RReverser
Copy link

At the moment --enable-bulk-memory and --enable-simd (If you pass WasmEnableSIMD) are the only post MVP features in the wasi build.

Yeah they're currently passed explicitly, but the thing is that explicitly enabling features from command line really shouldn't be necessary if target_features is preserved correctly.

@pavelsavara
Copy link
Member

Note that because we now produce WASI components, wasm-opt can't be used on those until WebAssembly/binaryen#6728

@mkhamoyan mkhamoyan removed their assignment Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-wasm WebAssembly architecture area-VM-meta-mono os-wasi Related to WASI variant of arch-wasm
Projects
None yet
Development

No branches or pull requests

4 participants