search

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.28k stars 4.73k forks source link

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

Open lewing opened 12 months ago

lewing commented 12 months ago

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 commented 12 months ago

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: -
RReverser commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

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 commented 3 months ago

Note that because we now produce WASI components, wasm-opt can't be used on those until https://github.com/WebAssembly/binaryen/issues/6728