[Clang] Rework LTO argument handling in the linker wrapper

[jhuber6]

Summary

The clang-linker-wrapper is responsible for performing embedded
linking jobs for the offloading device. In the linker wrapper we need to
implement our own handling of LTO to account for the diverse landscape
of linkers and assemblers for accelerators. However, we end up
duplication a lot of functionality. This patch changes the argument
generation for the device to match the arguments we use for host LTO.
This will make it much easier to add extra arguments.

One thing to be careful of however, is that not all linkers will not
accept -plugin-opt arguments if there is not a corresonding -plugin.
In order to combat this we need to make sure that all -plugin-opt
arguments indented for the device's LTO are not propagated to the host
linking job if we are using a linker that does not support it.

[jhuber6]

On second thought I'm not sure if overloading -plugin-opt is a good idea because we could have situations where we'd want them to be mutually exclusive, although I'd like to reuse the logic to set the arguments. I could change it to emit -offload-opt= instead or something similar to indicate it's for embedded offloading code, but that would cause some churn replacing every single --plugin-opt string in addLTOOptions. Would appreciate some feedback on this.

clang/lib/Driver/ToolChains/Clang.cpp

SmallVector<std::string, 12> BCLibs =
         RocmTC.getCommonDeviceLibNames(TCArgs, Arch.str());
 
     for (StringRef LibName : BCLibs)
       CmdArgs.push_back(Args.MakeArgString(
           "--bitcode-library=" + Action::GetOffloadKindName(Action::OFK_OpenMP) +
           "-" + TC->getTripleString() + "-" + Arch + "=" + LibName));
   }
 
-  if (D.isUsingLTO(/* IsOffload */ true)) {
-    // Pass in the optimization level to use for LTO.
-    if (const Arg *A = Args.getLastArg(options::OPT_O_Group)) {
-      StringRef OOpt;
-      if (A->getOption().matches(options::OPT_O4) ||
-          A->getOption().matches(options::OPT_Ofast))
-        OOpt = "3";
-      else if (A->getOption().matches(options::OPT_O)) {
-        OOpt = A->getValue();
-        if (OOpt == "g")
-          OOpt = "1";
-        else if (OOpt == "s" || OOpt == "z")
-          OOpt = "2";
-      } else if (A->getOption().matches(options::OPT_O0))
-        OOpt = "0";
-      if (!OOpt.empty())
-        CmdArgs.push_back(Args.MakeArgString(Twine("--opt-level=O") + OOpt));
-    }
-  }
+  if (D.isUsingLTO(/* IsOffload */ true) && !D.isUsingLTO())
+    addLTOOptions(getToolChain(), Args, CmdArgs, Output, Inputs[0],
+                  D.getLTOMode(true) == LTOK_Thin, /* IsOffload */ true);
 
   CmdArgs.push_back(
       Args.MakeArgString("--host-triple=" + TheTriple.getTriple()));
   if (Args.hasArg(options::OPT_v))
     CmdArgs.push_back("--verbose");
 
   if (const Arg *A = Args.getLastArg(options::OPT_g_Group)) {
     if (!A->getOption().matches(options::OPT_g0))
       CmdArgs.push_back("--device-debug");
   }
 
   for (const auto &A : Args.getAllArgValues(options::OPT_Xcuda_ptxas))
     CmdArgs.push_back(Args.MakeArgString("--ptxas-args=" + A));
 
   // Forward remarks passes to the LLVM backend in the wrapper.
-  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_EQ))
-    CmdArgs.push_back(
-        Args.MakeArgString(Twine("--pass-remarks=") + A->getValue()));
-  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_missed_EQ))
-    CmdArgs.push_back(
-        Args.MakeArgString(Twine("--pass-remarks-missed=") + A->getValue()));
-  if (const Arg *A = Args.getLastArg(options::OPT_Rpass_analysis_EQ))
-    CmdArgs.push_back(
-        Args.MakeArgString(Twine("--pass-remarks-analysis=") + A->getValue()));
   if (Args.getLastArg(options::OPT_save_temps_EQ))
     CmdArgs.push_back("--save-temps");
 
   // Construct the link job so we can wrap around it.
   Linker->ConstructJob(C, JA, Output, Inputs, Args, LinkingOutput);
   const auto &LinkCommand = C.getJobs().getJobs().back();
 
   // Forward -Xoffload-linker<-triple> arguments to the device link job.

clang/lib/Driver/ToolChains/CommonArgs.h

const char *SplitDebugName(const JobAction &JA, const llvm::opt::ArgList &Args,
                            const InputInfo &Input, const InputInfo &Output);
 
 void SplitDebugInfo(const ToolChain &TC, Compilation &C, const Tool &T,
                     const JobAction &JA, const llvm::opt::ArgList &Args,
                     const InputInfo &Output, const char *OutFile);
 
 void addLTOOptions(const ToolChain &ToolChain, const llvm::opt::ArgList &Args,
                    llvm::opt::ArgStringList &CmdArgs, const InputInfo &Output,
-                   const InputInfo &Input, bool IsThinLTO);
+                   const InputInfo &Input, bool IsThinLTO,
+                   bool IsOffload = false);
 
 std::tuple<llvm::Reloc::Model, unsigned, bool>
 ParsePICArgs(const ToolChain &ToolChain, const llvm::opt::ArgList &Args);
 
 unsigned ParseFunctionAlignment(const ToolChain &TC,
                                 const llvm::opt::ArgList &Args);
 
 unsigned ParseDebugDefaultVersion(const ToolChain &TC,

clang/lib/Driver/ToolChains/CommonArgs.cpp

 
 // CloudABI and PS4/PS5 use -ffunction-sections and -fdata-sections by default.
 bool tools::isUseSeparateSections(const llvm::Triple &Triple) {
   return Triple.getOS() == llvm::Triple::CloudABI || Triple.isPS();
 }
 
 void tools::addLTOOptions(const ToolChain &ToolChain, const ArgList &Args,
                           ArgStringList &CmdArgs, const InputInfo &Output,
-                          const InputInfo &Input, bool IsThinLTO) {
+                          const InputInfo &Input, bool IsThinLTO,
+                          bool IsOffload) {
   const char *Linker = Args.MakeArgString(ToolChain.GetLinkerPath());
   const Driver &D = ToolChain.getDriver();
   if (llvm::sys::path::filename(Linker) != "ld.lld" &&
-      llvm::sys::path::stem(Linker) != "ld.lld") {
+      llvm::sys::path::stem(Linker) != "ld.lld" && !IsOffload) {
     // Tell the linker to load the plugin. This has to come before
     // AddLinkerInputs as gold requires -plugin to come before any -plugin-opt
     // that -Wl might forward.
     CmdArgs.push_back("-plugin");
 
 #if defined(_WIN32)
     const char *Suffix = ".dll";
 #elif defined(__APPLE__)

clang/test/Driver/linker-wrapper.c

 // RUN:   --image=file=%S/Inputs/dummy-elf.o,kind=openmp,triple=x86_64-unknown-linux-gnu
 // RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o -fembed-offload-object=%t.out
 // RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
 // RUN:   --linker-path=/usr/bin/ld.lld -- %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=CPU_LINK
 
 // CPU_LINK: ld.lld{{.*}}-m elf_x86_64 -shared -Bsymbolic -o {{.*}}.out {{.*}}.o {{.*}}.o
 
 // RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o
-// RUN: clang-linker-wrapper --dry-run --host-triple=x86_64-unknown-linux-gnu -mllvm -abc \
+// RUN: clang-linker-wrapper --dry-run --host-triple=x86_64-unknown-linux-gnu -mllvm -pass-remarks=. \
 // RUN:   --linker-path=/usr/bin/ld.lld -- -a -b -c %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=HOST_LINK
 
 // HOST_LINK: ld.lld{{.*}}-a -b -c {{.*}}.o -o a.out
-// HOST_LINK-NOT: ld.lld{{.*}}-abc
+// HOST_LINK-NOT: ld.lld{{.*}}-pass-remarks=
+
+// RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o
+// RUN: clang-linker-wrapper --dry-run --host-triple=x86_64-unknown-linux-gnu \
+// RUN:   --linker-path=/usr/bin/ld -- -a -b -c -plugin-opt=a=b %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=HOST-PLUGIN
+
+// HOST-PLUGIN-NOT: ld.lld{{.*}}-plugin-opt
 
 // RUN: clang-offload-packager -o %t.out \
 // RUN:   --image=file=%S/Inputs/dummy-bc.bc,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70 \
 // RUN:   --image=file=%S/Inputs/dummy-bc.bc,kind=openmp,triple=nvptx64-nvidia-cuda,arch=sm_70
 // RUN: %clang -cc1 %s -triple x86_64-unknown-linux-gnu -emit-obj -o %t.o -fembed-offload-object=%t.out
 // RUN: clang-linker-wrapper --host-triple=x86_64-unknown-linux-gnu --dry-run \
 // RUN:   --linker-path=/usr/bin/ld -- %t.o -o a.out 2>&1 | FileCheck %s --check-prefix=LTO
 

clang/test/Driver/openmp-offload-gpu-new.c

 
 // CHECK-LTO-FEATURES: clang-offload-packager{{.*}}--image={{.*}}feature=+ptx{{[0-9]+}}
 
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp --offload-arch=sm_52 -nogpulib \
 // RUN:     -Xopenmp-target=nvptx64-nvidia-cuda --cuda-feature=+ptx64 -foffload-lto %s 2>&1 \
 // RUN:    | FileCheck --check-prefix=CHECK-SET-FEATURES %s
 
 // CHECK-SET-FEATURES: clang-offload-packager{{.*}}--image={{.*}}feature=+ptx64
+
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp --offload-arch=sm_52 -nogpulib \
+// RUN:     -foffload-lto -O3 -Rpass=. %s 2>&1 | FileCheck --check-prefix=CHECK-LTO-ARGS %s
+// CHECK-LTO-ARGS: clang-linker-wrapper{{.*}}"-plugin-opt=O3"{{.*}}"--plugin-opt=-pass-remarks=."
+// CHECK-LTO-ARGS-NOT: clang-linker-wrapper{{.*}}"-plugin"

clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp

 #include "llvm/Support/WithColor.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/Target/TargetMachine.h"
 
 using namespace llvm;
 using namespace llvm::opt;
 using namespace llvm::object;
 
-/// We use the command line parser only to forward options like `-pass-remarks`
-/// to the LLVM tools.
-static cl::OptionCategory
-    ClangLinkerWrapperCategory("clang-linker-wrapper options");
-static cl::opt<bool> Help("h", cl::desc("Alias for -help"), cl::Hidden,
-                          cl::cat(ClangLinkerWrapperCategory));
-static cl::list<std::string>
-    DummyArguments(cl::Sink, cl::Hidden, cl::cat(ClangLinkerWrapperCategory));
-
 /// Path of the current binary.
 static const char *LinkerExecutable;
 
 /// Ssave intermediary results.
 static bool SaveTemps = false;
 
 /// Print arguments without executing.
 static bool DryRun = false;

 
 namespace {
 using std::error_code;
 
 /// Must not overlap with llvm::opt::DriverFlag.
 enum WrapperFlags {
   WrapperOnlyOption = (1 << 4), // Options only used by the linker wrapper.
   DeviceOnlyOption = (1 << 5),  // Options only used for device linking.
+  PluginOnlyOption = (1 << 6),  // Options only used by the linker plugin.
 };
 
 enum ID {
   OPT_INVALID = 0, // This is not an option ID.
 #define OPTION(PREFIX, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM,  \
                HELPTEXT, METAVAR, VALUES)                                      \
   OPT_##ID,
 #include "LinkerWrapperOpts.inc"
Expected<StringRef> assemble(StringRef InputFile, const ArgList &Args,
   StringRef Arch = Args.getLastArgValue(OPT_arch_EQ);
   // Create a new file to write the linked device image to. Assume that the
   // input filename already has the device and architecture.
   auto TempFileOrErr = createOutputFile(sys::path::stem(InputFile), "cubin");
   if (!TempFileOrErr)
     return TempFileOrErr.takeError();
 
   SmallVector<StringRef, 16> CmdArgs;
-  StringRef OptLevel = Args.getLastArgValue(OPT_opt_level, "O2");
+  StringRef OptLevel = Args.getLastArgValue(OPT_opt_level, "2");
   CmdArgs.push_back(*PtxasPath);
   CmdArgs.push_back(Triple.isArch64Bit() ? "-m64" : "-m32");
   if (Verbose)
     CmdArgs.push_back("-v");
   for (StringRef Arg : Args.getAllArgValues(OPT_ptxas_arg))
     CmdArgs.push_back(Args.MakeArgString(Arg));
   CmdArgs.push_back("-o");
   CmdArgs.push_back(*TempFileOrErr);
-  CmdArgs.push_back(Args.MakeArgString("-" + OptLevel));
+  CmdArgs.push_back(Args.MakeArgString("-O" + OptLevel));
   CmdArgs.push_back("--gpu-name");
   CmdArgs.push_back(Arch);
   if (Args.hasArg(OPT_debug))
     CmdArgs.push_back("-g");
   if (RDC)
     CmdArgs.push_back("-c");
 
   CmdArgs.push_back(InputFile);
std::unique_ptr<lto::LTO> createLTO(
   lto::ThinBackend Backend;
   // TODO: Handle index-only thin-LTO
   Backend =
       lto::createInProcessThinBackend(llvm::heavyweight_hardware_concurrency());
 
   Conf.CPU = Arch.str();
   Conf.Options = codegen::InitTargetOptionsFromCodeGenFlags(Triple);
 
-  StringRef OptLevel = Args.getLastArgValue(OPT_opt_level, "O2");
+  StringRef OptLevel = Args.getLastArgValue(OPT_opt_level, "2");
   Conf.MAttrs = Features;
-  Conf.CGOptLevel = getCGOptLevel(OptLevel[1] - '0');
-  Conf.OptLevel = OptLevel[1] - '0';
+  Conf.CGOptLevel = getCGOptLevel(OptLevel[0] - '0');
+  Conf.OptLevel = OptLevel[0] - '0';
   if (Conf.OptLevel > 0)
     Conf.UseDefaultPipeline = true;
   Conf.DefaultTriple = Triple.getTriple();
 
   LTOError = false;
   Conf.DiagHandler = diagnosticHandler;
 
   Conf.PTO.LoopVectorization = Conf.OptLevel > 1;
Tbl.printHelp(
         Args.hasArg(OPT_help_hidden), Args.hasArg(OPT_help_hidden));
     return EXIT_SUCCESS;
   }
   if (Args.hasArg(OPT_v)) {
     printVersion(outs());
     return EXIT_SUCCESS;
   }
 
-  // This forwards '-pass-remarks=' to the LTO backend if present.
-  cl::HideUnrelatedOptions(ClangLinkerWrapperCategory);
-  cl::ParseCommandLineOptions(Argc, Argv);
+  // This forwards '-plugin-opt=' or '-mllvm' arguments to LLVM if present.
+  SmallVector<const char *> NewArgv = {Argv[0]};
+  for (const opt::Arg *Arg : Args.filtered(OPT_mllvm))
+    NewArgv.push_back(Arg->getValue());
+  for (const opt::Arg *Arg : Args.filtered(OPT_plugin_opt_eq_minus))
+    NewArgv.push_back(Args.MakeArgString(StringRef("-") + Arg->getValue()));
+  cl::ParseCommandLineOptions(NewArgv.size(), &NewArgv[0]);
 
   Verbose = Args.hasArg(OPT_verbose);
   DryRun = Args.hasArg(OPT_dry_run);
   SaveTemps = Args.hasArg(OPT_save_temps);
   ExecutableName = Args.getLastArgValue(OPT_o, "a.out");
   CudaBinaryPath = Args.getLastArgValue(OPT_cuda_path_EQ).str();
   if (!CudaBinaryPath.empty())
     CudaBinaryPath = CudaBinaryPath + "/bin";
-
   StringRef Root = Args.getLastArgValue(OPT_sysroot_EQ);
+  StringRef LinkerPath = Args.getLastArgValue(OPT_linker_path_EQ);
 
   SmallVector<StringRef> LibraryPaths;
   for (const opt::Arg *Arg : Args.filtered(OPT_library_path))
     LibraryPaths.push_back(Arg->getValue());
 
   // Try to extract device code from the linker input files.
   SmallVector<OffloadFile> InputFiles;
   SmallVector<OffloadFile> LazyInputFiles;
if (IsTargetUsed.contains(LazyFile))
       InputFiles.emplace_back(std::move(LazyFile));
   LazyInputFiles.clear();
 
   // Link and wrap the device images extracted from the linker input.
   auto FilesOrErr = linkAndWrapDeviceFiles(InputFiles, Args);
   if (!FilesOrErr)
     reportError(FilesOrErr.takeError());
 
-  // Render the linker arguments and add the newly created image. We add it
+  // Render the host linker arguments and add the newly created image. We add it
   // after the output file to ensure it is linked with the correct libraries.
   ArgStringList LinkerArgs;
   for (const opt::Arg *Arg : Args) {
+    bool NeedsPlugin = llvm::sys::path::filename(LinkerPath) != "ld.lld" &&
+                       llvm::sys::path::stem(LinkerPath) != "ld.lld";
+
+    // Do not render inputs only meant for the linker wrapper.
     if (Arg->getOption().hasFlag(WrapperOnlyOption))
       continue;
+    // Do not render plugin options without a plugin given if needed.
+    if (!Args.hasArg(OPT_plugin) && NeedsPlugin &&
+        Arg->getOption().hasFlag(PluginOnlyOption))
+      continue;
+
     Arg->render(Args, LinkerArgs);
     if (Arg->getOption().matches(OPT_o))
       llvm::transform(*FilesOrErr, std::back_inserter(LinkerArgs),
                       [&](StringRef Arg) { return Args.MakeArgString(Arg); });
   }
 
   // Run the host linking job with the rendered arguments.
-  StringRef LinkerPath = Args.getLastArgValue(OPT_linker_path_EQ);
   if (Error Err = runLinker(LinkerPath, LinkerArgs))
     reportError(std::move(Err));
 
   // Remove the temporary files created.
   if (!SaveTemps)
     for (const auto &TempFile : TempFiles)
       if (std::error_code EC = sys::fs::remove(TempFile))
         reportError(createFileError(TempFile, EC));
 
   return EXIT_SUCCESS;
 }

clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td

 include "llvm/Option/OptParser.td"
 
 def WrapperOnlyOption : OptionFlag;
 def DeviceOnlyOption : OptionFlag;
+def PluginOnlyOption : OptionFlag;
 
 def help : Flag<["--"], "help">,
   HelpText<"Display available options (--help-hidden for more)">;
 
 def help_hidden : Flag<["--"], "help-hidden">,
   HelpText<"Display all available options">;
 
 // Flags for the linker wrapper.
 def linker_path_EQ : Joined<["--"], "linker-path=">,
   Flags<[WrapperOnlyOption]>, MetaVarName<"<path>">,
   HelpText<"The linker executable to invoke">;
 def cuda_path_EQ : Joined<["--"], "cuda-path=">,
   Flags<[WrapperOnlyOption]>, MetaVarName<"<dir>">,
   HelpText<"Set the system CUDA path">;
 def host_triple_EQ : Joined<["--"], "host-triple=">,
   Flags<[WrapperOnlyOption]>, MetaVarName<"<triple>">,
   HelpText<"Triple to use for the host compilation">;
-def opt_level : Joined<["--"], "opt-level=">,
-  Flags<[WrapperOnlyOption]>, MetaVarName<"<O0, O1, O2, or O3>">,
-  HelpText<"Optimization level for LTO">;
 def bitcode_library_EQ : Joined<["--"], "bitcode-library=">,
   Flags<[WrapperOnlyOption]>, MetaVarName<"<kind>-<triple>-<arch>=<path>">,
   HelpText<"Extra bitcode library to link">;
 def device_linker_args_EQ : Joined<["--"], "device-linker=">,
   Flags<[WrapperOnlyOption]>, MetaVarName<"<value> or <triple>=<value>">,
   HelpText<"Arguments to pass to the device linker invocation">;
 def dry_run : Flag<["--"], "dry-run">,
   Flags<[WrapperOnlyOption]>,

 def rpath : Separate<["--", "-"], "rpath">;
 def rpath_EQ : Joined<["--", "-"], "rpath=">, Flags<[HelpHidden]>, Alias<rpath>;
 
 def dynamic_linker : Separate<["--", "-"], "dynamic-linker">;
 def dynamic_linker_EQ : Joined<["--", "-"], "dynamic-linker=">, Alias<dynamic_linker>;
 
 def v : Flag<["--", "-"], "v">, HelpText<"Display the version number and exit">;
 def version : Flag<["--", "-"], "version">, Flags<[HelpHidden]>, Alias<v>;
+
+// Standard flags passed to the plugin for host-LTO.
+def opt_level : Joined<["--", "-"], "plugin-opt=O">, Flags<[HelpHidden, PluginOnlyOption]>,
+  HelpText<"Optimization level for LTO">;
+
+// Sink all the other options here so we can ignore them if needed.
+def plugin_opt : Separate<["--", "-"], "plugin-opt">, Flags<[HelpHidden, PluginOnlyOption]>,
+  HelpText<"Options passed to the linker plugin">;
+def plugin_opt_eq : Joined<["--", "-"], "plugin-opt=">, Alias<plugin_opt>;
+def plugin_opt_eq_minus : Joined<["--", "-"], "plugin-opt=-">, Flags<[HelpHidden, PluginOnlyOption]>,
+  HelpText<"Options passed to LLVM">;
+
+// The precense of the plugin indicates if the host requires LTO as well.
+def plugin : Separate<["--", "-"], "plugin">, Flags<[HelpHidden]>,
+  HelpText<"The plugin passed to the linker">;