[PATCH] D46608: Add support for ThinLTO plugin option thinlto-object-suffix-replace

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

Could you explain what this option is? I cannot get what is expected for that option from this patch.



================
Comment at: lld/ELF/Driver.cpp:817
+      if (Config->ThinLTOObjectSuffixReplace.second.empty())
+        error("thinlto-object-suffix-replace expects 'old;new' format");
     } else if (!S.startswith("/") && !S.startswith("-fresolution=") &&
----------------
Please also add "but got" part.


================
Comment at: lld/ELF/InputFiles.cpp:1028
   // symbols later in the link stage).
+  llvm::StringRef Path = MB.getBufferIdentifier();
+  std::string UpdatedPath = Path.str();
----------------
Remove llvm::


================
Comment at: lld/ELF/LTO.cpp:269
 
-      std::string Path = getThinLTOOutputFile(F->getName());
-      std::unique_ptr<raw_fd_ostream> OS = openFile(Path + ".thinlto.bc");
+      llvm::StringRef Path = F->getName();
+      Path.consume_back(Config->ThinLTOObjectSuffixReplace.first);
----------------
Remove llvm::

(You probably should other code in the same file and write code in the same way as others.)


================
Comment at: lld/test/ELF/lto/thinlto_object_suffix_replace.ll:1
+; Test to make sure the thinlto-object-suffix-replace option is handled
+; correctly.
----------------
We use - instead of - in test file name.


================
Comment at: lld/test/ELF/lto/thinlto_object_suffix_replace.ll:18
+; RUN: rm -f %t1.o
+; RUN: ld.lld -m elf_x86_64 --plugin-opt=thinlto --plugin-opt=thinlto-index-only --plugin-opt=thinlto-object-suffix-replace=".thinlink.bc;.o" -shared %t1.thinlink.bc -o %t3
+; RUN: diff %t1.o.thinlto.bc.orig %t1.o.thinlto.bc
----------------
Please split a long line into shorter lines and concatenate them with \.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D46608