llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-driver Author: Quentin Michaud (mh4ck-Thales) <details> <summary>Changes</summary> 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. --- Full diff: https://github.com/llvm/llvm-project/pull/95208.diff 3 Files Affected: - (modified) clang/include/clang/Basic/LangOptions.h (+4) - (modified) clang/include/clang/Driver/Options.td (+8) - (modified) clang/lib/Driver/ToolChains/WebAssembly.cpp (+37-35) ``````````diff 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)); + } } } } `````````` </details> https://github.com/llvm/llvm-project/pull/95208 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
