[clang] Add flag to opt out of wasm-opt (PR #95208)

[Pavel Savara via cfe-commits]

pavelsavara wrote:

I would also love to see https://github.com/llvm/llvm-project/pull/98373 
merged. Thank you.

https://github.com/llvm/llvm-project/pull/95208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Add flag to opt out of wasm-opt (PR #95208)

[Pavel Savara via cfe-commits]

pavelsavara wrote:

I don't know who needs to hear this, but I got bitten by `wasm-opt` being on my 
PATH (while using WASI SDK 22).

I was trying to compile zlib-ng for WASIp2 and it's CMake platform detection 
was passing `-O3` to `check_type_size("void *" SIZEOF_DATA_PTR)`
It worked on some machines and it broke on others. Those which had `wasm-opt` 
on PATH.

Because in `WASIp2` the LLVM produces WASM component not WASM module. 
The wasm-opt doesn't know how to read the component binary and fails with 
`parse exception: surprising value (at 0:8)`

When that happens during CMake detetection, that problem is not surfaced and 
you are looking at
```
-- Check size of void *
-- sizeof(void *) is  bytes
  CMake Error at C:/Dev/runtime/src/native/external/zlib-ng/CMakeLists.txt:485 
(message):
sizeof(void *) is neither 32 nor 64 bit
```

Until now, I was naive user of CMake.

- Maybe the `--no-wasm-opt` should be the default for WASM components
- because not doing so is not breaking change, I guess.
- Maybe the `--no-wasm-opt` should be the default everywhere
- Maybe LLVM should check wasm-opt version or compatibility first ?
- Maybe `wasm-component-ld` should pass the module to wasm-opt before it makes 
component out of it ?
- Or maybe wasm-opt needs to learn how to unpack component, optimize the 
modules and pack it back ?
- if wasm-opt is expected on components too

Should I open new issue for this ? Which of it and where ?

https://github.com/llvm/llvm-project/pull/95208
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [WebAssembly] Disable running `wasm-opt` on components (PR #98373)

[Pavel Savara via cfe-commits]

@@ -61,6 +61,10 @@ std::string wasm::Linker::getLinkerPath(const ArgList &Args) 
const {
   return ToolChain.GetProgramPath(ToolChain.getDefaultLinker());
 }
 
+static bool IsWasip2(const llvm::Triple &TargetTriple) {
+  return TargetTriple.getOSName() == "wasip2";

pavelsavara wrote:

I guess WASI preview3 and the future will be the same. Should we invert the 
test and only check for `wasip1` ?

https://github.com/llvm/llvm-project/pull/98373
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [WebAssembly] Disable running `wasm-opt` on components (PR #98373)

[Pavel Savara via cfe-commits]

pavelsavara wrote:

could shell script named `wasm-opt` on PATH and doing nothing do the trick ?
Or even better it could write it's arguments into some file, so that they could 
be asserted in the test.

2c

https://github.com/llvm/llvm-project/pull/98373
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits