[PATCH] D79629: [Clang][Driver]Pass LLVM options to lld in case of LTO

[bin via Phabricator via cfe-commits]

bin.narwal added a comment.

In D79629#2026974 , @MaskRay wrote:

> `-mllvm,foobar` is for compilation (.c/.cc -> .o)
>
> `-Wl,-mllvm,foobar` for LTO options. For linking, `-mllvm,foobar` is not used 
> and thus warned.


Right, that's reasonable design.  The wrong UseLLD condition is a typo, anyway, 
I will discard this patch.

Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79629



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

[PATCH] D79629: [Clang][Driver]Pass LLVM options to lld in case of LTO

[Fangrui Song via Phabricator via cfe-commits]

MaskRay requested changes to this revision.
MaskRay added a comment.
This revision now requires changes to proceed.

`-mllvm,foobar` is for compilation (.c/.cc -> .o)

`-Wl,-mllvm,foobar` for LTO options. For linking, `-mllvm,foobar` is not used 
and thus warned.




Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:363
+ llvm::sys::path::stem(Linker) == "ld.lld");
+  if (UseLLD) {
 // Tell the linker to load the plugin. This has to come before

This is wrong. The intention is not to pass -plugin when the linker is clearly 
lld. lld does not need -plugin for LTO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79629



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

[PATCH] D79629: [Clang][Driver]Pass LLVM options to lld in case of LTO

[bin via Phabricator via cfe-commits]

bin.narwal created this revision.
bin.narwal added a reviewer: MaskRay.
bin.narwal added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith, inglorion.

Hi,
When debugging ThinLTO with clang -mllvm -debug-only=function-import -flto=thin 
-fuse-ld=lld ..., it gives me below warning message:
warning: argument unused during compilation: '-mllvm 
-debug-only=function-import' [-Wunused-command-line-argument]

Actually lld already supports passing arguments to llvm using -mllvm, it would 
be easier for debug if driver passes it along to lld.

This simple patch does this.  Note it also silently eats up such options if 
other linkers (ld.bfd, gold) are used.

Any comments?  Thanks


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79629

Files:
  clang/lib/Driver/ToolChains/CommonArgs.cpp


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -358,8 +358,9 @@
   ArgStringList , const InputInfo ,
   const InputInfo , bool IsThinLTO) {
   const char *Linker = Args.MakeArgString(ToolChain.GetLinkerPath());
-  if (llvm::sys::path::filename(Linker) != "ld.lld" &&
-  llvm::sys::path::stem(Linker) != "ld.lld") {
+  bool UseLLD = (llvm::sys::path::filename(Linker) == "ld.lld" ||
+ llvm::sys::path::stem(Linker) == "ld.lld");
+  if (UseLLD) {
 // 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.
@@ -381,6 +382,22 @@
 CmdArgs.push_back(Args.MakeArgString(Plugin));
   }
 
+  // Silence warning for "clang -mllvm ...".
+  for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
+A->claim();
+
+// Pass llvm options to lld.
+if (!UseLLD)
+  continue;
+
+// FIXME: This is now deprecated and should be removed.
+if (StringRef(A->getValue(0)) == "-disable-llvm-optzns") {
+  CmdArgs.push_back("-disable-llvm-optzns");
+} else {
+  A->render(Args, CmdArgs);
+}
+  }
+
   // Try to pass driver level flags relevant to LTO code generation down to
   // the plugin.
 


Index: clang/lib/Driver/ToolChains/CommonArgs.cpp
===
--- clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -358,8 +358,9 @@
   ArgStringList , const InputInfo ,
   const InputInfo , bool IsThinLTO) {
   const char *Linker = Args.MakeArgString(ToolChain.GetLinkerPath());
-  if (llvm::sys::path::filename(Linker) != "ld.lld" &&
-  llvm::sys::path::stem(Linker) != "ld.lld") {
+  bool UseLLD = (llvm::sys::path::filename(Linker) == "ld.lld" ||
+ llvm::sys::path::stem(Linker) == "ld.lld");
+  if (UseLLD) {
 // 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.
@@ -381,6 +382,22 @@
 CmdArgs.push_back(Args.MakeArgString(Plugin));
   }
 
+  // Silence warning for "clang -mllvm ...".
+  for (const Arg *A : Args.filtered(options::OPT_mllvm)) {
+A->claim();
+
+// Pass llvm options to lld.
+if (!UseLLD)
+  continue;
+
+// FIXME: This is now deprecated and should be removed.
+if (StringRef(A->getValue(0)) == "-disable-llvm-optzns") {
+  CmdArgs.push_back("-disable-llvm-optzns");
+} else {
+  A->render(Args, CmdArgs);
+}
+  }
+
   // Try to pass driver level flags relevant to LTO code generation down to
   // the plugin.
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits