[clang] b4ebf2d - [WebAssembly] Disable running `wasm-opt` on components (#98373)

[via cfe-commits]

Author: Alex Crichton
Date: 2024-07-23T07:51:49-07:00
New Revision: b4ebf2d2872e28883ac654dec5414cb3a565fa17

URL: https://github.com/llvm/llvm-project/commit/b4ebf2d2872e28883ac654dec5414cb3a565fa17
DIFF: https://github.com/llvm/llvm-project/commit/b4ebf2d2872e28883ac654dec5414cb3a565fa17.diff

LOG: [WebAssembly] Disable running `wasm-opt` on components (#98373)

This commit adds a check that disables `wasm-opt` for the
`wasm32-wasip2` target because `wasm-opt` doesn't support components at
this time. This also fixes a minor issue from #95208 where if `wasm-opt`
was disabled then the linker wouldn't run at all.

Added: 
    

Modified: 
    clang/lib/Driver/ToolChains/WebAssembly.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp b/clang/lib/Driver/ToolChains/WebAssembly.cpp
index 60bd97e0ee987..9aacda5fd5702 100644
--- a/clang/lib/Driver/ToolChains/WebAssembly.cpp
+++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp
@@ -61,6 +61,13 @@ std::string wasm::Linker::getLinkerPath(const ArgList &Args) const {
   return ToolChain.GetProgramPath(ToolChain.getDefaultLinker());
 }
 
+static bool TargetBuildsComponents(const llvm::Triple &TargetTriple) {
+  // WASIp2 and above are all based on components, so test for WASI but exclude
+  // the original `wasi` target in addition to the `wasip1` name.
+  return TargetTriple.isOSWASI() && TargetTriple.getOSName() != "wasip1" &&
+         TargetTriple.getOSName() != "wasi";
+}
+
 void wasm::Linker::ConstructJob(Compilation &C, const JobAction &JA,
                                 const InputInfo &Output,
                                 const InputInfoList &Inputs,
@@ -158,46 +165,52 @@ void wasm::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   CmdArgs.push_back("-o");
   CmdArgs.push_back(Output.getFilename());
 
-  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 = {};
-      }
+  // Don't use wasm-opt by default on `wasip2` as it doesn't have support for
+  // components at this time. Retain the historical default otherwise, though,
+  // of running `wasm-opt` by default.
+  bool WasmOptDefault = !TargetBuildsComponents(ToolChain.getTriple());
+  bool RunWasmOpt = Args.hasFlag(options::OPT_wasm_opt,
+                                 options::OPT_no_wasm_opt, WasmOptDefault);
+
+  // If wasm-opt is enabled and optimizations are happening look for the
+  // `wasm-opt` program. If it's not found auto-disable it.
+  std::string WasmOptPath;
+  if (RunWasmOpt && 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");
-    }
+  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));
-        }
+  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));
       }
     }
   }
@@ -241,7 +254,7 @@ WebAssembly::WebAssembly(const Driver &D, const llvm::Triple &Triple,
 }
 
 const char *WebAssembly::getDefaultLinker() const {
-  if (getOS() == "wasip2")
+  if (TargetBuildsComponents(getTriple()))
     return "wasm-component-ld";
   return "wasm-ld";
 }