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