[llvm] r336721 - [ThinLTO] Use std::map to get determistic imports files

Teresa Johnson via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 10 13:06:04 PDT 2018


Author: tejohnson
Date: Tue Jul 10 13:06:04 2018
New Revision: 336721

URL: http://llvm.org/viewvc/llvm-project?rev=336721&view=rev
Log:
[ThinLTO] Use std::map to get determistic imports files

Summary:
I noticed that the .imports files emitted for distributed ThinLTO
backends do not have consistent ordering. This is because StringMap
iteration order is not guaranteed to be deterministic. Since we already
have a std::map with this information, used when emitting the individual
index files (ModuleToSummariesForIndex), use it for the imports files as
well.

This issue is likely causing some unnecessary rebuilds of the ThinLTO
backends in our distributed build system as the imports files are inputs
to those backends.

Reviewers: pcc, steven_wu, mehdi_amini

Subscribers: mehdi_amini, inglorion, eraman, steven_wu, dexonsmith, llvm-commits

Differential Revision: https://reviews.llvm.org/D48783

Added:
    llvm/trunk/test/ThinLTO/X86/Inputs/emit_imports2.ll
Modified:
    llvm/trunk/include/llvm/Transforms/IPO/FunctionImport.h
    llvm/trunk/lib/LTO/LTO.cpp
    llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
    llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
    llvm/trunk/test/ThinLTO/X86/emit_imports.ll

Modified: llvm/trunk/include/llvm/Transforms/IPO/FunctionImport.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/IPO/FunctionImport.h?rev=336721&r1=336720&r2=336721&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/IPO/FunctionImport.h (original)
+++ llvm/trunk/include/llvm/Transforms/IPO/FunctionImport.h Tue Jul 10 13:06:04 2018
@@ -143,9 +143,9 @@ void gatherImportedSummariesForModule(
     std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex);
 
 /// Emit into \p OutputFilename the files module \p ModulePath will import from.
-std::error_code
-EmitImportsFiles(StringRef ModulePath, StringRef OutputFilename,
-                 const FunctionImporter::ImportMapTy &ModuleImports);
+std::error_code EmitImportsFiles(
+    StringRef ModulePath, StringRef OutputFilename,
+    const std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex);
 
 /// Resolve WeakForLinker values in \p TheModule based on the information
 /// recorded in the summaries during global summary-based analysis.

Modified: llvm/trunk/lib/LTO/LTO.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTO.cpp?rev=336721&r1=336720&r2=336721&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/LTO.cpp (original)
+++ llvm/trunk/lib/LTO/LTO.cpp Tue Jul 10 13:06:04 2018
@@ -1097,7 +1097,8 @@ public:
     WriteIndexToFile(CombinedIndex, OS, &ModuleToSummariesForIndex);
 
     if (ShouldEmitImportsFiles) {
-      EC = EmitImportsFiles(ModulePath, NewModulePath + ".imports", ImportList);
+      EC = EmitImportsFiles(ModulePath, NewModulePath + ".imports",
+                            ModuleToSummariesForIndex);
       if (EC)
         return errorCodeToError(EC);
     }

Modified: llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp?rev=336721&r1=336720&r2=336721&view=diff
==============================================================================
--- llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp (original)
+++ llvm/trunk/lib/LTO/ThinLTOCodeGenerator.cpp Tue Jul 10 13:06:04 2018
@@ -758,8 +758,14 @@ void ThinLTOCodeGenerator::emitImports(S
   ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists,
                            ExportLists);
 
+  std::map<std::string, GVSummaryMapTy> ModuleToSummariesForIndex;
+  llvm::gatherImportedSummariesForModule(ModulePath, ModuleToDefinedGVSummaries,
+                                         ImportLists[ModulePath],
+                                         ModuleToSummariesForIndex);
+
   std::error_code EC;
-  if ((EC = EmitImportsFiles(ModulePath, OutputName, ImportLists[ModulePath])))
+  if ((EC =
+           EmitImportsFiles(ModulePath, OutputName, ModuleToSummariesForIndex)))
     report_fatal_error(Twine("Failed to open ") + OutputName +
                        " to save imports lists\n");
 }

Modified: llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp?rev=336721&r1=336720&r2=336721&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp (original)
+++ llvm/trunk/lib/Transforms/IPO/FunctionImport.cpp Tue Jul 10 13:06:04 2018
@@ -707,15 +707,19 @@ void llvm::gatherImportedSummariesForMod
 }
 
 /// Emit the files \p ModulePath will import from into \p OutputFilename.
-std::error_code
-llvm::EmitImportsFiles(StringRef ModulePath, StringRef OutputFilename,
-                       const FunctionImporter::ImportMapTy &ModuleImports) {
+std::error_code llvm::EmitImportsFiles(
+    StringRef ModulePath, StringRef OutputFilename,
+    const std::map<std::string, GVSummaryMapTy> &ModuleToSummariesForIndex) {
   std::error_code EC;
   raw_fd_ostream ImportsOS(OutputFilename, EC, sys::fs::OpenFlags::F_None);
   if (EC)
     return EC;
-  for (auto &ILI : ModuleImports)
-    ImportsOS << ILI.first() << "\n";
+  for (auto &ILI : ModuleToSummariesForIndex)
+    // The ModuleToSummariesForIndex map includes an entry for the current
+    // Module (needed for writing out the index files). We don't want to
+    // include it in the imports file, however, so filter it out.
+    if (ILI.first != ModulePath)
+      ImportsOS << ILI.first << "\n";
   return std::error_code();
 }
 

Added: llvm/trunk/test/ThinLTO/X86/Inputs/emit_imports2.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/Inputs/emit_imports2.ll?rev=336721&view=auto
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/Inputs/emit_imports2.ll (added)
+++ llvm/trunk/test/ThinLTO/X86/Inputs/emit_imports2.ll Tue Jul 10 13:06:04 2018
@@ -0,0 +1,7 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @h() {
+entry:
+  ret void
+}

Modified: llvm/trunk/test/ThinLTO/X86/emit_imports.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/ThinLTO/X86/emit_imports.ll?rev=336721&r1=336720&r2=336721&view=diff
==============================================================================
--- llvm/trunk/test/ThinLTO/X86/emit_imports.ll (original)
+++ llvm/trunk/test/ThinLTO/X86/emit_imports.ll Tue Jul 10 13:06:04 2018
@@ -1,17 +1,19 @@
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/emit_imports.ll -o %t2.bc
+; RUN: opt -module-summary %p/Inputs/emit_imports2.ll -o %t2b.bc
 ; Include a file with an empty module summary index, to ensure that the expected
 ; output files are created regardless, for a distributed build system.
 ; RUN: opt -module-summary %p/Inputs/empty.ll -o %t3.bc
 ; RUN: rm -f %t3.bc.imports
-; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc %t3.bc
-; RUN: llvm-lto -thinlto-action=emitimports -thinlto-index %t.index.bc %t1.bc %t2.bc %t3.bc
+; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc %t2b.bc %t3.bc
+; RUN: llvm-lto -thinlto-action=emitimports -thinlto-index %t.index.bc %t1.bc %t2.bc %t2b.bc %t3.bc
 
 ; The imports file for this module contains the bitcode file for
 ; Inputs/emit_imports.ll
-; RUN: cat %t1.bc.imports | count 1
+; RUN: cat %t1.bc.imports | count 2
 ; RUN: cat %t1.bc.imports | FileCheck %s --check-prefix=IMPORTS1
 ; IMPORTS1: emit_imports.ll.tmp2.bc
+; IMPORTS1: emit_imports.ll.tmp2b.bc
 
 ; The imports file for Input/emit_imports.ll is empty as it does not import anything.
 ; RUN: cat %t2.bc.imports | count 0
@@ -22,13 +24,15 @@
 ; RUN: rm -f %t1.thinlto.bc %t1.bc.imports
 ; RUN: rm -f %t2.thinlto.bc %t2.bc.imports
 ; RUN: rm -f %t3.bc.thinlto.bc %t3.bc.imports
-; RUN: llvm-lto2 run %t1.bc %t2.bc %t3.bc -o %t.o -save-temps \
+; RUN: llvm-lto2 run %t1.bc %t2.bc %t2b.bc %t3.bc -o %t.o -save-temps \
 ; RUN:     -thinlto-distributed-indexes \
 ; RUN:     -r=%t1.bc,g, \
+; RUN:     -r=%t1.bc,h, \
 ; RUN:     -r=%t1.bc,f,px \
-; RUN:     -r=%t2.bc,g,px
+; RUN:     -r=%t2.bc,g,px \
+; RUN:     -r=%t2b.bc,h,px
 
-; RUN: cat %t1.bc.imports | count 1
+; RUN: cat %t1.bc.imports | count 2
 ; RUN: cat %t1.bc.imports | FileCheck %s --check-prefix=IMPORTS1
 
 ; The imports file for Input/emit_imports.ll is empty as it does not import anything.
@@ -44,9 +48,11 @@ target datalayout = "e-m:e-i64:64-f80:12
 target triple = "x86_64-unknown-linux-gnu"
 
 declare void @g(...)
+declare void @h(...)
 
 define void @f() {
 entry:
   call void (...) @g()
+  call void (...) @h()
   ret void
 }




More information about the llvm-commits mailing list