[PATCH] D138451: [lld/mac] Add support for distributed thinlto

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

Generally looks good. `lld/MachO/CMakeLists.txt` should add `BitWriter` to fix `-DBUILD_SHARED_LIBS=on` builds.



================
Comment at: lld/MachO/LTO.cpp:250
+  auto outputFilePath = [objPathIsDir](int i) {
+    SmallString<261> filePath("/tmp/lto.tmp");
+    if (!config->ltoObjPath.empty()) {
----------------
Why does this have `/tmp/`?


================
Comment at: lld/test/MachO/thinlto-index-only.ll:2
+; REQUIRES: x86
+; RUN: rm -rf %t; split-file %s %t
+
----------------
if you append `cd %t`, the `%t/` uses below can be removed.


================
Comment at: lld/test/MachO/thinlto-index-only.ll:10
+; RUN: ls %t/2.o.thinlto.bc
+; RUN: not test -e %t/%t/3
+; RUN: %lld -dylib %t/1.o %t/2.o -o %t/3
----------------
`not test -e %t/%t/3` this test is unneeded?


================
Comment at: lld/test/MachO/thinlto-index-only.ll:25
+
+;; Ensure lld generates an index even if the file is wrapped in --start-lib/--end-lib
+; RUN: rm -f %t/2.o.thinlto.bc %t/4
----------------



================
Comment at: lld/test/MachO/thinlto-no-index.ll:1
+; REQUIRES: x86
+; RUN: rm -rf %t; split-file %s %t
----------------
This test can be merged into thinlto-index-only.ll.

I'll remove test/ELF/lto/thinlto-no-index.ll


================
Comment at: lld/test/MachO/thinlto-no-index.ll:9
+
+; Ensure lld doesn't generates index files when thinlto-index-only is not enabled
+; RUN: rm -f %t1.o.thinlto.bc %t2.o.thinlto.bc %t3.o.thinlto.bc
----------------



================
Comment at: lld/test/MachO/thinlto-prefix-replace.ll:13
+
+;; Ensure that lld generates error if prefix replace option does not have 'old;new' format
+; RUN: rm -f %t/newpath/thinlto_prefix_replace.o.thinlto.bc
----------------



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

https://reviews.llvm.org/D138451