[PATCH] D46034: Support for distributed ThinLTO options

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Couple of post-commit suggestions for improvements below.



================
Comment at: lld/ELF/LTO.cpp:199
+  // Create the empty files which, if indexed, will be overwritten later.
+  writeEmptyDistributedBuildOutputs(Obj.getName(), OldPrefix, NewPrefix, false);
+
----------------
Sorry for not noticing this earlier - I was looking at the code again in the context of D46400. This seems inefficient - we have large links of tens of thousands of files, and writing empty index (and with D46400, empty imports) for each will slow down the thin link. In gold-plugin, only the necessary empty files are written. The createLTO function takes an optional callback function, which can be used to track which files get index files written for them. Take a look at how gold-plugin uses this to optionally call it's version of writeEmptyDistributedBuildOutputs (specifically, see how ObjectToIndexFileState is used). Can you do a follow-up change to implement the same support here so it is only called when needed?


================
Comment at: lld/ELF/LTO.cpp:302
+  // distributed environment.
+  if (Config->ThinLTOIndexOnly)
+    exit(0);
----------------
Can this be moved up above the cache pruning and the iteration over all the MaxTasks to create output files and save temps? Those should never be needed in the index-only case, and it would be more efficient to skip the MaxTasks Buff[I].empty() checks.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D46034