[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-17 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119612

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


[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-17 Thread Nico Weber via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG383ed82dd1f8: [clang] Pass more flags to ld64.lld (authored 
by thakis).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119612

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld-lto-lld.c
  clang/test/Driver/darwin-ld-lto.c
  clang/test/Driver/darwin-ld.c

Index: clang/test/Driver/darwin-ld.c
===
--- clang/test/Driver/darwin-ld.c
+++ clang/test/Driver/darwin-ld.c
@@ -236,6 +236,9 @@
 // RUN: %clang -target x86_64-apple-darwin12 -rdynamic -### %t.o \
 // RUN:   -fuse-ld= -mlinker-version=137 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_EXPORT_DYNAMIC %s < %t.log
+// RUN: %clang -target x86_64-apple-darwin12 -rdynamic -### %t.o \
+// RUN:   -fuse-ld=lld -B%S/Inputs/lld -mlinker-version=100 2> %t.log
+// RUN: FileCheck -check-prefix=LINK_EXPORT_DYNAMIC %s < %t.log
 // LINK_EXPORT_DYNAMIC: {{ld(.exe)?"}}
 // LINK_EXPORT_DYNAMIC: "-export_dynamic"
 
Index: clang/test/Driver/darwin-ld-lto.c
===
--- clang/test/Driver/darwin-ld-lto.c
+++ clang/test/Driver/darwin-ld-lto.c
@@ -20,13 +20,13 @@
 
 
 // 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
+// RUN: %clang -fuse-ld= -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 \
+// RUN: | FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
 // FULL_LTO_OBJECT_PATH: {{ld(.exe)?"}}
 // FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto"
 // FULL_LTO_OBJECT_PATH-SAME: {{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
+// RUN: %clang -fuse-ld= -target x86_64-apple-darwin10 %s -flto=thin -### 2>&1 \
+// RUN: | FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
 // THIN_LTO_OBJECT_PATH: {{ld(.exe)?"}}
 // THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto"
 // THIN_LTO_OBJECT_PATH-SAME: {{thinlto\-[a-zA-Z0-9_]+}}
Index: clang/test/Driver/darwin-ld-lto-lld.c
===
--- /dev/null
+++ clang/test/Driver/darwin-ld-lto-lld.c
@@ -0,0 +1,19 @@
+// REQUIRES: shell
+
+// Check that lld gets "-lto_library".
+// (Separate test file since darwin-ld-lto requires system-darwin but this
+// test doesn't require that.)
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -fuse-ld=lld -B%S/Inputs/lld -target x86_64-apple-darwin10 \
+// RUN: %s -flto=full -### 2>&1 \
+// RUN: | FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: {{ld(.exe)?"}}
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto"
+// FULL_LTO_OBJECT_PATH-SAME: {{cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -fuse-ld=lld -B%S/Inputs/lld -target x86_64-apple-darwin10 \
+// RUN: %s -flto=thin -### 2>&1 \
+// RUN: | FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// THIN_LTO_OBJECT_PATH: {{ld(.exe)?"}}
+// THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto"
+// THIN_LTO_OBJECT_PATH-SAME: {{thinlto\-[a-zA-Z0-9_]+}}
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -219,9 +219,8 @@
   !Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
 CmdArgs.push_back("-demangle");
 
-  // FIXME: Pass most of the flags below that check Version if LinkerIsLLD too.
-
-  if (Args.hasArg(options::OPT_rdynamic) && Version >= VersionTuple(137))
+  if (Args.hasArg(options::OPT_rdynamic) &&
+  (Version >= VersionTuple(137) || LinkerIsLLD))
 CmdArgs.push_back("-export_dynamic");
 
   // If we are using App Extension restrictions, pass a flag to the linker
@@ -230,7 +229,8 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO() && Version >= VersionTuple(116) && NeedsTempPath(Inputs)) {
+  if (D.isUsingLTO() && (Version >= VersionTuple(116) || LinkerIsLLD) &&
+  NeedsTempPath(Inputs)) {
 std::string TmpPathName;
 if (D.getLTOMode() == LTOK_Full) {
   // If we are using full LTO, then automatically create a temporary file
@@ -269,8 +269,11 @@
 CmdArgs.push_back(C.getArgs().MakeArgString(LibLTOPath));
   }
 
-  // ld64 version 262 and above run the deduplicate pass by default.
-  if (Version >= VersionTuple(262) && shouldLinkerNotDedup(C.getJobs().empty(), Args))
+  // ld64 version 262 and above runs the deduplicate pass by default.
+  // FIXME: lld doesn't dedup by default. Should we pass `--icf=safe`
+  //if `!shouldLinkerNotDedup()` if LinkerIsLLD here?
+  if (Version >= 

[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-17 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:272
 
-  // ld64 version 262 and above run the deduplicate pass by default.
-  if (Version >= VersionTuple(262) && 
shouldLinkerNotDedup(C.getJobs().empty(), Args))
+  // ld64 version 262 and above and lld run the deduplicate pass by default.
+  if ((Version >= VersionTuple(262) || LinkerIsLLD) &&

int3 wrote:
> thakis wrote:
> > MaskRay wrote:
> > > lld runs `--icf=none` (i.e. `-no_deduplicate`) by default.
> > Hm, good point. Should we pass `--icf=safe` for lld if 
> > `!shouldLinkerNotDedup` here then?
> I think our dedup pass is both a lot more effective and a lot more expensive 
> link-time-wise than ld64's so maybe we shouldn't do it by default...
> FIXME: lld doesn't dedup by default.

I agree that we should not do icf by default. It can easily take more than 10% 
time IIRC.


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

https://reviews.llvm.org/D119612

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


[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-17 Thread Jez Ng via Phabricator via cfe-commits
int3 accepted this revision.
int3 added a comment.
This revision is now accepted and ready to land.

lgtm!




Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:272
 
-  // ld64 version 262 and above run the deduplicate pass by default.
-  if (Version >= VersionTuple(262) && 
shouldLinkerNotDedup(C.getJobs().empty(), Args))
+  // ld64 version 262 and above and lld run the deduplicate pass by default.
+  if ((Version >= VersionTuple(262) || LinkerIsLLD) &&

thakis wrote:
> MaskRay wrote:
> > lld runs `--icf=none` (i.e. `-no_deduplicate`) by default.
> Hm, good point. Should we pass `--icf=safe` for lld if 
> `!shouldLinkerNotDedup` here then?
I think our dedup pass is both a lot more effective and a lot more expensive 
link-time-wise than ld64's so maybe we shouldn't do it by default...


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

https://reviews.llvm.org/D119612

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


[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-17 Thread Nico Weber via Phabricator via cfe-commits
thakis added a comment.

lld-macho folks, any concerns with this?


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

https://reviews.llvm.org/D119612

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


[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-16 Thread Nico Weber via Phabricator via cfe-commits
thakis updated this revision to Diff 409254.
thakis added a comment.

Pass `-B` with `-fuse-ld=lld` since the latter flag only has an effect if 
ld64.lld exists on disk, and clang/test doesn't depend on lld (and shouldn't).

Also move object_path_lto to its own file since it can run fine on non-darwin 
hosts.


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

https://reviews.llvm.org/D119612

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld-lto-lld.c
  clang/test/Driver/darwin-ld-lto.c
  clang/test/Driver/darwin-ld.c

Index: clang/test/Driver/darwin-ld.c
===
--- clang/test/Driver/darwin-ld.c
+++ clang/test/Driver/darwin-ld.c
@@ -236,6 +236,9 @@
 // RUN: %clang -target x86_64-apple-darwin12 -rdynamic -### %t.o \
 // RUN:   -fuse-ld= -mlinker-version=137 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_EXPORT_DYNAMIC %s < %t.log
+// RUN: %clang -target x86_64-apple-darwin12 -rdynamic -### %t.o \
+// RUN:   -fuse-ld=lld -B%S/Inputs/lld -mlinker-version=100 2> %t.log
+// RUN: FileCheck -check-prefix=LINK_EXPORT_DYNAMIC %s < %t.log
 // LINK_EXPORT_DYNAMIC: {{ld(.exe)?"}}
 // LINK_EXPORT_DYNAMIC: "-export_dynamic"
 
Index: clang/test/Driver/darwin-ld-lto.c
===
--- clang/test/Driver/darwin-ld-lto.c
+++ clang/test/Driver/darwin-ld-lto.c
@@ -20,13 +20,13 @@
 
 
 // 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
+// RUN: %clang -fuse-ld= -target x86_64-apple-darwin10 %s -flto=full -### 2>&1 \
+// RUN: | FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
 // FULL_LTO_OBJECT_PATH: {{ld(.exe)?"}}
 // FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto"
 // FULL_LTO_OBJECT_PATH-SAME: {{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
+// RUN: %clang -fuse-ld= -target x86_64-apple-darwin10 %s -flto=thin -### 2>&1 \
+// RUN: | FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
 // THIN_LTO_OBJECT_PATH: {{ld(.exe)?"}}
 // THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto"
 // THIN_LTO_OBJECT_PATH-SAME: {{thinlto\-[a-zA-Z0-9_]+}}
Index: clang/test/Driver/darwin-ld-lto-lld.c
===
--- /dev/null
+++ clang/test/Driver/darwin-ld-lto-lld.c
@@ -0,0 +1,19 @@
+// REQUIRES: shell
+
+// Check that lld gets "-lto_library".
+// (Separate test file since darwin-ld-lto requires system-darwin but this
+// test doesn't require that.)
+
+// Check that -object_lto_path is passed correctly to ld64
+// RUN: %clang -fuse-ld=lld -B%S/Inputs/lld -target x86_64-apple-darwin10 \
+// RUN: %s -flto=full -### 2>&1 \
+// RUN: | FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// FULL_LTO_OBJECT_PATH: {{ld(.exe)?"}}
+// FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto"
+// FULL_LTO_OBJECT_PATH-SAME: {{cc\-[a-zA-Z0-9_]+.o}}"
+// RUN: %clang -fuse-ld=lld -B%S/Inputs/lld -target x86_64-apple-darwin10 \
+// RUN: %s -flto=thin -### 2>&1 \
+// RUN: | FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// THIN_LTO_OBJECT_PATH: {{ld(.exe)?"}}
+// THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto"
+// THIN_LTO_OBJECT_PATH-SAME: {{thinlto\-[a-zA-Z0-9_]+}}
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -219,9 +219,8 @@
   !Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
 CmdArgs.push_back("-demangle");
 
-  // FIXME: Pass most of the flags below that check Version if LinkerIsLLD too.
-
-  if (Args.hasArg(options::OPT_rdynamic) && Version >= VersionTuple(137))
+  if (Args.hasArg(options::OPT_rdynamic) &&
+  (Version >= VersionTuple(137) || LinkerIsLLD))
 CmdArgs.push_back("-export_dynamic");
 
   // If we are using App Extension restrictions, pass a flag to the linker
@@ -230,7 +229,8 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO() && Version >= VersionTuple(116) && NeedsTempPath(Inputs)) {
+  if (D.isUsingLTO() && (Version >= VersionTuple(116) || LinkerIsLLD) &&
+  NeedsTempPath(Inputs)) {
 std::string TmpPathName;
 if (D.getLTOMode() == LTOK_Full) {
   // If we are using full LTO, then automatically create a temporary file
@@ -269,8 +269,11 @@
 CmdArgs.push_back(C.getArgs().MakeArgString(LibLTOPath));
   }
 
-  // ld64 version 262 and above run the deduplicate pass by default.
-  if (Version >= VersionTuple(262) && shouldLinkerNotDedup(C.getJobs().empty(), Args))
+  // ld64 version 262 and above runs the deduplicate pass by default.
+  // FIXME: lld doesn't dedup by default. Should we pass `--icf=safe`
+  

[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-16 Thread Nico Weber via Phabricator via cfe-commits
thakis updated this revision to Diff 409243.
thakis added a comment.

undo no_deduplicate bit again for now


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

https://reviews.llvm.org/D119612

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld-lto.c
  clang/test/Driver/darwin-ld.c


Index: clang/test/Driver/darwin-ld.c
===
--- clang/test/Driver/darwin-ld.c
+++ clang/test/Driver/darwin-ld.c
@@ -236,6 +236,9 @@
 // RUN: %clang -target x86_64-apple-darwin12 -rdynamic -### %t.o \
 // RUN:   -fuse-ld= -mlinker-version=137 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_EXPORT_DYNAMIC %s < %t.log
+// RUN: %clang -target x86_64-apple-darwin12 -rdynamic -### %t.o \
+// RUN:   -fuse-ld=lld -mlinker-version=100 2> %t.log
+// RUN: FileCheck -check-prefix=LINK_EXPORT_DYNAMIC %s < %t.log
 // LINK_EXPORT_DYNAMIC: {{ld(.exe)?"}}
 // LINK_EXPORT_DYNAMIC: "-export_dynamic"
 
Index: clang/test/Driver/darwin-ld-lto.c
===
--- clang/test/Driver/darwin-ld-lto.c
+++ clang/test/Driver/darwin-ld-lto.c
@@ -20,13 +20,17 @@
 
 
 // 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
+// RUN: %clang -fuse-ld= -target x86_64-apple-darwin10 %s -flto=full -### \
+// RUN: 2>&1 | FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// RUN: %clang -fuse-ld=lld -target x86_64-apple-darwin10 %s -flto=full -### \
+// RUN: 2>&1 | FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
 // FULL_LTO_OBJECT_PATH: {{ld(.exe)?"}}
 // FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto"
 // FULL_LTO_OBJECT_PATH-SAME: {{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
+// RUN: %clang -fuse-ld= -target x86_64-apple-darwin10 %s -flto=thin -### \
+// RUN: 2>&1 | FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// RUN: %clang -fuse-ld=lld -target x86_64-apple-darwin10 %s -flto=thin -### \
+// RUN: 2>&1 | FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
 // THIN_LTO_OBJECT_PATH: {{ld(.exe)?"}}
 // THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto"
 // THIN_LTO_OBJECT_PATH-SAME: {{thinlto\-[a-zA-Z0-9_]+}}
Index: clang/lib/Driver/ToolChains/Darwin.cpp
===
--- clang/lib/Driver/ToolChains/Darwin.cpp
+++ clang/lib/Driver/ToolChains/Darwin.cpp
@@ -219,9 +219,8 @@
   !Args.hasArg(options::OPT_Z_Xlinker__no_demangle))
 CmdArgs.push_back("-demangle");
 
-  // FIXME: Pass most of the flags below that check Version if LinkerIsLLD too.
-
-  if (Args.hasArg(options::OPT_rdynamic) && Version >= VersionTuple(137))
+  if (Args.hasArg(options::OPT_rdynamic) &&
+  (Version >= VersionTuple(137) || LinkerIsLLD))
 CmdArgs.push_back("-export_dynamic");
 
   // If we are using App Extension restrictions, pass a flag to the linker
@@ -230,7 +229,8 @@
options::OPT_fno_application_extension, false))
 CmdArgs.push_back("-application_extension");
 
-  if (D.isUsingLTO() && Version >= VersionTuple(116) && NeedsTempPath(Inputs)) 
{
+  if (D.isUsingLTO() && (Version >= VersionTuple(116) || LinkerIsLLD) &&
+  NeedsTempPath(Inputs)) {
 std::string TmpPathName;
 if (D.getLTOMode() == LTOK_Full) {
   // If we are using full LTO, then automatically create a temporary file
@@ -269,8 +269,11 @@
 CmdArgs.push_back(C.getArgs().MakeArgString(LibLTOPath));
   }
 
-  // ld64 version 262 and above run the deduplicate pass by default.
-  if (Version >= VersionTuple(262) && 
shouldLinkerNotDedup(C.getJobs().empty(), Args))
+  // ld64 version 262 and above runs the deduplicate pass by default.
+  // FIXME: lld doesn't dedup by default. Should we pass `--icf=safe`
+  //if `!shouldLinkerNotDedup()` if LinkerIsLLD here?
+  if (Version >= VersionTuple(262) &&
+  shouldLinkerNotDedup(C.getJobs().empty(), Args))
 CmdArgs.push_back("-no_deduplicate");
 
   // Derived from the "link" spec.
@@ -368,6 +371,7 @@
 // Check if the toolchain supports bitcode build flow.
 if (MachOTC.SupportsEmbeddedBitcode()) {
   CmdArgs.push_back("-bitcode_bundle");
+  // FIXME: Pass this if LinkerIsLLD too, once it implements this flag.
   if (C.getDriver().embedBitcodeMarkerOnly() &&
   Version >= VersionTuple(278)) {
 CmdArgs.push_back("-bitcode_process_mode");


Index: clang/test/Driver/darwin-ld.c
===
--- clang/test/Driver/darwin-ld.c
+++ clang/test/Driver/darwin-ld.c
@@ -236,6 +236,9 @@
 // RUN: %clang -target x86_64-apple-darwin12 -rdynamic -### %t.o \
 // RUN:   -fuse-ld= -mlinker-version=137 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_EXPORT_DYNAMIC %s < %t.log
+// RUN: %clang 

[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-16 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:272
 
-  // ld64 version 262 and above run the deduplicate pass by default.
-  if (Version >= VersionTuple(262) && 
shouldLinkerNotDedup(C.getJobs().empty(), Args))
+  // ld64 version 262 and above and lld run the deduplicate pass by default.
+  if ((Version >= VersionTuple(262) || LinkerIsLLD) &&

MaskRay wrote:
> lld runs `--icf=none` (i.e. `-no_deduplicate`) by default.
Hm, good point. Should we pass `--icf=safe` for lld if `!shouldLinkerNotDedup` 
here then?



Comment at: clang/test/Driver/darwin-ld-dedup.c:4
+// -no_deduplicate is only present from ld64 version 262 and later, or lld.
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld= %s \
 // RUN:   -mlinker-version=261 -O0 2>&1 | FileCheck -check-prefix=LINK_DEDUP %s

MaskRay wrote:
> If you are going to change the command line, prefer the canonical `--target=` 
> to the legacy `-target `.
"legacy" really has no semantic meaning here other than "I like that other 
spelling more". It's not like we're ever going to remove `-target`, it's way 
too actively used (see e.g. D119446 :P).

It seems like a change unrelated to what this patch is doing. If we want to 
change it, we can do it in a separate patch.


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

https://reviews.llvm.org/D119612

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


[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/test/Driver/darwin-ld-dedup.c:4
+// -no_deduplicate is only present from ld64 version 262 and later, or lld.
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld= %s \
 // RUN:   -mlinker-version=261 -O0 2>&1 | FileCheck -check-prefix=LINK_DEDUP %s

If you are going to change the command line, prefer the canonical `--target=` 
to the legacy `-target `.


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

https://reviews.llvm.org/D119612

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


[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-14 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:272
 
-  // ld64 version 262 and above run the deduplicate pass by default.
-  if (Version >= VersionTuple(262) && 
shouldLinkerNotDedup(C.getJobs().empty(), Args))
+  // ld64 version 262 and above and lld run the deduplicate pass by default.
+  if ((Version >= VersionTuple(262) || LinkerIsLLD) &&

lld runs `--icf=none` (i.e. `-no_deduplicate`) by default.


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

https://reviews.llvm.org/D119612

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


[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-14 Thread Leonard Grey via Phabricator via cfe-commits
lgrey added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:242
 } else if (D.getLTOMode() == LTOK_Thin)
   // If we are using thin LTO, then create a directory instead.
   TmpPathName = D.GetTemporaryDirectory("thinlto");

thakis wrote:
> lgrey: Do you think this should interact with 
> 134275d994d5fb38edfeb587ba45c8f495c8bf66 in any way, or should have any other 
> interesting side effects?
> 
> From what I understand, it tells the linker to put temporary files created 
> for LTO in the given directory, but then the clang driver cleans up that 
> directory when it exits (potentially after running dsymutil). But most people 
> run dsymutil later, separately, so I think there should be no interactions 
> there.
IIUC this is orthogonal to the cache's temp files. Something's actually not 
adding up here for me (why was it looking at the LTO cache files in the first 
place?), but this change shouldn't affect the cache thing.


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

https://reviews.llvm.org/D119612

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


[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-11 Thread Nico Weber via Phabricator via cfe-commits
thakis added inline comments.



Comment at: clang/lib/Driver/ToolChains/Darwin.cpp:242
 } else if (D.getLTOMode() == LTOK_Thin)
   // If we are using thin LTO, then create a directory instead.
   TmpPathName = D.GetTemporaryDirectory("thinlto");

lgrey: Do you think this should interact with 
134275d994d5fb38edfeb587ba45c8f495c8bf66 in any way, or should have any other 
interesting side effects?

From what I understand, it tells the linker to put temporary files created for 
LTO in the given directory, but then the clang driver cleans up that directory 
when it exits (potentially after running dsymutil). But most people run 
dsymutil later, separately, so I think there should be no interactions there.


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

https://reviews.llvm.org/D119612

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


[PATCH] D119612: [clang] Pass more flags to ld64.lld

2022-02-11 Thread Nico Weber via Phabricator via cfe-commits
thakis created this revision.
thakis added a reviewer: lld-macho.
Herald added subscribers: ormris, steven_wu, hiraditya.
thakis requested review of this revision.

- ld64.lld now completely supports -export_dynamic (D119372 
), so map -rdynamic to -export_dynamic like 
already done for ld64

- ld64.lld has been supporting -object_path_lto for well over a year (D92537 
), so pass it like already done for ld64

- ld64.lld has been doing ICF for a while, so pass -no_deduplicate in -O0 and 
-O1 builds like already done for ld64


https://reviews.llvm.org/D119612

Files:
  clang/lib/Driver/ToolChains/Darwin.cpp
  clang/test/Driver/darwin-ld-dedup.c
  clang/test/Driver/darwin-ld-lto.c
  clang/test/Driver/darwin-ld.c

Index: clang/test/Driver/darwin-ld.c
===
--- clang/test/Driver/darwin-ld.c
+++ clang/test/Driver/darwin-ld.c
@@ -235,6 +235,8 @@
 
 // RUN: %clang -target x86_64-apple-darwin12 -rdynamic -### %t.o \
 // RUN:   -fuse-ld= -mlinker-version=137 2> %t.log
+// RUN: %clang -target x86_64-apple-darwin12 -rdynamic -### %t.o \
+// RUN:   -fuse-ld=lld -mlinker-version=100 2> %t.log
 // RUN: FileCheck -check-prefix=LINK_EXPORT_DYNAMIC %s < %t.log
 // LINK_EXPORT_DYNAMIC: {{ld(.exe)?"}}
 // LINK_EXPORT_DYNAMIC: "-export_dynamic"
Index: clang/test/Driver/darwin-ld-lto.c
===
--- clang/test/Driver/darwin-ld-lto.c
+++ clang/test/Driver/darwin-ld-lto.c
@@ -20,13 +20,17 @@
 
 
 // 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
+// RUN: %clang -fuse-ld= -target x86_64-apple-darwin10 %s -flto=full -### \
+// RUN: 2>&1 | FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
+// RUN: %clang -fuse-ld=lld -target x86_64-apple-darwin10 %s -flto=full -### \
+// RUN: 2>&1 | FileCheck -check-prefix=FULL_LTO_OBJECT_PATH %s
 // FULL_LTO_OBJECT_PATH: {{ld(.exe)?"}}
 // FULL_LTO_OBJECT_PATH-SAME: "-object_path_lto"
 // FULL_LTO_OBJECT_PATH-SAME: {{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
+// RUN: %clang -fuse-ld= -target x86_64-apple-darwin10 %s -flto=thin -### \
+// RUN: 2>&1 | FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
+// RUN: %clang -fuse-ld=lld -target x86_64-apple-darwin10 %s -flto=thin -### \
+// RUN: 2>&1 | FileCheck -check-prefix=THIN_LTO_OBJECT_PATH %s
 // THIN_LTO_OBJECT_PATH: {{ld(.exe)?"}}
 // THIN_LTO_OBJECT_PATH-SAME: "-object_path_lto"
 // THIN_LTO_OBJECT_PATH-SAME: {{thinlto\-[a-zA-Z0-9_]+}}
Index: clang/test/Driver/darwin-ld-dedup.c
===
--- clang/test/Driver/darwin-ld-dedup.c
+++ clang/test/Driver/darwin-ld-dedup.c
@@ -1,42 +1,58 @@
 // REQUIRES: system-darwin
 
-// -no_deduplicate is only present from ld64 version 262 and later.
-// RUN: %clang -target x86_64-apple-darwin10 -### %s \
+// -no_deduplicate is only present from ld64 version 262 and later, or lld.
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld= %s \
 // RUN:   -mlinker-version=261 -O0 2>&1 | FileCheck -check-prefix=LINK_DEDUP %s
 
 // Add -no_deduplicate when either -O0 or -O1 is explicitly specified
-// RUN: %clang -target x86_64-apple-darwin10 -### %s \
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld= %s \
 // RUN:   -mlinker-version=262 -O0 2>&1 | FileCheck -check-prefix=LINK_NODEDUP %s
-// RUN: %clang -target x86_64-apple-darwin10 -### %s \
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld=lld %s \
+// RUN:   -mlinker-version=261 -O0 2>&1 | FileCheck -check-prefix=LINK_NODEDUP %s
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld= %s \
 // RUN:   -mlinker-version=262 -O1 2>&1 | FileCheck -check-prefix=LINK_NODEDUP %s
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld=lld %s \
+// RUN:   -mlinker-version=261 -O1 2>&1 | FileCheck -check-prefix=LINK_NODEDUP %s
 
-// RUN: %clang -target x86_64-apple-darwin10 -### %s \
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld= %s \
 // RUN:   -mlinker-version=262 -O2 2>&1 | FileCheck -check-prefix=LINK_DEDUP %s
-// RUN: %clang -target x86_64-apple-darwin10 -### %s \
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld=lld %s \
+// RUN:   -mlinker-version=262 -O2 2>&1 | FileCheck -check-prefix=LINK_DEDUP %s
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld= %s \
 // RUN:   -mlinker-version=262 -O3 2>&1 | FileCheck -check-prefix=LINK_DEDUP %s
-// RUN: %clang -target x86_64-apple-darwin10 -### %s \
+// RUN: %clang -target x86_64-apple-darwin10 -### -fuse-ld= %s \
 // RUN:   -mlinker-version=262 -Os 2>&1 | FileCheck -check-prefix=LINK_DEDUP %s
-// RUN: %clang -target x86_64-apple-darwin10 -### %s \
+// RUN: