tejohnson added a comment.

Thanks for the review, comments below.
Teresa


================
Comment at: lib/CodeGen/CodeGenAction.cpp:190
@@ -169,3 +189,3 @@
                                 [=](const DiagnosticInfo &DI) {
-                                  linkerDiagnosticHandler(DI, LinkModule);
+                                  linkerDiagnosticHandler(DI, LinkModule, 
Diags);
                                 },
----------------
joker.eph wrote:
> Is this an unrelated change to the `-fthinlto-backend` one that could be 
> committed separately?
It is related in that I am now calling linkerDiagnosticHandler from outside 
BackendConsumer, which required moving it out of that class, and therefore we 
need to pass in the DiagnosticsEngine.

================
Comment at: lib/CodeGen/CodeGenAction.cpp:821
@@ +820,3 @@
+                                linkerDiagnosticHandler(DI, TheModule.get(),
+                                                        CI.getDiagnostics());
+                              }, llvm::Linker::Flags::None, Index.get()))
----------------
joker.eph wrote:
> This lambda is the same as the one just above for `getFunctionIndexForFile`, 
> name it and define it once?
Ok

================
Comment at: lib/CodeGen/CodeGenAction.cpp:826
@@ +825,3 @@
+      TheModule = std::move(Combined);
+    }
+
----------------
joker.eph wrote:
> So for the renaming we need to duplicate completely the module? It cannot be 
> done by morphing the existing module in place? That seems quite inefficient :(
For now that is how the ModuleLinker works unfortunately. Rafael is working on 
a change to split this into two parts, and the symbol resolution would be done 
in place. So this should be resolved eventually.


http://reviews.llvm.org/D15025



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to