[PATCH] D15390: [ThinLTO] Launch importing backends in parallel threads from gold plugin

[Rafael Ávila de Espíndola via llvm-commits]

rafael added inline comments.

================
Comment at: tools/gold/gold-plugin.cpp:891
@@ +890,3 @@
+  // Run backend threads.
+  splitCodeGen(std::move(M), OSPtrs.vec(), options::mcpu, Features.getString(),
+               Options, RelocationModel, CodeModel::Default, CGOptLevel);
----------------
splitCodeGen can take a ArrayRef too. Why do you need the vec?


================
Comment at: tools/gold/gold-plugin.cpp:975
@@ +974,3 @@
+
+TaskInfo::~TaskInfo() {
+  // Close the output file descriptor before we pass it to gold.
----------------
It seems odd how much work the destructor of TaskInfo is doing.

Most of the work is here because gold is not thread safe, correct? It so, it seems better to write this code explicitly after

  ThinLTOThreadPool.wait();


================
Comment at: tools/gold/gold-plugin.cpp:998
@@ +997,3 @@
+
+  std::vector<claimed_file *> Worklist;
+  for (claimed_file &F : Modules)
----------------
Why do you need a worklist?

Can't you just just a simple loop over Modules?

================
Comment at: tools/gold/gold-plugin.cpp:1033
@@ +1032,3 @@
+        std::move(InputFile), std::move(OS), NewFilename.c_str(), TempOutFile);
+    Tasks.emplace_back(std::move(Task));
+  }
----------------
This can be just

    Tasks.emplace_back(new TaskInfo(std::move(InputFile), std::move(OS),
                                    NewFilename.c_str(), TempOutFile));


http://reviews.llvm.org/D15390