[PATCH] D33370: Don't verify cross-imported bitcode in FunctionImporter

[Adrian Prantl via Phabricator via llvm-commits]

aprantl created this revision.
Herald added a subscriber: eraman.

After landing https://reviews.llvm.org/D33151 I promptly broke the green dragon ThinLTO buildbot[1]. It turns out that the SrcModule in FunctionImporter is in a really inconsistent intermediate state at the point where I ran the Verifier. After poking around a bit I came to the conclusion that it probably isn't even desirable to run the full-module verifier on each cross-imported module: Over the course of building the entire project the same module will be verified over and over again, which is a waste of resources. When we want to lazy-load metadata running a whole-module Verifier is also problematic.

This patch partially reverts https://reviews.llvm.org/D33151 and instead only runs the verifier once on the module after the cross-imports are done.

The downside is that this won't be able to catch verifier issues that will cause crashes during the import itself, but I think that is an acceptable trade-off.

[1] http://green.lab.llvm.org/green/job/clang-stage2-configure-Rthinlto_build/1850/consoleFull


Repository:
  rL LLVM

https://reviews.llvm.org/D33370

Files:
  include/llvm/Transforms/IPO/FunctionImport.h
  lib/LTO/ThinLTOCodeGenerator.cpp
  lib/Transforms/IPO/FunctionImport.cpp


Index: lib/Transforms/IPO/FunctionImport.cpp
===================================================================
--- lib/Transforms/IPO/FunctionImport.cpp
+++ lib/Transforms/IPO/FunctionImport.cpp
@@ -646,8 +646,7 @@
 // index.
 //
 Expected<bool> FunctionImporter::importFunctions(
-    Module &DestModule, const FunctionImporter::ImportMapTy &ImportList,
-    Optional<std::function<void(Module &)>> PostBitcodeLoading) {
+    Module &DestModule, const FunctionImporter::ImportMapTy &ImportList) {
   DEBUG(dbgs() << "Starting import for Module "
                << DestModule.getModuleIdentifier() << "\n");
   unsigned ImportedCount = 0;
@@ -755,8 +754,6 @@
     // Upgrade debug info after we're done materializing all the globals and we
     // have loaded all the required metadata!
     UpgradeDebugInfo(*SrcModule);
-    if (PostBitcodeLoading)
-      (*PostBitcodeLoading)(*SrcModule);
 
     // Link in the specified functions.
     if (renameModuleForThinLTO(*SrcModule, Index, &GlobalsToImport))
Index: lib/LTO/ThinLTOCodeGenerator.cpp
===================================================================
--- lib/LTO/ThinLTOCodeGenerator.cpp
+++ lib/LTO/ThinLTOCodeGenerator.cpp
@@ -201,16 +201,17 @@
   };
 
   FunctionImporter Importer(Index, Loader);
-  Expected<bool> Result =
-      Importer.importFunctions(TheModule, ImportList, {verifyLoadedModule});
+  Expected<bool> Result = Importer.importFunctions(TheModule, ImportList);
   if (!Result) {
     handleAllErrors(Result.takeError(), [&](ErrorInfoBase &EIB) {
       SMDiagnostic Err = SMDiagnostic(TheModule.getModuleIdentifier(),
                                       SourceMgr::DK_Error, EIB.message());
       Err.print("ThinLTO", errs());
     });
     report_fatal_error("importFunctions failed");
   }
+  // Verify again after cross-importing.
+  verifyLoadedModule(TheModule);
 }
 
 static void optimizeModule(Module &TheModule, TargetMachine &TM,
Index: include/llvm/Transforms/IPO/FunctionImport.h
===================================================================
--- include/llvm/Transforms/IPO/FunctionImport.h
+++ include/llvm/Transforms/IPO/FunctionImport.h
@@ -53,9 +53,7 @@
       : Index(Index), ModuleLoader(std::move(ModuleLoader)) {}
 
   /// Import functions in Module \p M based on the supplied import list.
-  Expected<bool> importFunctions(
-      Module &M, const ImportMapTy &ImportList,
-      Optional<std::function<void(Module &)>> PostBitcodeLoading = None);
+  Expected<bool> importFunctions(Module &M, const ImportMapTy &ImportList);
 
 private:
   /// The summaries index used to trigger importing.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33370.99631.patch
Type: text/x-patch
Size: 2607 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170519/53a1057b/attachment.bin>