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

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 1 18:08:48 PST 2015


joker.eph 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;
 
----------------
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.

================
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)
----------------
This does not seem right to me: I don't understand how you don't do a double-free when deleting the Index.


http://reviews.llvm.org/D15024





More information about the llvm-commits mailing list