[PATCH] D19139: [LTO] SplitCodeGen new API

[Mehdi AMINI via llvm-commits]

joker.eph added a comment.

Looks OK, see some inline comments.


================
Comment at: include/llvm/CodeGen/ParallelCG.h:49
@@ +48,3 @@
+             ArrayRef<llvm::raw_pwrite_stream *> BCOSs,
+             std::function<std::unique_ptr<TargetMachine>()> F,
+             TargetMachine::CodeGenFileType FT = TargetMachine::CGFT_ObjectFile,
----------------
Is there a better name for  F? 
Also can you add a doxygen for this API (you can refer to the other saying this is a variant that takes a factory function for the TargetMachine)

================
Comment at: lib/CodeGen/ParallelCG.cpp:29
@@ -28,5 +28,3 @@
 static void codegen(Module *M, llvm::raw_pwrite_stream &OS,
-                    const Target *TheTarget, StringRef CPU, StringRef Features,
-                    const TargetOptions &Options, Reloc::Model RM,
-                    CodeModel::Model CM, CodeGenOpt::Level OL,
+                    std::function<std::unique_ptr<TargetMachine>()> F,
                     TargetMachine::CodeGenFileType FileType) {
----------------
Use another name than F, and take a `const &`.

================
Comment at: lib/CodeGen/ParallelCG.cpp:51
@@ +50,3 @@
+  return splitCodeGen(std::move(M), OSs, BCOSs,
+    [=]() {
+      return std::unique_ptr<TargetMachine>(TheTarget->createTargetMachine(
----------------
Do you really mean `=` here? I'd use `&`.


Repository:
  rL LLVM

http://reviews.llvm.org/D19139