[PATCH] D17172: Minimum LTO implementation

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/Driver.cpp:297-300
@@ -292,1 +296,6 @@
 template <class ELFT> void LinkerDriver::link(opt::InputArgList &Args) {
+  // For LTO
+  InitializeAllTargets();
+  InitializeAllTargetMCs();
+  InitializeAllAsmPrinters();
+
----------------
Do you know the cost of this initialization? (I mean if it is not cheap, say if it takes more than 10 milliseconds, then we want to do that lazily.)

================
Comment at: ELF/InputFiles.cpp:443-446
@@ +442,6 @@
+    SmallString<64> Name;
+    {
+      raw_svector_ostream OS(Name);
+      Sym.printName(OS);
+    }
+    StringRef NameRef = Saver.save(StringRef(Name));
----------------
According to the documentation of raw_svector_ostream, it is guranteed that the output is not buffered, so you don't need to enclose it in a narrower scope.

================
Comment at: ELF/InputFiles.h:192
@@ +191,3 @@
+  llvm::BumpPtrAllocator Alloc;
+  llvm::StringSaver Saver{Alloc};
+};
----------------
Ah, this is a nice way of initializing a StringSaver.

================
Comment at: ELF/SymbolTable.cpp:83
@@ -76,1 +82,3 @@
 
+  if (auto *F = dyn_cast<BitcodeFile>(FileP)) {
+    BitcodeFiles.emplace_back(cast<BitcodeFile>(File.release()));
----------------
  // LLVM bitcode file

================
Comment at: ELF/SymbolTable.cpp:99
@@ -84,1 +98,3 @@
 
+static std::unique_ptr<InputFile> codegen(Module &M) {
+  StringRef TripleStr = M.getTargetTriple();
----------------
This function needs a comment.

================
Comment at: ELF/SymbolTable.cpp:116
@@ +115,3 @@
+
+  {
+    raw_svector_ostream OS(Driver->OwningLTOData);
----------------
You can remove this scope.

================
Comment at: ELF/SymbolTable.cpp:126
@@ +125,3 @@
+      MemoryBuffer::getMemBuffer(Driver->OwningLTOData, "", false);
+  Driver->OwningMBs.push_back(std::move(Buffer));
+  return createObjectFile(*Driver->OwningMBs.back());
----------------
So you included Driver.h just for this line? If so, we want to move this buffer somewhere else so that we don't need to include that header.

================
Comment at: ELF/SymbolTable.cpp:130
@@ +129,3 @@
+
+template <class ELFT>
+ObjectFile<ELFT> *SymbolTable<ELFT>::createCombinedLtoObject() {
----------------
This function needs a comment.


http://reviews.llvm.org/D17172