Remove "ExportingModule" from ThinLTO Index

[mehdi_amini]

Summary

There is no real reason the index has to have the concept of an
exporting Module. We should be able to have one single unique
instance of the Index, and it should be read-only after creation
for the whole ThinLTO processing.
The linker plugin should be able to process multiple modules (in
parallel or in sequence) with the same index.

The only reason the ExportingModule was present seems to be to
implement hasExportedFunctions() that is used by the Module linker
to decide what to do with the current Module.
For now I replaced it with a query to the map of Modules path to
see if this module was declared in the Index and consider that if
it is the case then it is probably exporting function.
On the long term the Linker interface needs to evolve and this
call should not be needed anymore.

[tejohnson]

Yes your new check should be sufficient. Eventually when we prune functions from the index that are unlikely to be profitable to import (e.g. they are cold), we should ensure the module string isn't added to the combined index. Looks like that will happen naturally if we do the pruning before we add the module string in FunctionInfoIndex::mergeFrom().

LGTM

[mehdi_amini]

r254581

include/llvm/Bitcode/ReaderWriter.h

namespace llvm {
   parseBitcodeFile(MemoryBufferRef Buffer, LLVMContext &Context,
                    DiagnosticHandlerFunction DiagnosticHandler = nullptr);
 
   /// Check if the given bitcode buffer contains a function summary block.
   bool hasFunctionSummary(MemoryBufferRef Buffer,
                           DiagnosticHandlerFunction DiagnosticHandler);
 
   /// Parse the specified bitcode buffer, returning the function info index.
-  /// If ExportingModule is true, check for functions in the index from this
-  /// module when the combined index is built during parsing and set flag.
   /// If IsLazy is true, parse the entire function summary into
   /// the index. Otherwise skip the function summary section, and only create
   /// an index object with a map from function name to function summary offset.
   /// The index is used to perform lazy function summary reading later.
   ErrorOr<std::unique_ptr<FunctionInfoIndex>> getFunctionInfoIndex(
       MemoryBufferRef Buffer, DiagnosticHandlerFunction DiagnosticHandler,
-      const Module *ExportingModule = nullptr, bool IsLazy = false);
+      bool IsLazy = false);
 
   /// This method supports lazy reading of function summary data from the
   /// combined index during function importing. When reading the combined index
   /// file, getFunctionInfoIndex is first invoked with IsLazy=true.
   /// Then this method is called for each function considered for importing,
   /// to parse the summary information for the given function name into
   /// the index.
   std::error_code readFunctionSummary(

include/llvm/IR/FunctionInfo.h

 private:
   /// Map from function name to list of function information instances
   /// for functions of that name (may be duplicates in the COMDAT case, e.g.).
   FunctionInfoMapTy FunctionMap;
 
   /// Holds strings for combined index, mapping to the corresponding module ID.
   ModulePathStringTableTy ModulePathStringTable;
 
-  /// The main module being compiled, that we are importing into, if applicable.
-  /// Used to check if any of its functions are in the index and therefore
-  /// potentially exported.
-  const Module *ExportingModule;
-
-  /// Flag indicating whether the exporting module has any functions in the
-  /// index and therefore potentially exported (imported into another module).
-  bool HasExportedFunctions;
-
 public:
-  FunctionInfoIndex(const Module *M = nullptr)
-      : ExportingModule(M), HasExportedFunctions(false){};
-  ~FunctionInfoIndex() = default;
+  FunctionInfoIndex() = default;
 
   // Disable the copy constructor and assignment operators, so
   // no unexpected copying/moving occurs.
   FunctionInfoIndex(const FunctionInfoIndex &) = delete;
   void operator=(const FunctionInfoIndex &) = delete;
 
   funcinfo_iterator begin() { return FunctionMap.begin(); }
   const_funcinfo_iterator begin() const { return FunctionMap.begin(); }
   funcinfo_iterator end() { return FunctionMap.end(); }
   const_funcinfo_iterator end() const { return FunctionMap.end(); }
 
   /// Get the list of function info objects for a given function.
   const FunctionInfoList &getFunctionInfoList(StringRef FuncName) {
     return FunctionMap[FuncName];
   }
 
   /// Get the list of function info objects for a given function.
   const const_funcinfo_iterator findFunctionInfoList(StringRef FuncName) const {
     return FunctionMap.find(FuncName);
   }
 
   /// Add a function info for a function of the given name.
   void addFunctionInfo(StringRef FuncName, std::unique_ptr<FunctionInfo> Info) {
-    // Update the HasExportedFunctions flag, but only if we had a function
-    // summary (i.e. we aren't parsing them lazily or have a bitcode file
-    // without a function summary section).
-    if (ExportingModule && Info->functionSummary()) {
-      if (ExportingModule->getModuleIdentifier() ==
-          Info->functionSummary()->modulePath())
-        HasExportedFunctions = true;
-    }
     FunctionMap[FuncName].push_back(std::move(Info));
   }
 
   /// Iterator to allow writer to walk through table during emission.
   iterator_range<StringMap<uint64_t>::const_iterator>
   modPathStringEntries() const {
     return llvm::make_range(ModulePathStringTable.begin(),
                             ModulePathStringTable.end());
public:
   /// Add a new module path, mapped to the given module Id, and return StringRef
   /// owned by string table map.
   StringRef addModulePath(StringRef ModPath, uint64_t ModId) {
     return ModulePathStringTable.insert(std::make_pair(ModPath, ModId))
         .first->first();
   }
 
   /// Check if the given Module has any functions available for exporting
-  /// in the index.
-  bool hasExportedFunctions(const Module *M) const {
-    assert(M == ExportingModule &&
-           "Checking for exported functions on unexpected module");
-    return HasExportedFunctions;
+  /// in the index. We consider any module present in the ModulePathStringTable
+  /// to have exported functions.
+  bool hasExportedFunctions(const Module &M) const {
+    return ModulePathStringTable.count(M.getModuleIdentifier());
   }
 };
 
 } // End llvm namespace
 
 #endif

include/llvm/Object/FunctionIndexObjectFile.h

public:
   hasFunctionSummaryInMemBuffer(MemoryBufferRef Object,
                                 DiagnosticHandlerFunction DiagnosticHandler);
 
   /// \brief Parse function index in the given memory buffer.
   /// Return new FunctionIndexObjectFile instance containing parsed function
   /// summary/index.
   static ErrorOr<std::unique_ptr<FunctionIndexObjectFile>>
   create(MemoryBufferRef Object, DiagnosticHandlerFunction DiagnosticHandler,
-         const Module *ExportingModule = nullptr, bool IsLazy = false);
+         bool IsLazy = false);
 
   /// \brief Parse the function summary information for function with the
   /// given name out of the given buffer. Parsed information is
   /// stored on the index object saved in this object.
   std::error_code
   findFunctionSummaryInMemBuffer(MemoryBufferRef Object,
                                  DiagnosticHandlerFunction DiagnosticHandler,
                                  StringRef FunctionName);
 };
 }
 
 /// Parse the function index out of an IR file and return the function
 /// index object if found, or nullptr if not.
 ErrorOr<std::unique_ptr<FunctionInfoIndex>>
 getFunctionIndexForFile(StringRef Path,
-                        DiagnosticHandlerFunction DiagnosticHandler,
-                        const Module *ExportingModule = nullptr);
+                        DiagnosticHandlerFunction DiagnosticHandler);
 }
 
 #endif

lib/Bitcode/Reader/BitcodeReader.cpp

 // Parse the specified bitcode buffer, returning the function info index.
 // If IsLazy is false, parse the entire function summary into
 // the index. Otherwise skip the function summary section, and only create
 // an index object with a map from function name to function summary offset.
 // The index is used to perform lazy function summary reading later.
 ErrorOr<std::unique_ptr<FunctionInfoIndex>>
 llvm::getFunctionInfoIndex(MemoryBufferRef Buffer,
                            DiagnosticHandlerFunction DiagnosticHandler,
-                           const Module *ExportingModule, bool IsLazy) {
+                           bool IsLazy) {
   std::unique_ptr<MemoryBuffer> Buf = MemoryBuffer::getMemBuffer(Buffer, false);
   FunctionIndexBitcodeReader R(Buf.get(), DiagnosticHandler, IsLazy);
 
-  std::unique_ptr<FunctionInfoIndex> Index =
-      llvm::make_unique<FunctionInfoIndex>(ExportingModule);
+  auto Index = llvm::make_unique<FunctionInfoIndex>();
 
   auto cleanupOnError = [&](std::error_code EC) {
     R.releaseBuffer(); // Never take ownership on error.
     return EC;
   };
 
   if (std::error_code EC = R.parseSummaryIndexInto(nullptr, Index.get()))
     return cleanupOnError(EC);

lib/Linker/LinkModules.cpp

ModuleLinker(Module *dstM, Linker::IdentifiedStructTypeSet &Set, Module *srcM,
         DoneLinkingBodies(false) {
     assert((ImportIndex || !ImportFunction) &&
            "Expect a FunctionInfoIndex when importing");
     // If we have a FunctionInfoIndex but no function to import,
     // then this is the primary module being compiled in a ThinLTO
     // backend compilation, and we need to see if it has functions that
     // may be exported to another backend compilation.
     if (ImportIndex && !ImportFunction)
-      HasExportedFunctions = ImportIndex->hasExportedFunctions(SrcM);
+      HasExportedFunctions = ImportIndex->hasExportedFunctions(*SrcM);
   }
 
   bool run();
   Value *materializeDeclFor(Value *V);
   void materializeInitFor(GlobalValue *New, GlobalValue *Old);
 
 private:
   bool shouldOverrideFromSrc() { return Flags & Linker::OverrideFromSrc; }

lib/Object/FunctionIndexObjectFile.cpp

 }
 
 // Parse function index in the given memory buffer.
 // Return new FunctionIndexObjectFile instance containing parsed
 // function summary/index.
 ErrorOr<std::unique_ptr<FunctionIndexObjectFile>>
 FunctionIndexObjectFile::create(MemoryBufferRef Object,
                                 DiagnosticHandlerFunction DiagnosticHandler,
-                                const Module *ExportingModule, bool IsLazy) {
+                                bool IsLazy) {
   std::unique_ptr<FunctionInfoIndex> Index;
 
   ErrorOr<MemoryBufferRef> BCOrErr = findBitcodeInMemBuffer(Object);
   if (!BCOrErr)
     return BCOrErr.getError();
 
   ErrorOr<std::unique_ptr<FunctionInfoIndex>> IOrErr = getFunctionInfoIndex(
-      BCOrErr.get(), DiagnosticHandler, ExportingModule, IsLazy);
+      BCOrErr.get(), DiagnosticHandler, IsLazy);
 
   if (std::error_code EC = IOrErr.getError())
     return EC;
 
   Index = std::move(IOrErr.get());
 
   return llvm::make_unique<FunctionIndexObjectFile>(Object, std::move(Index));
 }
default:
     return object_error::invalid_file_type;
   }
 }
 
 // Parse the function index out of an IR file and return the function
 // index object if found, or nullptr if not.
 ErrorOr<std::unique_ptr<FunctionInfoIndex>>
 llvm::getFunctionIndexForFile(StringRef Path,
-                              DiagnosticHandlerFunction DiagnosticHandler,
-                              const Module *ExportingModule) {
+                              DiagnosticHandlerFunction DiagnosticHandler) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr =
       MemoryBuffer::getFileOrSTDIN(Path);
   std::error_code EC = FileOrErr.getError();
   if (EC)
     return EC;
   MemoryBufferRef BufferRef = (FileOrErr.get())->getMemBufferRef();
   ErrorOr<std::unique_ptr<object::FunctionIndexObjectFile>> ObjOrErr =
-      object::FunctionIndexObjectFile::create(BufferRef, DiagnosticHandler,
-                                              ExportingModule);
+      object::FunctionIndexObjectFile::create(BufferRef, DiagnosticHandler);
   EC = ObjOrErr.getError();
   if (EC)
     return EC;
 
   object::FunctionIndexObjectFile &Obj = **ObjOrErr;
   return Obj.takeIndex();
 }

tools/llvm-link/llvm-link.cpp

if (verifyModule(*M, &errs())) {
       return false;
     }
 
     // If a function index is supplied, load it so linkInModule can treat
     // local functions/variables as exported and promote if necessary.
     std::unique_ptr<FunctionInfoIndex> Index;
     if (!FunctionIndex.empty()) {
       ErrorOr<std::unique_ptr<FunctionInfoIndex>> IndexOrErr =
-          llvm::getFunctionIndexForFile(FunctionIndex, diagnosticHandler, &*M);
+          llvm::getFunctionIndexForFile(FunctionIndex, diagnosticHandler);
       std::error_code EC = IndexOrErr.getError();
       if (EC) {
         errs() << EC.message() << '\n';
         return false;
       }
       Index = std::move(IndexOrErr.get());
     }