jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, MaskRay, tra, ABataev, 
yaxunl, tianshilei1992.
Herald added subscribers: StephenFan, inglorion.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

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.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129581

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/CommonArgs.h
  clang/test/Driver/linker-wrapper.c
  clang/test/Driver/openmp-offload-gpu-new.c
  clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
  clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td

Index: clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
===================================================================
--- clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
+++ clang/tools/clang-linker-wrapper/LinkerWrapperOpts.td
@@ -2,6 +2,7 @@
 
 def WrapperOnlyOption : OptionFlag;
 def DeviceOnlyOption : OptionFlag;
+def PluginOnlyOption : OptionFlag;
 
 def help : Flag<["--"], "help">,
   HelpText<"Display available options (--help-hidden for more)">;
@@ -19,9 +20,6 @@
 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">;
@@ -109,3 +107,18 @@
 
 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">;
Index: clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
===================================================================
--- clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
+++ clang/tools/clang-linker-wrapper/ClangLinkerWrapper.cpp
@@ -56,15 +56,6 @@
 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;
 
@@ -132,6 +123,7 @@
 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 {
@@ -436,7 +428,7 @@
     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)
@@ -445,7 +437,7 @@
     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))
@@ -797,10 +789,10 @@
   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();
@@ -1412,9 +1404,13 @@
     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);
@@ -1423,8 +1419,8 @@
   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))
@@ -1487,12 +1483,21 @@
   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),
@@ -1500,7 +1505,6 @@
   }
 
   // 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));
 
Index: clang/test/Driver/openmp-offload-gpu-new.c
===================================================================
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -131,3 +131,8 @@
 // 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"
Index: clang/test/Driver/linker-wrapper.c
===================================================================
--- clang/test/Driver/linker-wrapper.c
+++ clang/test/Driver/linker-wrapper.c
@@ -39,11 +39,17 @@
 // 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 \
Index: clang/lib/Driver/ToolChains/CommonArgs.h
===================================================================
--- clang/lib/Driver/ToolChains/CommonArgs.h
+++ clang/lib/Driver/ToolChains/CommonArgs.h
@@ -91,7 +91,8 @@
 
 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);
Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===================================================================
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -481,11 +481,12 @@
 
 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.
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -8418,25 +8418,9 @@
           "-" + 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()));
@@ -8452,15 +8436,6 @@
     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");
 
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D129581: [Clang] Rewo... Joseph Huber via Phabricator via cfe-commits

Reply via email to