dschuff added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:96
+ if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+ if (const char *WasmOptPath = getenv("WASM_OPT")) {
+ StringRef OOpt = "s";
----------------
sunfish wrote:
> dschuff wrote:
> > sunfish wrote:
> > > dschuff wrote:
> > > > What would you think about adding a way to pass arguments through to
> > > > wasm-opt on the command line, like we have for the linker, assembler,
> > > > etc? Something like `-Wo,-O2` (or `-Ww` or whatever; analogous to `-Wl`
> > > > and `-Wa`). That seems nicer than an env var, although it doesn't solve
> > > > the problem of controlling whether to run the optimizer in the first
> > > > place.
> > > My guess here is that we don't need clang to have an option for passing
> > > additional flags -- if people want to do extra special things with
> > > wasm-opt on clang's output they can just run wasm-opt directly
> > > themselves. Does that sound reasonable?
> > Maybe. But I still don't like the use of an env var for this kind of
> > behavior-effecting (i.e. non-debugging) use case. It's hard enough to get
> > reproducible and hermetic build behavior as it is, I definitely wouldn't
> > want to worry about the environment affecting the output in addition to all
> > the random flags, included files, etc.
> If we did -Wo,-O2 or so, we'd still need to be able to find the wasm-opt
> program to be able to run it. We could just search for it in PATH, but that's
> also a little dicey from a hermetic build perspective.
>
> I borrowed "WASM_OPT" from
> [cargo-wasi](https://github.com/bytecodealliance/cargo-wasi). I'm also not a
> fan of environment variables in general, but this way does have the advantage
> that people can set it once, and not have to modify their Makefiles to add
> new flags. Users can think of it as just being part of -O2 and friends.
>
What's the usual way to locate things like external assemblers? Presumably we
could use the same mechanism for wasm-opt?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70500/new/
https://reviews.llvm.org/D70500
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
