[PATCH] D15024: [ThinLTO] Support for specifying function index from pass manager

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: include/llvm/IR/Module.h:177
@@ -175,2 +176,3 @@
   DataLayout DL;                  ///< DataLayout associated with the module
+  std::unique_ptr<FunctionInfoIndex> FunctionIndex;
 
----------------
joker.eph wrote:
> It does not make sense to me conceptually: the FunctionIndex is not owned by a Module. It is a common data-structure for all Modules that we're dealing with.
Ok, I had been thinking that this would be owned by the module we are importing into. But agree after thinking about the case you mentioned where there is one copy in memory in the linker, shared by all the backend threads.

Probably I should just pass the in-memory index directly in PassManagerBuilder. But the question is how to set this from clang (in D15025). The CodeGenOptions where I save the index file name doesn't seem like the right place, but perhaps I can save it on the CompilerInvocation object, and set it on the PassManagerBuilder from there. WDYT?

================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:254
@@ -249,10 +253,3 @@
   bool runOnModule(Module &M) override {
-    if (SummaryFile.empty()) {
-      report_fatal_error("error: -function-import requires -summary-file\n");
-    }
-    std::string Error;
-    std::unique_ptr<FunctionInfoIndex> Index =
-        getFunctionIndexForFile(SummaryFile, Error, diagnosticHandler);
-    if (!Index) {
-      errs() << "Error loading file '" << SummaryFile << "': " << Error << "\n";
-      return false;
+    std::unique_ptr<FunctionInfoIndex> Index(M.getFunctionIndex());
+    if (SummaryFile.empty() && !Index)
----------------
joker.eph wrote:
> This does not seem right to me: I don't understand how you don't do a double-free when deleting the Index.
You're right, this should just be a pointer not a unique_ptr.


http://reviews.llvm.org/D15024