Add flag to opt out of wasm-opt (llvm#95208)

This PR fixes llvm#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.
llvm-project-tlp · Jun 27, 2024 · 680315c · 680315c
1 parent f4e6d25
commit 680315c
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 35 deletions.
diff --git 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
@@ -8771,3 +8771,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
@@ -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));
+        }
       }
     }
   }