[PATCH] D43415: [ThinLTO] Always create linked objects file for --thinlto-index-only=

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added inline comments.


================
Comment at: llvm/include/llvm/LTO/LTO.h:219
+                              bool ShouldEmitImportsFiles,
+                              std::unique_ptr<raw_fd_ostream> LinkedObjectsFile,
+                              IndexWriteCallback OnWrite);
----------------
While here, can you add the missing documentation for this parameter?


================
Comment at: llvm/lib/LTO/LTO.cpp:1037
   bool ShouldEmitImportsFiles;
-
-  std::string LinkedObjectsFileName;
-  std::unique_ptr<llvm::raw_fd_ostream> LinkedObjectsFile;
-
+  std::shared_ptr<raw_fd_ostream> LinkedObjectsFile;
   lto::IndexWriteCallback OnWrite;
----------------
We only have one backend object - why use a shared_ptr instead of a unique_ptr?


================
Comment at: llvm/tools/gold/gold-plugin.cpp:739
 
+// Creates and return output stream with a list of object files for final
+// linking of distributed ThinLTO.
----------------
s/return/returns/


================
Comment at: llvm/tools/gold/gold-plugin.cpp:742
+static std::unique_ptr<raw_fd_ostream> CreateLinkedObjectsFile() {
+  if (!options::thinlto_linked_objects_file.empty()) {
+    std::error_code EC;
----------------
Suggest changing order to do early return of nullptr if filename is empty.


https://reviews.llvm.org/D43415