sunfish marked an inline comment as done.
sunfish 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";
----------------
dschuff wrote:
> 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?
It checks the COMPILER_PATH environment variable and -B command-line flags,
which I'm not sure we should use here, but it looks like it does fall back to
checking PATH at the end.
So, what would you think of just checking PATH 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
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
