[PATCH] D129581: [Clang] Rework LTO argument handling in the linker wrapper

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129581/new/

https://reviews.llvm.org/D129581

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D129581: [Clang] Rework LTO argument handling in the linker wrapper

[Joseph Huber via Phabricator via cfe-commits]

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<"">,
   HelpText<"Triple to use for the host compilation">;
-def opt_level : Joined<["--"], "opt-level=">,
-  Flags<[WrapperOnlyOption]>, MetaVarName<"">,
-  HelpText<"Optimization level for LTO">;
 def bitcode_library_EQ : Joined<["--"], "bitcode-library=">,
   Flags<[WrapperOnlyOption]>, MetaVarName<"--=">,
   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;
+
+// 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;
+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 Help("h", cl::desc("Alias for -help"), cl::Hidden,
-  cl::cat(ClangLinkerWrapperCategory));
-static cl::list
-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 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));