[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:80
@@ +79,3 @@
+
+/// Class to own information used by a thread or during its join.
+class ThreadInfo {
----------------
thread or task now?

================
Comment at: tools/gold/gold-plugin.cpp:920
@@ +919,3 @@
+/// pipelines.
+static void ThinLTOBackendThread(claimed_file &F, const void *View,
+                                 ld_plugin_input_file &File,
----------------
Start the function name with a lower case.

This is not a thread anymore. Task maybe?


================
Comment at: tools/gold/gold-plugin.cpp:925
@@ +924,3 @@
+                                 raw_fd_ostream *OS) {
+  // Not thread safe, open a new one for each thread.
+  LLVMContext Context;
----------------
It is thread safe since you create a new one for each thread, no?

================
Comment at: tools/gold/gold-plugin.cpp:940
@@ +939,3 @@
+  std::unique_ptr<llvm::Module> RenamedModule =
+      renameModuleForThinLTO(M, &CombinedIndex);
+  if (!RenamedModule)
----------------
Don't we want something that uses gold's symbol resolution?

renameModuleForThinLTO will copy even stuff that gold has marked as preempted, no?

================
Comment at: tools/gold/gold-plugin.cpp:971
@@ +970,3 @@
+  if (!options::obj_path.empty())
+    Filename = options::obj_path;
+  else if (options::TheOutputType == options::OT_SAVE_TEMPS)
----------------
This seems incompatible with threading or even multiple outputs, no? It looks like every thread will try to use the same output file name.

I would suggest producing an error for now.


http://reviews.llvm.org/D15390