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:
> > > > 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?
I suspect we'll end up with -B flags if/when people start building interesting
or nontrivial toolchains with clang (or we try to make emscripten more
standardish), but I'm fine with leaving that out for now. Checking PATH for
wasm-opt seems fine to me to locate the binary. Did you have that in mind as
also the way to determine whether or not to run wasm-opt (i.e. run if it's
there, don't if it's not)? That seems slightly error-prone in the sense that
there would be a silent behavior change if something went wrong (e.g. wasm-opt
goes missing) but in a world where most clang users are using wasm-opt then
using wasm-opt by default seems reasonable; so that seems fine as a way to
start out.
I do think we will eventually want some way to modify the behavior of wasm-opt
though. For that matter wasm-opt might at some point become able to optimize
object files (allowing faster links at the cost of less LTO-optimized binaries;
we'd run a reduced set of passes post-link or none at all). In that case
there'd also have to be further changes if we want builtin support for that.
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
