[PATCH] D13117: [DarwinDriver] Use -lto_library to specify the path for libLTO.dylib

[Bruno Cardoso Lopes via Phabricator via cfe-commits]

bruno closed this revision.
bruno added a comment.

Done way back in r248932


Repository:
  rL LLVM

https://reviews.llvm.org/D13117



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

Re: [PATCH] D13117: [DarwinDriver] Use -lto_library to specify the path for libLTO.dylib

[Bruno Cardoso Lopes via cfe-commits]

bruno updated this revision to Diff 35664.
bruno added a comment.

Added test per Duncan's comment.


Repository:
  rL LLVM

http://reviews.llvm.org/D13117

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/Tools.cpp
  test/Driver/darwin-ld.c

Index: test/Driver/darwin-ld.c
===
--- test/Driver/darwin-ld.c
+++ test/Driver/darwin-ld.c
@@ -110,6 +110,14 @@
 // LINK_OBJECT_LTO_PATH: {{ld(.exe)?"}}
 // LINK_OBJECT_LTO_PATH: "-object_path_lto"
 
+// RUN: %clang -target x86_64-apple-darwin10 -### %s \
+// RUN:   -mlinker-version=133 -flto 2> %t.log
+// RUN: cat %t.log
+// RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s < %t.log
+//
+// LINK_LTOLIB_PATH: {{ld(.exe)?"}}
+// LINK_LTOLIB_PATH: "-lto_library"
+
 // RUN: %clang -target x86_64-apple-darwin10 -### %t.o \
 // RUN:   -force_load a -force_load b 2> %t.log
 // RUN: cat %t.log
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -6501,15 +6501,34 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  // If we are using LTO, then automatically create a temporary file path for
-  // the linker to use, so that it's lifetime will extend past a possible
-  // dsymutil step.
-  if (Version[0] >= 116 && D.IsUsingLTO(Args) && NeedsTempPath(Inputs)) {
-const char *TmpPath = C.getArgs().MakeArgString(
-D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object)));
-C.addTempFile(TmpPath);
-CmdArgs.push_back("-object_path_lto");
-CmdArgs.push_back(TmpPath);
+  if (D.IsUsingLTO(Args)) {
+// If we are using LTO, then automatically create a temporary file path for
+// the linker to use, so that it's lifetime will extend past a possible
+// dsymutil step.
+if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
+  const char *TmpPath = C.getArgs().MakeArgString(
+  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  C.addTempFile(TmpPath);
+  CmdArgs.push_back("-object_path_lto");
+  CmdArgs.push_back(TmpPath);
+}
+
+// Use -lto_library option to specify the libLTO.dylib path. Try to find
+// it in clang installed libraries. If not found, the option is not used
+// and 'ld' will use its default mechanism to search for libLTO.dylib.
+if (Version[0] >= 133) {
+  // Search for libLTO in /../lib/libLTO.dylib
+  StringRef P = llvm::sys::path::parent_path(D.getInstalledDir());
+  SmallString<128> LibLTOPath(P);
+  llvm::sys::path::append(LibLTOPath, "lib");
+  llvm::sys::path::append(LibLTOPath, "libLTO.dylib");
+  if (llvm::sys::fs::exists(LibLTOPath)) {
+CmdArgs.push_back("-lto_library");
+CmdArgs.push_back(C.getArgs().MakeArgString(LibLTOPath));
+  } else {
+D.Diag(diag::warn_lto_libpath);
+  }
+}
   }
 
   // Derived from the "link" spec.
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -117,6 +117,7 @@
 def err_drv_no_neon_modifier : Error<"[no]neon is not accepted as modifier, 
please use [no]simd instead">;
 
 def warn_O4_is_O3 : Warning<"-O4 is equivalent to -O3">, InGroup;
+def warn_lto_libpath : Warning<"libLTO.dylib relative to clang installed dir 
not found; using 'ld' default search path instead">;
 def warn_drv_optimization_value : Warning<"optimization level '%0' is not 
supported; using '%1%2' instead">,
   InGroup;
 def warn_ignored_gcc_optimization : Warning<"optimization flag '%0' is not 
supported">,


Index: test/Driver/darwin-ld.c
===
--- test/Driver/darwin-ld.c
+++ test/Driver/darwin-ld.c
@@ -110,6 +110,14 @@
 // LINK_OBJECT_LTO_PATH: {{ld(.exe)?"}}
 // LINK_OBJECT_LTO_PATH: "-object_path_lto"
 
+// RUN: %clang -target x86_64-apple-darwin10 -### %s \
+// RUN:   -mlinker-version=133 -flto 2> %t.log
+// RUN: cat %t.log
+// RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s < %t.log
+//
+// LINK_LTOLIB_PATH: {{ld(.exe)?"}}
+// LINK_LTOLIB_PATH: "-lto_library"
+
 // RUN: %clang -target x86_64-apple-darwin10 -### %t.o \
 // RUN:   -force_load a -force_load b 2> %t.log
 // RUN: cat %t.log
Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -6501,15 +6501,34 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  // If we are using LTO, then automatically create a temporary file path for
-  // the linker to use, so that it's lifetime will extend past a possible
-  // dsymutil step.
-  if (Version[0] >= 116 && D.IsUsingLTO(Args) && NeedsTempPath(Inputs)) {
-

Re: [PATCH] D13117: [DarwinDriver] Use -lto_library to specify the path for libLTO.dylib

[Duncan P. N. Exon Smith via cfe-commits]

dexonsmith added a comment.

Sorry for the review fragmentation, but I just noticed there's no test
for the new warning.  Can you add one?  After that, LGTM.


Repository:
  rL LLVM

http://reviews.llvm.org/D13117



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

Re: [PATCH] D13117: [DarwinDriver] Use -lto_library to specify the path for libLTO.dylib

[Duncan P. N. Exon Smith via cfe-commits]

Sorry for the review fragmentation, but I just noticed there's no test
for the new warning.  Can you add one?  After that, LGTM.

> On 2015-Sep-24, at 11:57, Bruno Cardoso Lopes  wrote:
> 
> bruno updated this revision to Diff 35664.
> bruno added a comment.
> 
> Added test per Duncan's comment.
> 
> 
> Repository:
>  rL LLVM
> 
> http://reviews.llvm.org/D13117
> 
> Files:
>  include/clang/Basic/DiagnosticDriverKinds.td
>  lib/Driver/Tools.cpp
>  test/Driver/darwin-ld.c
> 
> Index: test/Driver/darwin-ld.c
> ===
> --- test/Driver/darwin-ld.c
> +++ test/Driver/darwin-ld.c
> @@ -110,6 +110,14 @@
> // LINK_OBJECT_LTO_PATH: {{ld(.exe)?"}}
> // LINK_OBJECT_LTO_PATH: "-object_path_lto"
> 
> +// RUN: %clang -target x86_64-apple-darwin10 -### %s \
> +// RUN:   -mlinker-version=133 -flto 2> %t.log
> +// RUN: cat %t.log
> +// RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s < %t.log
> +//
> +// LINK_LTOLIB_PATH: {{ld(.exe)?"}}
> +// LINK_LTOLIB_PATH: "-lto_library"
> +
> // RUN: %clang -target x86_64-apple-darwin10 -### %t.o \
> // RUN:   -force_load a -force_load b 2> %t.log
> // RUN: cat %t.log
> Index: lib/Driver/Tools.cpp
> ===
> --- lib/Driver/Tools.cpp
> +++ lib/Driver/Tools.cpp
> @@ -6501,15 +6501,34 @@
>options::OPT_fno_application_extension, false))
> CmdArgs.push_back("-application_extension");
> 
> -  // If we are using LTO, then automatically create a temporary file path for
> -  // the linker to use, so that it's lifetime will extend past a possible
> -  // dsymutil step.
> -  if (Version[0] >= 116 && D.IsUsingLTO(Args) && NeedsTempPath(Inputs)) {
> -const char *TmpPath = C.getArgs().MakeArgString(
> -D.GetTemporaryPath("cc", 
> types::getTypeTempSuffix(types::TY_Object)));
> -C.addTempFile(TmpPath);
> -CmdArgs.push_back("-object_path_lto");
> -CmdArgs.push_back(TmpPath);
> +  if (D.IsUsingLTO(Args)) {
> +// If we are using LTO, then automatically create a temporary file path 
> for
> +// the linker to use, so that it's lifetime will extend past a possible
> +// dsymutil step.
> +if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
> +  const char *TmpPath = C.getArgs().MakeArgString(
> +  D.GetTemporaryPath("cc", 
> types::getTypeTempSuffix(types::TY_Object)));
> +  C.addTempFile(TmpPath);
> +  CmdArgs.push_back("-object_path_lto");
> +  CmdArgs.push_back(TmpPath);
> +}
> +
> +// Use -lto_library option to specify the libLTO.dylib path. Try to find
> +// it in clang installed libraries. If not found, the option is not used
> +// and 'ld' will use its default mechanism to search for libLTO.dylib.
> +if (Version[0] >= 133) {
> +  // Search for libLTO in /../lib/libLTO.dylib
> +  StringRef P = llvm::sys::path::parent_path(D.getInstalledDir());
> +  SmallString<128> LibLTOPath(P);
> +  llvm::sys::path::append(LibLTOPath, "lib");
> +  llvm::sys::path::append(LibLTOPath, "libLTO.dylib");
> +  if (llvm::sys::fs::exists(LibLTOPath)) {
> +CmdArgs.push_back("-lto_library");
> +CmdArgs.push_back(C.getArgs().MakeArgString(LibLTOPath));
> +  } else {
> +D.Diag(diag::warn_lto_libpath);
> +  }
> +}
>   }
> 
>   // Derived from the "link" spec.
> Index: include/clang/Basic/DiagnosticDriverKinds.td
> ===
> --- include/clang/Basic/DiagnosticDriverKinds.td
> +++ include/clang/Basic/DiagnosticDriverKinds.td
> @@ -117,6 +117,7 @@
> def err_drv_no_neon_modifier : Error<"[no]neon is not accepted as modifier, 
> please use [no]simd instead">;
> 
> def warn_O4_is_O3 : Warning<"-O4 is equivalent to -O3">, InGroup;
> +def warn_lto_libpath : Warning<"libLTO.dylib relative to clang installed dir 
> not found; using 'ld' default search path instead">;

^ This warning.

> def warn_drv_optimization_value : Warning<"optimization level '%0' is not 
> supported; using '%1%2' instead">,
>   InGroup;
> def warn_ignored_gcc_optimization : Warning<"optimization flag '%0' is not 
> supported">,
> 
> 
> 

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

Re: [PATCH] D13117: [DarwinDriver] Use -lto_library to specify the path for libLTO.dylib

[Duncan P. N. Exon Smith via cfe-commits]

dexonsmith added a subscriber: dexonsmith.
dexonsmith added a comment.

The code LGTM, but there ought to be a way to add a LIT test for this.

Looking for -object_path_lto, I found this:
---

// RUN: %clang -target x86_64-apple-darwin10 -### %s \
// RUN:   -mlinker-version=117 -flto 2> %t.log
// RUN: cat %t.log
// RUN: FileCheck -check-prefix=LINK_OBJECT_LTO_PATH %s < %t.log
//
// LINK_OBJECT_LTO_PATH: {{ld(.exe)?"}}

// LINK_OBJECT_LTO_PATH: "-object_path_lto"
---

in test/Driver/darwin-ld.c.  Can you add a similar test there?


Repository:
  rL LLVM

http://reviews.llvm.org/D13117



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

Re: [PATCH] D13117: [DarwinDriver] Use -lto_library to specify the path for libLTO.dylib

[Duncan P. N. Exon Smith via cfe-commits]

The code LGTM, but there ought to be a way to add a LIT test for this.

Looking for -object_path_lto, I found this:
--
// RUN: %clang -target x86_64-apple-darwin10 -### %s \
// RUN:   -mlinker-version=117 -flto 2> %t.log
// RUN: cat %t.log
// RUN: FileCheck -check-prefix=LINK_OBJECT_LTO_PATH %s < %t.log
//
// LINK_OBJECT_LTO_PATH: {{ld(.exe)?"}}
// LINK_OBJECT_LTO_PATH: "-object_path_lto"
--
in test/Driver/darwin-ld.c.  Can you add a similar test there?

> On 2015-Sep-23, at 13:56, Bruno Cardoso Lopes  wrote:
> 
> bruno created this revision.
> bruno added a reviewer: dexonsmith.
> bruno added subscribers: cfe-commits, kledzik, bob.wilson.
> bruno set the repository for this revision to rL LLVM.
> 
> Usually, when using LTO with a clang installation newer than the system one, 
> there's a libLTO.dylib version mismatch and LTO fails. One solution to this 
> is to make ld point to the right libLTO.dylib by changing DYLD_LIBRARY_PATH.
> 
> However, ld64 supports specifying the complete path to the desired 
> libLTO.dylib through the -lto_library option. This patch adds support for the 
> clang driver to use this option whenever it's capable of finding a 
> libLTO.dylib in clang's installed library directory. This way, we don't need 
> to rely on DYLD_LIBRARY_PATH nor get caught by version mismatches.
> 
> Repository:
>  rL LLVM
> 
> http://reviews.llvm.org/D13117
> 
> Files:
>  include/clang/Basic/DiagnosticDriverKinds.td
>  lib/Driver/Tools.cpp
> 
> Index: lib/Driver/Tools.cpp
> ===
> --- lib/Driver/Tools.cpp
> +++ lib/Driver/Tools.cpp
> @@ -6501,15 +6501,34 @@
>options::OPT_fno_application_extension, false))
> CmdArgs.push_back("-application_extension");
> 
> -  // If we are using LTO, then automatically create a temporary file path for
> -  // the linker to use, so that it's lifetime will extend past a possible
> -  // dsymutil step.
> -  if (Version[0] >= 116 && D.IsUsingLTO(Args) && NeedsTempPath(Inputs)) {
> -const char *TmpPath = C.getArgs().MakeArgString(
> -D.GetTemporaryPath("cc", 
> types::getTypeTempSuffix(types::TY_Object)));
> -C.addTempFile(TmpPath);
> -CmdArgs.push_back("-object_path_lto");
> -CmdArgs.push_back(TmpPath);
> +  if (D.IsUsingLTO(Args)) {
> +// If we are using LTO, then automatically create a temporary file path 
> for
> +// the linker to use, so that it's lifetime will extend past a possible
> +// dsymutil step.
> +if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
> +  const char *TmpPath = C.getArgs().MakeArgString(
> +  D.GetTemporaryPath("cc", 
> types::getTypeTempSuffix(types::TY_Object)));
> +  C.addTempFile(TmpPath);
> +  CmdArgs.push_back("-object_path_lto");
> +  CmdArgs.push_back(TmpPath);
> +}
> +
> +// Use -lto_library option to specify the libLTO.dylib path. Try to find
> +// it in clang installed libraries. If not found, the option is not used
> +// and 'ld' will use its default mechanism to search for libLTO.dylib.
> +if (Version[0] >= 133) {
> +  // Search for libLTO in /../lib/libLTO.dylib
> +  StringRef P = llvm::sys::path::parent_path(D.getInstalledDir());
> +  SmallString<128> LibLTOPath(P);
> +  llvm::sys::path::append(LibLTOPath, "lib");
> +  llvm::sys::path::append(LibLTOPath, "libLTO.dylib");
> +  if (llvm::sys::fs::exists(LibLTOPath)) {
> +CmdArgs.push_back("-lto_library");
> +CmdArgs.push_back(C.getArgs().MakeArgString(LibLTOPath));
> +  } else {
> +D.Diag(diag::warn_lto_libpath);
> +  }
> +}
>   }
> 
>   // Derived from the "link" spec.
> Index: include/clang/Basic/DiagnosticDriverKinds.td
> ===
> --- include/clang/Basic/DiagnosticDriverKinds.td
> +++ include/clang/Basic/DiagnosticDriverKinds.td
> @@ -117,6 +117,7 @@
> def err_drv_no_neon_modifier : Error<"[no]neon is not accepted as modifier, 
> please use [no]simd instead">;
> 
> def warn_O4_is_O3 : Warning<"-O4 is equivalent to -O3">, InGroup;
> +def warn_lto_libpath : Warning<"libLTO.dylib relative to clang installed dir 
> not found; using 'ld' default search path instead">;
> def warn_drv_optimization_value : Warning<"optimization level '%0' is not 
> supported; using '%1%2' instead">,
>   InGroup;
> def warn_ignored_gcc_optimization : Warning<"optimization flag '%0' is not 
> supported">,
> 
> 
> 

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

[PATCH] D13117: [DarwinDriver] Use -lto_library to specify the path for libLTO.dylib

[Bruno Cardoso Lopes via cfe-commits]

bruno created this revision.
bruno added a reviewer: dexonsmith.
bruno added subscribers: cfe-commits, kledzik, bob.wilson.
bruno set the repository for this revision to rL LLVM.

Usually, when using LTO with a clang installation newer than the system one, 
there's a libLTO.dylib version mismatch and LTO fails. One solution to this is 
to make ld point to the right libLTO.dylib by changing DYLD_LIBRARY_PATH.

However, ld64 supports specifying the complete path to the desired libLTO.dylib 
through the -lto_library option. This patch adds support for the clang driver 
to use this option whenever it's capable of finding a libLTO.dylib in clang's 
installed library directory. This way, we don't need to rely on 
DYLD_LIBRARY_PATH nor get caught by version mismatches.

Repository:
  rL LLVM

http://reviews.llvm.org/D13117

Files:
  include/clang/Basic/DiagnosticDriverKinds.td
  lib/Driver/Tools.cpp

Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -6501,15 +6501,34 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  // If we are using LTO, then automatically create a temporary file path for
-  // the linker to use, so that it's lifetime will extend past a possible
-  // dsymutil step.
-  if (Version[0] >= 116 && D.IsUsingLTO(Args) && NeedsTempPath(Inputs)) {
-const char *TmpPath = C.getArgs().MakeArgString(
-D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object)));
-C.addTempFile(TmpPath);
-CmdArgs.push_back("-object_path_lto");
-CmdArgs.push_back(TmpPath);
+  if (D.IsUsingLTO(Args)) {
+// If we are using LTO, then automatically create a temporary file path for
+// the linker to use, so that it's lifetime will extend past a possible
+// dsymutil step.
+if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
+  const char *TmpPath = C.getArgs().MakeArgString(
+  D.GetTemporaryPath("cc", 
types::getTypeTempSuffix(types::TY_Object)));
+  C.addTempFile(TmpPath);
+  CmdArgs.push_back("-object_path_lto");
+  CmdArgs.push_back(TmpPath);
+}
+
+// Use -lto_library option to specify the libLTO.dylib path. Try to find
+// it in clang installed libraries. If not found, the option is not used
+// and 'ld' will use its default mechanism to search for libLTO.dylib.
+if (Version[0] >= 133) {
+  // Search for libLTO in /../lib/libLTO.dylib
+  StringRef P = llvm::sys::path::parent_path(D.getInstalledDir());
+  SmallString<128> LibLTOPath(P);
+  llvm::sys::path::append(LibLTOPath, "lib");
+  llvm::sys::path::append(LibLTOPath, "libLTO.dylib");
+  if (llvm::sys::fs::exists(LibLTOPath)) {
+CmdArgs.push_back("-lto_library");
+CmdArgs.push_back(C.getArgs().MakeArgString(LibLTOPath));
+  } else {
+D.Diag(diag::warn_lto_libpath);
+  }
+}
   }
 
   // Derived from the "link" spec.
Index: include/clang/Basic/DiagnosticDriverKinds.td
===
--- include/clang/Basic/DiagnosticDriverKinds.td
+++ include/clang/Basic/DiagnosticDriverKinds.td
@@ -117,6 +117,7 @@
 def err_drv_no_neon_modifier : Error<"[no]neon is not accepted as modifier, 
please use [no]simd instead">;
 
 def warn_O4_is_O3 : Warning<"-O4 is equivalent to -O3">, InGroup;
+def warn_lto_libpath : Warning<"libLTO.dylib relative to clang installed dir 
not found; using 'ld' default search path instead">;
 def warn_drv_optimization_value : Warning<"optimization level '%0' is not 
supported; using '%1%2' instead">,
   InGroup;
 def warn_ignored_gcc_optimization : Warning<"optimization flag '%0' is not 
supported">,


Index: lib/Driver/Tools.cpp
===
--- lib/Driver/Tools.cpp
+++ lib/Driver/Tools.cpp
@@ -6501,15 +6501,34 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  // If we are using LTO, then automatically create a temporary file path for
-  // the linker to use, so that it's lifetime will extend past a possible
-  // dsymutil step.
-  if (Version[0] >= 116 && D.IsUsingLTO(Args) && NeedsTempPath(Inputs)) {
-const char *TmpPath = C.getArgs().MakeArgString(
-D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object)));
-C.addTempFile(TmpPath);
-CmdArgs.push_back("-object_path_lto");
-CmdArgs.push_back(TmpPath);
+  if (D.IsUsingLTO(Args)) {
+// If we are using LTO, then automatically create a temporary file path for
+// the linker to use, so that it's lifetime will extend past a possible
+// dsymutil step.
+if (Version[0] >= 116 && NeedsTempPath(Inputs)) {
+  const char *TmpPath = C.getArgs().MakeArgString(
+  D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object)));
+