[PATCH] D12308: gold-plugin: Implement parallel LTO code generation using llvm::LinkedCodeGen.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: tools/gold/gold-plugin.cpp:134
@@ -131,1 +133,3 @@
       OptLevel = opt[1] - '0';
+    } else if (opt.startswith("jobs=")) {
+      Parallelism = atoi(opt_ + 5);
----------------
joker.eph wrote:
> any reason not re-using the same `-j` that is used by most tool for the jobs? 
> (Maybe it is more coherent with gold, I don't know).
`-j` is fine for the testing tools. But for user-facing tools I think a more descriptive flag would be more appropriate.

================
Comment at: tools/gold/gold-plugin.cpp:135
@@ -132,1 +134,3 @@
+    } else if (opt.startswith("jobs=")) {
+      Parallelism = atoi(opt_ + 5);
     } else {
----------------
joker.eph wrote:
> strtol?
Seems that `StringRef::getAsInteger` is the canonical way to do this.

================
Comment at: tools/gold/gold-plugin.cpp:786
@@ -781,3 +785,3 @@
 
-  runLTOPasses(M, *TM);
+  runLTOPasses(*M.get(), *TM);
 
----------------
joker.eph wrote:
> Is the `.get()` useful here?
Removed

================
Comment at: tools/gold/gold-plugin.cpp:789
@@ -784,5 +788,3 @@
   if (options::TheOutputType == options::OT_SAVE_TEMPS)
-    saveBCFile(output_name + ".opt.bc", M);
-
-  legacy::PassManager CodeGenPasses;
+    saveBCFile(output_name + ".opt.bc", *M.get());
 
----------------
joker.eph wrote:
> same
Removed

================
Comment at: tools/gold/gold-plugin.cpp:803
@@ +802,3 @@
+    std::list<llvm::raw_fd_ostream> OSs;
+    std::vector<llvm::raw_pwrite_stream *> OSPtrs(options::Parallelism);
+    for (unsigned I = 0; I != options::Parallelism; ++I) {
----------------
joker.eph wrote:
> Same question as for llvm-lto, what about tool_output_file?
I believe that `tool_output_file` cannot be used with temporary files.


http://reviews.llvm.org/D12308