From c0320ef47b16272464900df8a7c5a4cb3d1f20c9 Mon Sep 17 00:00:00 2001 From: Teresa Johnson Date: Tue, 10 Jul 2018 20:06:04 +0000 Subject: [PATCH] [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 llvm-svn: 336721 --- llvm/include/llvm/Transforms/IPO/FunctionImport.h | 6 +++--- llvm/lib/LTO/LTO.cpp | 3 ++- llvm/lib/LTO/ThinLTOCodeGenerator.cpp | 8 +++++++- llvm/lib/Transforms/IPO/FunctionImport.cpp | 14 +++++++++----- llvm/test/ThinLTO/X86/Inputs/emit_imports2.ll | 7 +++++++ llvm/test/ThinLTO/X86/emit_imports.ll | 18 ++++++++++++------ 6 files changed, 40 insertions(+), 16 deletions(-) create mode 100644 llvm/test/ThinLTO/X86/Inputs/emit_imports2.ll diff --git a/llvm/include/llvm/Transforms/IPO/FunctionImport.h b/llvm/include/llvm/Transforms/IPO/FunctionImport.h index 5fedde1..369a12e 100644 --- a/llvm/include/llvm/Transforms/IPO/FunctionImport.h +++ b/llvm/include/llvm/Transforms/IPO/FunctionImport.h @@ -143,9 +143,9 @@ void gatherImportedSummariesForModule( std::map &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 &ModuleToSummariesForIndex); /// Resolve WeakForLinker values in \p TheModule based on the information /// recorded in the summaries during global summary-based analysis. diff --git a/llvm/lib/LTO/LTO.cpp b/llvm/lib/LTO/LTO.cpp index 6077b3b..34cca32 100644 --- a/llvm/lib/LTO/LTO.cpp +++ b/llvm/lib/LTO/LTO.cpp @@ -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); } diff --git a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp index d7fac6a..90d0f9b 100644 --- a/llvm/lib/LTO/ThinLTOCodeGenerator.cpp +++ b/llvm/lib/LTO/ThinLTOCodeGenerator.cpp @@ -758,8 +758,14 @@ void ThinLTOCodeGenerator::emitImports(StringRef ModulePath, ComputeCrossModuleImport(Index, ModuleToDefinedGVSummaries, ImportLists, ExportLists); + std::map 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"); } diff --git a/llvm/lib/Transforms/IPO/FunctionImport.cpp b/llvm/lib/Transforms/IPO/FunctionImport.cpp index 38f4d98..c290793 100644 --- a/llvm/lib/Transforms/IPO/FunctionImport.cpp +++ b/llvm/lib/Transforms/IPO/FunctionImport.cpp @@ -707,15 +707,19 @@ void llvm::gatherImportedSummariesForModule( } /// 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 &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(); } diff --git a/llvm/test/ThinLTO/X86/Inputs/emit_imports2.ll b/llvm/test/ThinLTO/X86/Inputs/emit_imports2.ll new file mode 100644 index 0000000..2136ec3 --- /dev/null +++ b/llvm/test/ThinLTO/X86/Inputs/emit_imports2.ll @@ -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 +} diff --git a/llvm/test/ThinLTO/X86/emit_imports.ll b/llvm/test/ThinLTO/X86/emit_imports.ll index fc025f4..41dc148 100644 --- a/llvm/test/ThinLTO/X86/emit_imports.ll +++ b/llvm/test/ThinLTO/X86/emit_imports.ll @@ -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:128-n8:16:32:64-S128" 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 } -- 2.7.4