[PATCH] D70500: [WebAssembly] Enable use of wasm-opt and LTO-enabled system libraries

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

(sorry for the slow response here)

>> If you need to run an external program, look for it next to the compiler, like we do for gas with -fno-integrated-as, linker, etc. People can use the existing -B flag to make clang look elsewhere if they need that.
> 
> Unfortunately, wasm-opt isn't typically installed alongside the compiler. It's also not a typical system tool like an assembler, which is why it didn't seem right to search the -B paths.
> 
> The first version of my patch used a dedicated environment variable, WASM_OPT, rather than PATH, but I changed it to PATH after review feedback. If people feel using -B paths, and COMPILER_PATH, are appropriate, we can use `GetProgramPath` itself, though note that that still does have a PATH fallback.

Here's the scenario I'm worried about: Imagine someone packaging a hermetic toolchain that bundles clang, lld, and so on, but it doesn't bundle wasm-opt. After this patch, the toolchain won't behave hermetically for wasm compilations, since it now does different things based on if wasm-opt is on PATH (which it is if a dev is locally playing with emscripten, say).

It's true that the driver falls back to PATH as a last resort measure for the linker, but, as you say, that's usually next to the compiler binary and the fallback isn't needed, but wasm-opt usually isn't there.

What should be the way forward for that? Options:

- Add a driver flag that explicitly sets if PATH should be used as final fallback for all tools (including this one)
  - and maybe change the default to false since this behavior now goes form "basically never used" to "usually used, for wasm compilations"
- Add a dedicated flag that controls if wasm-opt should be looked for in PATH
- ...?

(See also http://blog.llvm.org/2019/11/deterministic-builds-with-clang-and-lld.html ctrl-f "environment variables")


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70500/new/

https://reviews.llvm.org/D70500