[PATCH] D12309: [lld] COFF: Implement parallel LTO code generation.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: COFF/Driver.cpp:382
@@ +381,3 @@
+      StringRef Jobs = StringRef(S).substr(11);
+      if (Jobs.getAsInteger(10, Config->LTOJobs) || Config->LTOJobs == 0) {
+        error("/opt:lldltojobs: invalid job count: " + Jobs);
----------------
Remove { and }.

================
Comment at: COFF/SymbolTable.cpp:377
@@ +376,3 @@
+
+  for (auto Obj : Objs)
+    addCombinedLTOObject(Obj);
----------------
The type of Obj is not obvious from the context, so please use its real type.

================
Comment at: COFF/SymbolTable.cpp:377
@@ +376,3 @@
+
+  for (auto Obj : Objs)
+    addCombinedLTOObject(Obj);
----------------
ruiu wrote:
> The type of Obj is not obvious from the context, so please use its real type.
Please use the real type instead of "auto".

================
Comment at: COFF/SymbolTable.cpp:418
@@ +417,3 @@
+  Objs.resize(Config->LTOJobs);
+  std::list<raw_svector_ostream> OSs;
+  std::vector<raw_pwrite_stream *> OSPtrs;
----------------
Why std::list? (My understanding is std::list is generally slower than std::vector, and there seems no obvious reason to use that instead of std::vector here.)

================
Comment at: COFF/SymbolTable.cpp:420
@@ +419,3 @@
+  std::vector<raw_pwrite_stream *> OSPtrs;
+  for (auto &&Obj : Objs) {
+    OSs.emplace_back(Obj);
----------------
Ditto

================
Comment at: COFF/SymbolTable.cpp:420
@@ +419,3 @@
+  std::vector<raw_pwrite_stream *> OSPtrs;
+  for (auto &&Obj : Objs) {
+    OSs.emplace_back(Obj);
----------------
ruiu wrote:
> Ditto
Use real type.

================
Comment at: COFF/SymbolTable.cpp:429
@@ +428,3 @@
+  std::vector<ObjectFile *> ObjFiles;
+  for (auto &&Obj : Objs) {
+    auto *ObjFile = new ObjectFile(
----------------
Ditto

================
Comment at: COFF/SymbolTable.cpp:429
@@ +428,3 @@
+  std::vector<ObjectFile *> ObjFiles;
+  for (auto &&Obj : Objs) {
+    auto *ObjFile = new ObjectFile(
----------------
ruiu wrote:
> Ditto
Ditto

================
Comment at: COFF/SymbolTable.h:111
@@ -109,3 +110,3 @@
   std::vector<BitcodeFile *> BitcodeFiles;
-  std::unique_ptr<MemoryBuffer> LTOMB;
+  std::vector<SmallVector<char, 0>> Objs;
   llvm::BumpPtrAllocator Alloc;
----------------
Previously, LTOMB is a member of this class to keep the ownership of the memory buffer. Now this member owns nothing. Can we make this a function-local variable now?


http://reviews.llvm.org/D12309