[PATCH] D46034: Support for distributed ThinLTO options

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: lld/ELF/InputFiles.h:262
   InputFile *fetch();
+  bool addedToLink();
+  bool isFileBitcode();
----------------
Instead of adding this function, can you give `Seen` a clearer name and make it public?


================
Comment at: lld/ELF/InputFiles.h:263
+  bool addedToLink();
+  bool isFileBitcode();
 
----------------
I don't think you need this function. You should be able to use `isBitcode` to test whether `MB->File` is bitcode directly in the caller.


================
Comment at: lld/ELF/LTO.cpp:85
+                                              const std::string &NewPrefix,
+                                              bool SkipModule) {
+  std::string NewModulePath =
----------------
This function is always called with `SkipModule` = true. There's also a case in the gold plugin where `SkipModule` is false (which just creates an empty file); that's the case where the file was added to the link but we end up not emitting an index for it. There's no easy way to detect this case using the current API, so probably the easiest thing to do would be to create the empty files in `BitcodeCompiler::add` and let them be overwritten later.


================
Comment at: lld/ELF/LTO.cpp:175
+BitcodeCompiler::BitcodeCompiler() {
+  if (Config->ThinLTOIndexOnly)
+    LinkedObjects = createLinkedObjectsFile();
----------------
I don't think you need the if.


================
Comment at: lld/ELF/LTO.cpp:196
+
+  if ((Config->ThinLTOIndexOnly) && (Obj.symbols().size() == 0)) {
+    std::string OldPrefix, NewPrefix;
----------------
This part doesn't seem right. We don't want to skip input files without symbols because they could contain section contributions that could be significant.


================
Comment at: lld/ELF/SymbolTable.cpp:132
 
+  // If LazyObjFile has not been added to link, emit empty index files
+  if (Config->ThinLTOIndexOnly) {
----------------
Consider moving this part to `BitcodeCompiler::compile()`.


================
Comment at: lld/ELF/SymbolTable.cpp:135
+    for (LazyObjFile *F : LazyObjFiles) {
+      if ((!F->addedToLink()) && (F->isFileBitcode()))
+        LTO->addLazyObjFile(F);
----------------
Remove unnecessary parens.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D46034