[PATCH] D127777: [ThinLTO][ELF] Add --thinlto-emit-index-files option

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: lld/test/ELF/lto/thinlto-emit-index.ll:39
+; Test that LLD generates an empty index even for lazy object file that is not added to link.
+; Test LLD generates empty imports file either because of thinlto-emit-imports-files option.
+; RUN: rm -f %t1.o.thinlto.bc
----------------
I don't follow this comment, and we shouldn't be generating any imports files with this option I believe? But comment on that later on - looks like that would be trivial to support.


================
Comment at: llvm/include/llvm/LTO/LTO.h:201
+using IndexWriteCallback = std::function<void(const std::string &)>;
+ThinBackend createInProcessThinBackend(ThreadPoolStrategy Parallelism,
+                                       IndexWriteCallback OnWrite = nullptr,
----------------
Document new parameters


================
Comment at: llvm/lib/LTO/LTO.cpp:1230
+      : ThinBackendProc(Conf, CombinedIndex, ModuleToDefinedGVSummaries,
+                        OnWrite, /*ShouldEmitImportsFiles*/ false),
         BackendThreadPool(ThinLTOParallelism), AddStream(std::move(AddStream)),
----------------
It seems pretty straightforward to support import file emission along with the index files - maybe just go ahead and wire it up?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127777