[ThinLTO] Always emit a summary when compiling in ThinLTO mode

Summary:
Emit an empty summary section, instead of no summary section, when
there are no global variables in the index. This ensures that LTO
will treat these files as ThinLTO inputs, instead of as regular
LTO inputs.

In addition to not being what the user likely intended when
compiling with -flto=thin, the current behavior is problematic for
distributed build systems that expect to get ThinLTO index and imports
files back for each input compiled with -flto=thin. Combining into
a single regular LTO module also reduces the backend parallelism.
And in the case where the index was suppressed due to uses in
inline assembly, combining into a single LTO module could provoke
renaming of duplicates that we were trying to prevent by suppressing
the index.

This change required a couple of fixes to handle the empty summary
section.

Reviewers: mehdi_amini

Subscribers: mehdi_amini, llvm-commits, pcc

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

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@282037 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Bitcode/Reader/BitcodeReader.cpp b/lib/Bitcode/Reader/BitcodeReader.cpp
index 33f1a7e454e89ad50f6ee01203f0e33263d4710a..d45360f0049d7bdb013e7a54e8b5660111803c83 100644 (file)
--- a/lib/Bitcode/Reader/BitcodeReader.cpp
+++ b/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -6097,13 +6097,18 @@ std::error_code ModuleSummaryIndexBitcodeReader::parseModule() {
           return error("Invalid record");
         break;
       case bitc::GLOBALVAL_SUMMARY_BLOCK_ID:
-        assert(VSTOffset > 0 && "Expected non-zero VST offset");
         assert(!SeenValueSymbolTable &&
                "Already read VST when parsing summary block?");
-        if (std::error_code EC =
-                parseValueSymbolTable(VSTOffset, ValueIdToLinkageMap))
-          return EC;
-        SeenValueSymbolTable = true;
+        // We might not have a VST if there were no values in the
+        // summary. An empty summary block generated when we are
+        // performing ThinLTO compiles so we don't later invoke
+        // the regular LTO process on them.
+        if (VSTOffset > 0) {
+          if (std::error_code EC =
+                  parseValueSymbolTable(VSTOffset, ValueIdToLinkageMap))
+            return EC;
+          SeenValueSymbolTable = true;
+        }
         SeenGlobalValSummary = true;
         if (std::error_code EC = parseEntireSummary())
           return EC;

diff --git a/lib/Bitcode/Writer/BitcodeWriter.cpp b/lib/Bitcode/Writer/BitcodeWriter.cpp
index df700a8aba1a3531e6d00770c1b0e84d77ccce51..a353edf7aec193bcee6d906e5b11bc040406c29f 100644 (file)
--- a/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ b/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -3341,13 +3341,15 @@ static const uint64_t INDEX_VERSION = 1;
 /// Emit the per-module summary section alongside the rest of
 /// the module's bitcode.
 void ModuleBitcodeWriter::writePerModuleGlobalValueSummary() {
-  if (Index->begin() == Index->end())
-    return;
-
   Stream.EnterSubblock(bitc::GLOBALVAL_SUMMARY_BLOCK_ID, 4);
 
   Stream.EmitRecord(bitc::FS_VERSION, ArrayRef<uint64_t>{INDEX_VERSION});
 
+  if (Index->begin() == Index->end()) {
+    Stream.ExitBlock();
+    return;
+  }
+
   // Abbrev for FS_PERMODULE.
   BitCodeAbbrev *Abbv = new BitCodeAbbrev();
   Abbv->Add(BitCodeAbbrevOp(bitc::FS_PERMODULE));

diff --git a/lib/LTO/LTO.cpp b/lib/LTO/LTO.cpp
index 0d47cd49a172ea9f501ce955352e1444bb7c5e0a..017a45ac7ba8ca932d9615150f380d1a53aea05f 100644 (file)
--- a/lib/LTO/LTO.cpp
+++ b/lib/LTO/LTO.cpp
@@ -713,6 +713,16 @@ Error LTO::runThinLTO(AddOutputFn AddOutput, bool HasRegularLTO) {
       ModuleToDefinedGVSummaries(ThinLTO.ModuleMap.size());
   ThinLTO.CombinedIndex.collectDefinedGVSummariesPerModule(
       ModuleToDefinedGVSummaries);
+  // Create entries for any modules that didn't have any GV summaries
+  // (either they didn't have any GVs to start with, or we suppressed
+  // generation of the summaries because they e.g. had inline assembly
+  // uses that couldn't be promoted/renamed on export). This is so
+  // InProcessThinBackend::start can still launch a backend thread, which
+  // is passed the map of summaries for the module, without any special
+  // handling for this case.
+  for (auto &Mod : ThinLTO.ModuleMap)
+    if (!ModuleToDefinedGVSummaries.count(Mod.first))
+      ModuleToDefinedGVSummaries.try_emplace(Mod.first);
 
   StringMap<FunctionImporter::ImportMapTy> ImportLists(
       ThinLTO.ModuleMap.size());

diff --git a/test/Bitcode/thinlto-empty-summary-section.ll b/test/Bitcode/thinlto-empty-summary-section.ll
new file mode 100644 (file)
index 0000000..9708661
--- /dev/null
+++ b/test/Bitcode/thinlto-empty-summary-section.ll
@@ -0,0 +1,7 @@
+; Ensure we get a summary block even when the file is empty.
+; RUN: opt -module-summary %s -o %t.o
+; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s
+
+; CHECK: <GLOBALVAL_SUMMARY_BLOCK
+; CHECK: <VERSION op0=
+; CHECK: </GLOBALVAL_SUMMARY_BLOCK>

diff --git a/test/ThinLTO/X86/Inputs/emit_imports.ll b/test/ThinLTO/X86/Inputs/emit_imports.ll
index 4e0840f3691e291694f7e36d588301195b57f495..31c72ec4653a644b0266f5eeb78a988454612b45 100644 (file)
--- a/test/ThinLTO/X86/Inputs/emit_imports.ll
+++ b/test/ThinLTO/X86/Inputs/emit_imports.ll
@@ -1,3 +1,6 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
 define void @g() {
 entry:
   ret void

diff --git a/test/ThinLTO/X86/Inputs/empty.ll b/test/ThinLTO/X86/Inputs/empty.ll
new file mode 100644 (file)
index 0000000..a3c99cd
--- /dev/null
+++ b/test/ThinLTO/X86/Inputs/empty.ll
@@ -0,0 +1,2 @@
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"

diff --git a/test/ThinLTO/X86/emit_imports.ll b/test/ThinLTO/X86/emit_imports.ll
index a8215d502307fa1635cbd60081f85af22a1a90b5..64ea02d857e6a46c1d95b8cf765ecc82f51bf693 100644 (file)
--- a/test/ThinLTO/X86/emit_imports.ll
+++ b/test/ThinLTO/X86/emit_imports.ll
@@ -1,7 +1,11 @@
 ; RUN: opt -module-summary %s -o %t1.bc
 ; RUN: opt -module-summary %p/Inputs/emit_imports.ll -o %t2.bc
-; RUN: llvm-lto -thinlto-action=thinlink -o %t.index.bc %t1.bc %t2.bc
-; RUN: llvm-lto -thinlto-action=emitimports -thinlto-index %t.index.bc %t1.bc %t2.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
 
 ; The imports file for this module contains the bitcode file for
 ; Inputs/emit_imports.ll
@@ -12,9 +16,13 @@
 ; The imports file for Input/emit_imports.ll is empty as it does not import anything.
 ; RUN: cat %t2.bc.imports | count 0
 
+; The imports file for Input/empty.ll is empty but should exist.
+; RUN: cat %t3.bc.imports | count 0
+
 ; RUN: rm -f %t1.thinlto.bc %t1.bc.imports
 ; RUN: rm -f %t2.thinlto.bc %t2.bc.imports
-; RUN: llvm-lto2 %t1.bc %t2.bc -o %t.o \
+; RUN: rm -f %t3.bc.thinlto.bc %t3.bc.imports
+; RUN: llvm-lto2 %t1.bc %t2.bc %t3.bc -o %t.o -save-temps \
 ; RUN:     -thinlto-distributed-indexes \
 ; RUN:     -r=%t1.bc,g, \
 ; RUN:     -r=%t1.bc,f,px \
@@ -26,6 +34,15 @@
 ; The imports file for Input/emit_imports.ll is empty as it does not import anything.
 ; RUN: cat %t2.bc.imports | count 0
 
+; The imports file for Input/empty.ll is empty but should exist.
+; RUN: cat %t3.bc.imports | count 0
+
+; The index file should be created even for the input with an empty summary.
+; RUN: ls %t3.bc.thinlto.bc
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
 declare void @g(...)
 
 define void @f() {

diff --git a/test/tools/gold/X86/Inputs/thinlto_empty.ll b/test/tools/gold/X86/Inputs/thinlto_empty.ll
new file mode 100644 (file)
index 0000000..e69de29

diff --git a/test/tools/gold/X86/thinlto_emit_imports.ll b/test/tools/gold/X86/thinlto_emit_imports.ll
index 845b691236c20e14a5b8e9e96811a31c6647c289..4346ece7ce22c3b23fbc2a3ab9ec1c22998e2985 100644 (file)
--- a/test/tools/gold/X86/thinlto_emit_imports.ll
+++ b/test/tools/gold/X86/thinlto_emit_imports.ll
@@ -1,13 +1,17 @@
 ; Generate summary sections and test gold handling.
 ; RUN: opt -module-summary %s -o %t.o
 ; RUN: opt -module-summary %p/Inputs/thinlto.ll -o %t2.o
+; 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/thinlto_empty.ll -o %t3.o
 
 ; Ensure gold generates imports files if requested for distributed backends.
+; RUN: rm -f %t3.o.imports %t3.o.thinlto.bc
 ; RUN: %gold -plugin %llvmshlibdir/LLVMgold.so \
 ; RUN:    --plugin-opt=thinlto \
 ; RUN:    --plugin-opt=thinlto-index-only \
 ; RUN:    --plugin-opt=thinlto-emit-imports-files \
-; RUN:    -shared %t.o %t2.o -o %t3
+; RUN:    -shared %t.o %t2.o %t3.o -o %t4
 
 ; The imports file for this module contains the bitcode file for
 ; Inputs/thinlto.ll
@@ -18,6 +22,12 @@
 ; The imports file for Input/thinlto.ll is empty as it does not import anything.
 ; RUN: cat %t2.o.imports | count 0
 
+; The imports file for Input/thinlto_empty.ll is empty but should exist.
+; RUN: cat %t3.o.imports | count 0
+
+; The index file should be created even for the input with an empty summary.
+; RUN: ls %t3.o.thinlto.bc
+
 declare void @g(...)
 
 define void @f() {