[PATCH] D15025: [ThinLTO] Option to invoke ThinLTO backend passes and importing
Mehdi AMINI via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 26 10:14:12 PST 2015
joker.eph added inline comments.
================
Comment at: lib/CodeGen/BackendUtil.cpp:308
@@ +307,3 @@
+ return;
+ }
+
----------------
It does not seem to be nicely integrated within the context of this function. What about all the options set just a few line below? It is not clear if `CodeGenOpts.DisableLLVMOpts` is well honored either.
================
Comment at: lib/CodeGen/CodeGenAction.cpp:30
@@ -29,2 +29,3 @@
#include "llvm/IR/LLVMContext.h"
+#include "llvm/IR/FunctionInfo.h"
#include "llvm/IR/Module.h"
----------------
Not well sorted :)
================
Comment at: lib/CodeGen/CodeGenAction.cpp:190
@@ -169,3 +189,3 @@
[=](const DiagnosticInfo &DI) {
- linkerDiagnosticHandler(DI, LinkModule);
+ linkerDiagnosticHandler(DI, LinkModule, Diags);
},
----------------
Is this an unrelated change to the `-fthinlto-backend` one that could be committed separately?
================
Comment at: lib/CodeGen/CodeGenAction.cpp:821
@@ +820,3 @@
+ linkerDiagnosticHandler(DI, TheModule.get(),
+ CI.getDiagnostics());
+ }, llvm::Linker::Flags::None, Index.get()))
----------------
This lambda is the same as the one just above for `getFunctionIndexForFile`, name it and define it once?
================
Comment at: lib/CodeGen/CodeGenAction.cpp:826
@@ +825,3 @@
+ TheModule = std::move(Combined);
+ }
+
----------------
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 :(
http://reviews.llvm.org/D15025
More information about the cfe-commits
mailing list