From 177fe1d4df4e9f02382c2f8f5dce9de4d935d763 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 10 Jul 2024 12:18:06 -0700 Subject: [PATCH 1/4] [WebAssembly] Disable running `wasm-opt` on components 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. --- clang/lib/Driver/ToolChains/WebAssembly.cpp | 85 ++++++++++++--------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp b/clang/lib/Driver/ToolChains/WebAssembly.cpp index 60bd97e0ee987..2753645f4700f 100644 --- a/clang/lib/Driver/ToolChains/WebAssembly.cpp +++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp @@ -61,6 +61,10 @@ std::string wasm::Linker::getLinkerPath(const ArgList &Args) const { return ToolChain.GetProgramPath(ToolChain.getDefaultLinker()); } +static bool IsWasip2(const llvm::Triple &TargetTriple) { + return TargetTriple.getOSName() == "wasip2"; +} + void wasm::Linker::ConstructJob(Compilation &C, const JobAction &JA, const InputInfo &Output, const InputInfoList &Inputs, @@ -158,46 +162,51 @@ 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 = !IsWasip2(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(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( - JA, *this, ResponseFileSupport::AtFileCurCP(), WasmOpt, OptArgs, - Inputs, Output)); - } + C.addCommand(std::make_unique(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( + JA, *this, ResponseFileSupport::AtFileCurCP(), WasmOpt, OptArgs, + Inputs, Output)); } } } @@ -241,7 +250,7 @@ WebAssembly::WebAssembly(const Driver &D, const llvm::Triple &Triple, } const char *WebAssembly::getDefaultLinker() const { - if (getOS() == "wasip2") + if (IsWasip2(getTriple())) return "wasm-component-ld"; return "wasm-ld"; } From cb76384fb8ebaba9ec62dde79b278033eaae20f2 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 10 Jul 2024 13:09:24 -0700 Subject: [PATCH 2/4] Run clang-format --- clang/lib/Driver/ToolChains/WebAssembly.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp b/clang/lib/Driver/ToolChains/WebAssembly.cpp index 2753645f4700f..2ad2513dbc8ab 100644 --- a/clang/lib/Driver/ToolChains/WebAssembly.cpp +++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp @@ -166,7 +166,8 @@ void wasm::Linker::ConstructJob(Compilation &C, const JobAction &JA, // components at this time. Retain the historical default otherwise, though, // of running `wasm-opt` by default. bool WasmOptDefault = !IsWasip2(ToolChain.getTriple()); - bool RunWasmOpt = Args.hasFlag(options::OPT_wasm_opt, options::OPT_no_wasm_opt, WasmOptDefault); + 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. From 92a3ba63779c5b6bc295db2bb79a9aa35c38e599 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 10 Jul 2024 13:11:31 -0700 Subject: [PATCH 3/4] Explicitly exclude wasip1 instead of including wasip2 --- clang/lib/Driver/ToolChains/WebAssembly.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp b/clang/lib/Driver/ToolChains/WebAssembly.cpp index 2ad2513dbc8ab..b3f084e62bde9 100644 --- a/clang/lib/Driver/ToolChains/WebAssembly.cpp +++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp @@ -61,8 +61,12 @@ std::string wasm::Linker::getLinkerPath(const ArgList &Args) const { return ToolChain.GetProgramPath(ToolChain.getDefaultLinker()); } -static bool IsWasip2(const llvm::Triple &TargetTriple) { - return TargetTriple.getOSName() == "wasip2"; +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, @@ -165,7 +169,7 @@ void wasm::Linker::ConstructJob(Compilation &C, const JobAction &JA, // 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 = !IsWasip2(ToolChain.getTriple()); + bool WasmOptDefault = !TargetBuildsComponents(ToolChain.getTriple()); bool RunWasmOpt = Args.hasFlag(options::OPT_wasm_opt, options::OPT_no_wasm_opt, WasmOptDefault); @@ -251,7 +255,7 @@ WebAssembly::WebAssembly(const Driver &D, const llvm::Triple &Triple, } const char *WebAssembly::getDefaultLinker() const { - if (IsWasip2(getTriple())) + if (TargetBuildsComponents(getTriple())) return "wasm-component-ld"; return "wasm-ld"; } From 1f27f6ae73fbfe7cf25c046df1ff4ef691a57fb7 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 10 Jul 2024 13:23:43 -0700 Subject: [PATCH 4/4] Fix formatting --- clang/lib/Driver/ToolChains/WebAssembly.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp b/clang/lib/Driver/ToolChains/WebAssembly.cpp index b3f084e62bde9..9aacda5fd5702 100644 --- a/clang/lib/Driver/ToolChains/WebAssembly.cpp +++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp @@ -64,9 +64,8 @@ std::string wasm::Linker::getLinkerPath(const ArgList &Args) const { 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"; + return TargetTriple.isOSWASI() && TargetTriple.getOSName() != "wasip1" && + TargetTriple.getOSName() != "wasi"; } void wasm::Linker::ConstructJob(Compilation &C, const JobAction &JA,