[PATCH] D46400: Add support for thinlto option to emit import files for thinlink

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/ELF/LTO.cpp:102
+  if (Config->ThinLTOEmitImportsFiles) {
+    raw_fd_ostream OS(NewModulePath + ".imports", EC,
+                      sys::fs::OpenFlags::F_None);
----------------
rdhindsa wrote:
> ruiu wrote:
> > I don't understand this part of code. I think OS will be closed at end of this `if` block, so I don't think you are writing anything to the file. Is this code correct?
> For all the bitcode files, empty import files are generated at first. For the files selected by the linker having summary, lto::createWriteIndexesThinBackend overwrites the imports files . However, for the files which are either not selected by linker or they don't have summary, we don't write anything to .imports file since they don't import anything. These files are required for distributed ThinLTO build file staging.
Got it. Let's factor it out as a separate function

  static void createEmptyFile(const Twine &Path);

that creates an empty file at a given path. That should make things clear.


================
Comment at: lld/ELF/LTO.cpp:105
+    if (EC)
+      error("failed to write " + (NewModulePath + ".imports") + ": " +
+            EC.message());
----------------
failed to write -> cannot open

for consistency with other error messages.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D46400