[clang] Add flag to opt out of wasm-opt (PR #95208)
Quentin Michaud via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 12 02:12:59 PDT 2024
https://github.com/mh4ck-Thales created https://github.com/llvm/llvm-project/pull/95208
This PR fixes #55781 by adding the `--no-wasm-opt` and `--wasm-opt` flags in clang to disable/enable the `wasm-opt` optimizations. The default is to enable `wasm-opt` as before in order to not break existing workflows.
I think that adding a warning when no flag or the `--wasm-opt` flag is given but `wasm-opt` wasn't found in the path may be relevant here. It allows people using `wasm-opt` to be aware of if it have been used on their produced binary or not. The only downside I see to this is that people already using the toolchain with the `-O` and `-Werror` flags but without `wasm-opt` in the path will see their toolchain break (with an easy fix: either adding `--no-wasm-opt` or add `wasm-opt` to the path). I haven't implemented this here because I haven't figured out how to add such a warning, and I don't know if this warning should be added here or in another PR.
CC @sunfishcode that proposed in the associated issue to review this patch.
>From 8c7052d5da074eb1d754aeddc4257d33e4e299aa Mon Sep 17 00:00:00 2001
From: Quentin Michaud <quentin.michaud at thalesgroup.com>
Date: Tue, 11 Jun 2024 16:25:51 +0200
Subject: [PATCH] Add flag to opt out of wasm-opt (#55781)
---
clang/include/clang/Basic/LangOptions.h | 4 ++
clang/include/clang/Driver/Options.td | 8 +++
clang/lib/Driver/ToolChains/WebAssembly.cpp | 72 +++++++++++----------
3 files changed, 49 insertions(+), 35 deletions(-)
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 75e88afbd9705..91f1c2f2e6239 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -575,6 +575,10 @@ class LangOptions : public LangOptionsBase {
// implementation on real-world examples.
std::string OpenACCMacroOverride;
+ // Indicates if the wasm-opt binary must be ignored in the case of a
+ // WebAssembly target.
+ bool NoWasmOpt = false;
+
LangOptions();
/// Set language defaults for the given input language and
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index d44faa55c456f..22a400c9707b1 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -8740,3 +8740,11 @@ def spirv : DXCFlag<"spirv">,
def fspv_target_env_EQ : Joined<["-"], "fspv-target-env=">, Group<dxc_Group>,
HelpText<"Specify the target environment">,
Values<"vulkan1.2, vulkan1.3">;
+def no_wasm_opt : Flag<["--"], "no-wasm-opt">,
+ Group<m_Group>,
+ HelpText<"Disable the wasm-opt optimizer">,
+ MarshallingInfoFlag<LangOpts<"NoWasmOpt">>;
+def wasm_opt : Flag<["--"], "wasm-opt">,
+ Group<m_Group>,
+ HelpText<"Enable the wasm-opt optimizer (default)">,
+ MarshallingInfoNegativeFlag<LangOpts<"NoWasmOpt">>;
diff --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index 5b763df9b3329..60bd97e0ee987 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -158,44 +158,46 @@ void wasm::Linker::ConstructJob(Compilation &C, const JobAction &JA,
CmdArgs.push_back("-o");
CmdArgs.push_back(Output.getFilename());
- // When optimizing, if wasm-opt is available, run it.
- std::string WasmOptPath;
- if (Args.getLastArg(options::OPT_O_Group)) {
- WasmOptPath = ToolChain.GetProgramPath("wasm-opt");
- if (WasmOptPath == "wasm-opt") {
- WasmOptPath = {};
+ if (Args.hasFlag(options::OPT_wasm_opt, options::OPT_no_wasm_opt, true)) {
+ // When optimizing, if wasm-opt is available, run it.
+ std::string WasmOptPath;
+ if (Args.getLastArg(options::OPT_O_Group)) {
+ WasmOptPath = ToolChain.GetProgramPath("wasm-opt");
+ if (WasmOptPath == "wasm-opt") {
+ WasmOptPath = {};
+ }
}
- }
-
- if (!WasmOptPath.empty()) {
- CmdArgs.push_back("--keep-section=target_features");
- }
- C.addCommand(std::make_unique<Command>(JA, *this,
- ResponseFileSupport::AtFileCurCP(),
- Linker, CmdArgs, Inputs, Output));
-
- if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
if (!WasmOptPath.empty()) {
- StringRef OOpt = "s";
- if (A->getOption().matches(options::OPT_O4) ||
- A->getOption().matches(options::OPT_Ofast))
- OOpt = "4";
- else if (A->getOption().matches(options::OPT_O0))
- OOpt = "0";
- else if (A->getOption().matches(options::OPT_O))
- OOpt = A->getValue();
-
- if (OOpt != "0") {
- const char *WasmOpt = Args.MakeArgString(WasmOptPath);
- ArgStringList OptArgs;
- OptArgs.push_back(Output.getFilename());
- OptArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
- OptArgs.push_back("-o");
- OptArgs.push_back(Output.getFilename());
- C.addCommand(std::make_unique<Command>(
- JA, *this, ResponseFileSupport::AtFileCurCP(), WasmOpt, OptArgs,
- Inputs, Output));
+ CmdArgs.push_back("--keep-section=target_features");
+ }
+
+ C.addCommand(std::make_unique<Command>(JA, *this,
+ ResponseFileSupport::AtFileCurCP(),
+ Linker, CmdArgs, Inputs, Output));
+
+ if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+ if (!WasmOptPath.empty()) {
+ StringRef OOpt = "s";
+ if (A->getOption().matches(options::OPT_O4) ||
+ A->getOption().matches(options::OPT_Ofast))
+ OOpt = "4";
+ else if (A->getOption().matches(options::OPT_O0))
+ OOpt = "0";
+ else if (A->getOption().matches(options::OPT_O))
+ OOpt = A->getValue();
+
+ if (OOpt != "0") {
+ const char *WasmOpt = Args.MakeArgString(WasmOptPath);
+ ArgStringList OptArgs;
+ OptArgs.push_back(Output.getFilename());
+ OptArgs.push_back(Args.MakeArgString(llvm::Twine("-O") + OOpt));
+ OptArgs.push_back("-o");
+ OptArgs.push_back(Output.getFilename());
+ C.addCommand(std::make_unique<Command>(
+ JA, *this, ResponseFileSupport::AtFileCurCP(), WasmOpt, OptArgs,
+ Inputs, Output));
+ }
}
}
}
More information about the cfe-commits
mailing list