[PATCH] D46034: Support for distributed ThinLTO options

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lld/ELF/Driver.cpp:806
+      if (!Config->ThinLTOPrefixReplace.contains(';'))
+        error("thinlto-prefix-replace expects 'old;new' format");
+    } else if (!S.startswith("/") && !S.startswith("-fresolution=") &&
----------------
Please provide the testcase for this error.


================
Comment at: lld/ELF/LTO.cpp:87
+  std::string NewModulePath =
+      llvm::lto::getThinLTOOutputFile(ModulePath, OldPrefix, NewPrefix);
+  std::error_code EC;
----------------
You do not need `llvm::` prefix I think.


================
Comment at: lld/ELF/LTO.cpp:93
+  if (EC)
+    error("Failed to write " + NewModulePath + ".thinlto.bc" + ": " +
+          EC.message());
----------------
Error messages start from the lower case usually.
Also, the test would be nice to have if possible (i think you could create the file and so this code should error out when try to overwrite it, but I am not sure).


================
Comment at: lld/ELF/LTO.cpp:99
+    Index.setSkipModuleByDistributedBackend();
+    llvm::WriteIndexToFile(Index, OS);
+  }
----------------
The same.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D46034