[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO
This revision was automatically updated to reflect the committed changes.
Closed by commit rC350970: [Darwin][Driver] Dont pass a file as
object_path_lto during ThinLTO (authored by steven_wu, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D56608?vs=181325=181361#toc
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56608/new/
https://reviews.llvm.org/D56608
Files:
include/clang/Driver/Driver.h
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Darwin.cpp
test/Driver/darwin-ld-lto.c
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
/// GCC goes to extra lengths here to be a bit more robust.
std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
+ /// GetTemporaryDirectory - Return the pathname of a temporary directory to
+ /// use as part of compilation; the directory will have the given prefix.
+ std::string GetTemporaryDirectory(StringRef Prefix) const;
+
/// Return the pathname of the pch file in clang-cl mode.
std::string GetClPchPath(Compilation , StringRef BaseName) const;
Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
// RUN: %clang -target x86_64-apple-darwin10 -### %s \
// RUN: -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
// RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN: FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto"
"{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=thin -### 2>&1 | \
+// RUN: FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// THIN_LTO_OBJECT_PATH: /usr/bin/ld
+// THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto"
"{{[a-zA-Z0-9_\/]+\/thinlto\-[a-zA-Z0-9_]+}}"
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
CmdArgs.push_back("-application_extension");
- if (D.isUsingLTO()) {
-// 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)));
+ if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+ // If we are using full 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.
+ TmpPathName =
+ D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+ // If we are using thin LTO, then create a directory instead.
+ TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+ auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
C.addTempFile(TmpPath);
CmdArgs.push_back("-object_path_lto");
CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
return Path.str();
}
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+ SmallString<128> Path;
+ std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+ if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+ }
+
+ return Path.str();
+}
+
std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
SmallString<128> Output;
if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
/// GCC goes to extra lengths here to be a bit more robust.
std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
+ /// GetTemporaryDirectory - Return the pathname of a temporary directory to
+ /// use as part of compilation; the directory will have the given prefix.
+ std::string GetTemporaryDirectory(StringRef Prefix) const;
+
/// Return
[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO
arphaman accepted this revision. arphaman added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56608/new/ https://reviews.llvm.org/D56608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO
steven_wu updated this revision to Diff 181325.
steven_wu added a comment.
Fix the comment
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56608/new/
https://reviews.llvm.org/D56608
Files:
include/clang/Driver/Driver.h
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Darwin.cpp
test/Driver/darwin-ld-lto.c
Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
// RUN: %clang -target x86_64-apple-darwin10 -### %s \
// RUN: -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
// RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN: FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto"
"{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=thin -### 2>&1 | \
+// RUN: FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// THIN_LTO_OBJECT_PATH: /usr/bin/ld
+// THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto"
"{{[a-zA-Z0-9_\/]+\/thinlto\-[a-zA-Z0-9_]+}}"
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
CmdArgs.push_back("-application_extension");
- if (D.isUsingLTO()) {
-// 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)));
+ if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+ // If we are using full 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.
+ TmpPathName =
+ D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+ // If we are using thin LTO, then create a directory instead.
+ TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+ auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
C.addTempFile(TmpPath);
CmdArgs.push_back("-object_path_lto");
CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
return Path.str();
}
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+ SmallString<128> Path;
+ std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+ if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+ }
+
+ return Path.str();
+}
+
std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
SmallString<128> Output;
if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
/// GCC goes to extra lengths here to be a bit more robust.
std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
+ /// GetTemporaryDirectory - Return the pathname of a temporary directory to
+ /// use as part of compilation; the directory will have the given prefix.
+ std::string GetTemporaryDirectory(StringRef Prefix) const;
+
/// Return the pathname of the pch file in clang-cl mode.
std::string GetClPchPath(Compilation , StringRef BaseName) const;
Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
// RUN: %clang -target x86_64-apple-darwin10 -### %s \
// RUN: -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
// RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN: FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto" "{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10 %s
[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO
arphaman added inline comments. Comment at: include/clang/Driver/Driver.h:508 + /// GetTemporaryPath - Return the pathname of a temporary directory to use + /// as part of compilation; the directory will have the given prefix. Old function name in the comment. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56608/new/ https://reviews.llvm.org/D56608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO
steven_wu updated this revision to Diff 181324.
steven_wu added a comment.
I was planning to add a test but I am not sure how to check the file type of
temporary files.
I add a test to check for temp file names because I do create file and
directory with different prefix.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56608/new/
https://reviews.llvm.org/D56608
Files:
include/clang/Driver/Driver.h
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Darwin.cpp
test/Driver/darwin-ld-lto.c
Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
// RUN: %clang -target x86_64-apple-darwin10 -### %s \
// RUN: -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
// RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN: FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: /usr/bin/ld
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto"
"{{[a-zA-Z0-9_\/]+\/cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=thin -### 2>&1 | \
+// RUN: FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// THIN_LTO_OBJECT_PATH: /usr/bin/ld
+// THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto"
"{{[a-zA-Z0-9_\/]+\/thinlto\-[a-zA-Z0-9_]+}}"
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
CmdArgs.push_back("-application_extension");
- if (D.isUsingLTO()) {
-// 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)));
+ if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+ // If we are using full 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.
+ TmpPathName =
+ D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+ // If we are using thin LTO, then create a directory instead.
+ TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+ auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
C.addTempFile(TmpPath);
CmdArgs.push_back("-object_path_lto");
CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
return Path.str();
}
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+ SmallString<128> Path;
+ std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+ if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+ }
+
+ return Path.str();
+}
+
std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
SmallString<128> Output;
if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
/// GCC goes to extra lengths here to be a bit more robust.
std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
+ /// GetTemporaryPath - Return the pathname of a temporary directory to use
+ /// as part of compilation; the directory will have the given prefix.
+ std::string GetTemporaryDirectory(StringRef Prefix) const;
+
/// Return the pathname of the pch file in clang-cl mode.
std::string GetClPchPath(Compilation , StringRef BaseName) const;
Index: test/Driver/darwin-ld-lto.c
===
--- test/Driver/darwin-ld-lto.c
+++ test/Driver/darwin-ld-lto.c
@@ -17,3 +17,14 @@
// RUN: %clang -target x86_64-apple-darwin10 -### %s \
// RUN: -ccc-install-dir %S/dummytestdir -mlinker-version=133 2> %t.log
// RUN: FileCheck -check-prefix=LINK_LTOLIB_PATH %s -input-file %t.log
+
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 | \
+// RUN: FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO
dexonsmith added a comment. The code looks good. Can you add a test too? Might need to require “shell”. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D56608/new/ https://reviews.llvm.org/D56608 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D56608: [Darwin][Driver] Don't pass a file as object_path_lto during ThinLTO
steven_wu created this revision.
steven_wu added reviewers: arphaman, dexonsmith.
Herald added subscribers: jkorous, inglorion, mehdi_amini.
After r327851, Driver::GetTemporaryPath will create the file rather than
just create a potientially unqine filename. If clang driver pass the
file as parameter as -object_path_lto, ld64 will pass it back to libLTO
as GeneratedObjectsDirectory, which is going to cause a LLVM ERROR if it
is not a directory.
Now during thinLTO, pass a temp directory path to linker instread.
rdar://problem/47194182
Repository:
rC Clang
https://reviews.llvm.org/D56608
Files:
include/clang/Driver/Driver.h
lib/Driver/Driver.cpp
lib/Driver/ToolChains/Darwin.cpp
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
CmdArgs.push_back("-application_extension");
- if (D.isUsingLTO()) {
-// 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)));
+ if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+ // If we are using full 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.
+ TmpPathName =
+ D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+ // If we are using thin LTO, then create a directory instead.
+ TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+ auto *TmpPath = C.getArgs().MakeArgString(TmpPathName);
C.addTempFile(TmpPath);
CmdArgs.push_back("-object_path_lto");
CmdArgs.push_back(TmpPath);
Index: lib/Driver/Driver.cpp
===
--- lib/Driver/Driver.cpp
+++ lib/Driver/Driver.cpp
@@ -4478,6 +4478,17 @@
return Path.str();
}
+std::string Driver::GetTemporaryDirectory(StringRef Prefix) const {
+ SmallString<128> Path;
+ std::error_code EC = llvm::sys::fs::createUniqueDirectory(Prefix, Path);
+ if (EC) {
+Diag(clang::diag::err_unable_to_make_temp) << EC.message();
+return "";
+ }
+
+ return Path.str();
+}
+
std::string Driver::GetClPchPath(Compilation , StringRef BaseName) const {
SmallString<128> Output;
if (Arg *FpArg = C.getArgs().getLastArg(options::OPT__SLASH_Fp)) {
Index: include/clang/Driver/Driver.h
===
--- include/clang/Driver/Driver.h
+++ include/clang/Driver/Driver.h
@@ -505,6 +505,10 @@
/// GCC goes to extra lengths here to be a bit more robust.
std::string GetTemporaryPath(StringRef Prefix, StringRef Suffix) const;
+ /// GetTemporaryPath - Return the pathname of a temporary directory to use
+ /// as part of compilation; the directory will have the given prefix.
+ std::string GetTemporaryDirectory(StringRef Prefix) const;
+
/// Return the pathname of the pch file in clang-cl mode.
std::string GetClPchPath(Compilation , StringRef BaseName) const;
Index: lib/Driver/ToolChains/Darwin.cpp
===
--- lib/Driver/ToolChains/Darwin.cpp
+++ lib/Driver/ToolChains/Darwin.cpp
@@ -224,13 +224,20 @@
options::OPT_fno_application_extension, false))
CmdArgs.push_back("-application_extension");
- if (D.isUsingLTO()) {
-// 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)));
+ if (D.isUsingLTO() && Version[0] >= 116 && NeedsTempPath(Inputs)) {
+std::string TmpPathName;
+if (D.getLTOMode() == LTOK_Full) {
+ // If we are using full 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.
+ TmpPathName =
+ D.GetTemporaryPath("cc", types::getTypeTempSuffix(types::TY_Object));
+} else if (D.getLTOMode() == LTOK_Thin)
+ // If we are using thin LTO, then create a directory instead.
+ TmpPathName = D.GetTemporaryDirectory("thinlto");
+
+if (!TmpPathName.empty()) {
+ auto *TmpPath =
