Apply summary-based dead stripping to regular LTO modules with summaries.

[pcc]

Summary

If a regular LTO module has a summary index, then instead of linking
it into the combined regular LTO module right away, add it to the
combined summary index and associate it with a special module that
represents the combined regular LTO module.

Any such modules are linked during LTO::run(), at which time we use
the results of summary-based dead stripping to control whether to
link prevailing symbols.

Depends on D33921

[tejohnson]

llvm/include/llvm/IR/ModuleSummaryIndex.h

694 ↗

(On Diff #101484)

The patch adds a couple different new types named Module - is it possible to use something else rather than overloading that name?

697 ↗

(On Diff #101484)

Update description of return value.

llvm/include/llvm/LTO/LTO.h

287 ↗

(On Diff #101484)

Can you define this structure here in the header? I think it would be clearer.

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

724 ↗

(On Diff #101484)

I'm a bit confused about the relationship between ModulePath and Mod. It seems from the code in addThisModule() that Mod is being used to save on insertion attempts into the table, or is there another reason? If just an optimization, can that be committed separately?

I don't quite understand the last part of the comment "if it is expecting to read a per-module summary index". Do you mean it is only filled in when we are (in the process of) reading a per-module summary index?

4695 ↗

(On Diff #101484)

I take it we were always guaranteed to have ModuleId == 0 when we called addThisModulePath?

Also, previously we would repeatedly try to re-add the module path - is saving the result of addModule in the reader and checking it here just an optimization, or required because of the other changes?

5509 ↗

(On Diff #101484)

It wasn't clear to me when I first looked at this part of the patch why the ModuleIdentifier on the BitcodeModule wouldn't be used, as it is in getSummary below (or was here before). I think it would be clearer to pass in a flag indicating that this is a regularLTO summary, and then here either use the ModuleIdentifier or "", depending on the flag.

llvm/lib/LTO/LTO.cpp

605 ↗

(On Diff #101484)

I think the old comment is more accurate (we can link it as available_externally, but only need to if it is undefined). Perhaps just add that we will decide later when we link this module, based on whether it is undefined.

[tejohnson]

llvm/lib/LTO/LTO.cpp

509 ↗

(On Diff #101484)

Why not just link all of the regular LTO objects during runRegularLTO? I guess you still need to keep track of which had summaries though. How much of addRegularLTO could then be deferred until runRegularLTO (i.e. keeping track of the resolution info, like we do for ThinLTO, but delaying building the Module, computing Keep, etc)? It seems conceptually simpler to do all the linking in one place.

[pcc]

llvm/include/llvm/IR/ModuleSummaryIndex.h

694 ↗

(On Diff #101484)

I will rename the other one to AddedModule.

697 ↗

(On Diff #101484)

Will do

llvm/include/llvm/LTO/LTO.h

287 ↗

(On Diff #101484)

Will do

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

724 ↗

(On Diff #101484)

I think I originally wanted this to work by changing addModule so that it would always either create a module or error out rather than possibly returning an existing one, and then have the client always create the module on its own (so that it could control whether or not to create a module for each input file, and as a side effect we would get the optimization that you mention), but then I thought that that wouldn't necessarily work because the pointer could become invalidated by the StringMap (although now that I look at StringMap's rehash implementation I don't think that could actually happen). So I ended up moving the Module creation back into the reader.

I'd be fine with changing this back to be more like how it was and we can think about changing the API at a later point.

Regarding the comment, I mean that the caller (i.e. readSummary) will fill this field in as it is expecting to read a per-module summary index. I'll make this comment clearer if I make this change in a followup.

4695 ↗

(On Diff #101484)

Yes, if !Mod at this point we must have been called from getSummary for which the current behaviour is that the module id will be 0 if it is a per-module summary.

It's more of a pure optimization at this point, so I'll revert it as I mentioned on the other comment.

5509 ↗

(On Diff #101484)

I did it this way because I wanted LTO to handle regular LTO module summaries on its own (well, as much as possible I suppose). With that suggestion, we would move more of the handling into the bitcode reader, which I'm not entirely opposed to, but let me know what you think of that rationale.

llvm/lib/LTO/LTO.cpp

509 ↗

(On Diff #101484)

I think the first thing I tried was something along those lines, but the code turned out to be less clean in my opinion, precisely because there was more state to keep track of. I guess it's just a matter of opinion, but this seems cleaner to me, as there turned out to be a relatively clean boundary between preparing to link a module and linking it.

605 ↗

(On Diff #101484)

Will do

[pcc]

llvm/include/llvm/IR/ModuleSummaryIndex.h

697 ↗

(On Diff #101484)

Done (in D34124).

[pcc]

Address review comments

[eugenis]

llvm/lib/LTO/LTO.cpp

474 ↗

(On Diff #102266)

Move this to ModuleSummaryIndex? I'm adding the same helper function to LowerTypeTests in D34168.

[tobiasvk]

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

5666 ↗

(On Diff #102266)

So this will be a bit misleading if you change it as you suggested in D34156 to check just for ThinLTO. Can we rename it to hasThinLTOSummary, or maybe even get rid of it and instead create a getBitcodeLTOInfo(MemoryBufferRef) function so clients can figure things out directly from the LTOInfo?

[pcc]

• Address review comments

[pcc]

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

5666 ↗

(On Diff #102266)

Replaced with a getBitcodeLTOInfo function.

[pcc]

• Remove the old IsLiveByGUID function

[tejohnson]

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

5509 ↗

(On Diff #101484)

Ok, but please document the fact that we are ignoring the ModuleIdentifier because of client-dependent behavior around the desired module path in the combined index.

llvm/lib/LTO/LTO.cpp

509 ↗

(On Diff #101484)

Ok. But is this affected by the change being discussed in D34156, where my understanding is that all regular LTO modules will have summaries?

[pcc]

• Add a comment explaining why we aren't using ModuleIdentifier

[pcc]

llvm/lib/LTO/LTO.cpp

509 ↗

(On Diff #101484)

After that change lands all LTO modules produced by clang will have summaries, so we will normally be following the new code path that defers the link step. But we will still need to be able to read modules without summaries for backwards compatibility reasons and in order to handle bitcode created by non-clang producers.

[tejohnson]

LGTM

llvm/lib/LTO/LTO.cpp

509 ↗

(On Diff #101484)

I suspect we may eventually want to reorganize this so that regularLTO operates more like ThinLTO (read the summary and symbols here only, read the module later during runRegularLTO). But let's go forward with this approach for now, since it is a smaller delta compared to the current approach.

cfe/trunk/lib/CodeGen/BackendUtil.cpp

void EmitAssemblyHelper::EmitAssemblyWithNewPassManager(
   }
 }
 
 Expected<BitcodeModule> clang::FindThinLTOModule(MemoryBufferRef MBRef) {
   Expected<std::vector<BitcodeModule>> BMsOrErr = getBitcodeModuleList(MBRef);
   if (!BMsOrErr)
     return BMsOrErr.takeError();
 
-  // The bitcode file may contain multiple modules, we want the one with a
-  // summary.
+  // The bitcode file may contain multiple modules, we want the one that is
+  // marked as being the ThinLTO module.
   for (BitcodeModule &BM : *BMsOrErr) {
-    Expected<bool> HasSummary = BM.hasSummary();
-    if (HasSummary && *HasSummary)
+    Expected<BitcodeLTOInfo> LTOInfo = BM.getLTOInfo();
+    if (LTOInfo && LTOInfo->IsThinLTO)
       return BM;
   }
 
   return make_error<StringError>("Could not find module summary",
                                  inconvertibleErrorCode());
 }
 
 static void runThinLTOBackend(ModuleSummaryIndex *CombinedIndex, Module *M,

cfe/trunk/test/CodeGen/Inputs/thinlto-multi-module.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @f2() {
+  ret void
+}
+
+!0 = !{i32 1, !"ThinLTO", i32 0}
+!llvm.module.flags = !{ !0 }

cfe/trunk/test/CodeGen/thinlto-multi-module.ll

 ; REQUIRES: x86-registered-target
 
 ; RUN: opt -module-summary -o %t1.o %s
 ; RUN: llvm-lto -thinlto -o %t %t1.o
 
-; RUN: opt -o %t2.o %S/Inputs/thinlto_backend.ll
-; RUN: llvm-cat -b -o %t1cat.o %t1.o %t2.o
+; RUN: opt -module-summary -o %t2.o %S/Inputs/thinlto-multi-module.ll
+; RUN: llvm-cat -b -o %t1cat.o %t2.o %t1.o
 ; RUN: cp %t1cat.o %t1.o
 ; RUN: %clang -target x86_64-unknown-linux-gnu -O2 -o %t3.o -x ir %t1.o -c -fthinlto-index=%t.thinlto.bc
 ; RUN: llvm-nm %t3.o | FileCheck --check-prefix=CHECK-OBJ %s
 ; CHECK-OBJ: T f1
 ; CHECK-OBJ: U f2
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 declare void @f2()
 
 define void @f1() {
   call void @f2()
   ret void
 }

llvm/trunk/include/llvm/Bitcode/BitcodeReader.h

namespace llvm {
   ErrorOr<T> expectedToErrorOrAndEmitErrors(LLVMContext &Ctx, Expected<T> Val) {
     if (!Val)
       return errorToErrorCodeAndEmitErrors(Ctx, Val.takeError());
     return std::move(*Val);
   }
 
   struct BitcodeFileContents;
 
+  /// Basic information extracted from a bitcode module to be used for LTO.
+  struct BitcodeLTOInfo {
+    bool IsThinLTO;
+    bool HasSummary;
+  };
+
   /// Represents a module in a bitcode file.
   class BitcodeModule {
     // This covers the identification (if present) and module blocks.
     ArrayRef<uint8_t> Buffer;
     StringRef ModuleIdentifier;
 
     // The string table used to interpret this module.
     StringRef Strtab;
public:
     /// importing into another module.
     Expected<std::unique_ptr<Module>> getLazyModule(LLVMContext &Context,
                                                     bool ShouldLazyLoadMetadata,
                                                     bool IsImporting);
 
     /// Read the entire bitcode module and return it.
     Expected<std::unique_ptr<Module>> parseModule(LLVMContext &Context);
 
-    /// Check if the given bitcode buffer contains a summary block.
-    Expected<bool> hasSummary();
+    /// Returns information about the module to be used for LTO: whether to
+    /// compile with ThinLTO, and whether it has a summary.
+    Expected<BitcodeLTOInfo> getLTOInfo();
 
     /// Parse the specified bitcode buffer, returning the module summary index.
     Expected<std::unique_ptr<ModuleSummaryIndex>> getSummary();
 
     /// Parse the specified bitcode buffer and merge its module summary index
     /// into CombinedIndex.
-    Error readSummary(ModuleSummaryIndex &CombinedIndex, unsigned ModuleId);
+    Error readSummary(ModuleSummaryIndex &CombinedIndex, StringRef ModulePath,
+                      uint64_t ModuleId);
   };
 
   struct BitcodeFileContents {
     std::vector<BitcodeModule> Mods;
   };
 
   /// Returns the contents of a bitcode file.
   Expected<BitcodeFileContents> getBitcodeFileContents(MemoryBufferRef Buffer);
namespace llvm {
   /// producer string information. If successful, this returns a string. On
   /// error, this returns "".
   Expected<std::string> getBitcodeProducerString(MemoryBufferRef Buffer);
 
   /// Read the specified bitcode file, returning the module.
   Expected<std::unique_ptr<Module>> parseBitcodeFile(MemoryBufferRef Buffer,
                                                      LLVMContext &Context);
 
-  /// Check if the given bitcode buffer contains a summary block.
-  Expected<bool> hasGlobalValueSummary(MemoryBufferRef Buffer);
+  /// Returns LTO information for the specified bitcode file.
+  Expected<BitcodeLTOInfo> getBitcodeLTOInfo(MemoryBufferRef Buffer);
 
   /// Parse the specified bitcode buffer, returning the module summary index.
   Expected<std::unique_ptr<ModuleSummaryIndex>>
   getModuleSummaryIndex(MemoryBufferRef Buffer);
 
   /// Parse the specified bitcode buffer and merge the index into CombinedIndex.
   Error readModuleSummaryIndex(MemoryBufferRef Buffer,
                                ModuleSummaryIndex &CombinedIndex,
-                               unsigned ModuleId);
+                               uint64_t ModuleId);
 
   /// Parse the module summary index out of an IR file and return the module
   /// summary index object if found, or an empty summary if not. If Path refers
   /// to an empty file and IgnoreEmptyThinLTOIndexFile is true, then
   /// this function will return nullptr.
   Expected<std::unique_ptr<ModuleSummaryIndex>>
   getModuleSummaryIndexForFile(StringRef Path,
                                bool IgnoreEmptyThinLTOIndexFile = false);

llvm/trunk/include/llvm/IR/ModuleSummaryIndex.h

public:
   }
   void setWithGlobalValueDeadStripping() {
     WithGlobalValueDeadStripping = true;
   }
 
   bool isGlobalValueLive(const GlobalValueSummary *GVS) const {
     return !WithGlobalValueDeadStripping || GVS->isLive();
   }
+  bool isGUIDLive(GlobalValue::GUID GUID) const;
 
   /// Return a ValueInfo for GUID if it exists, otherwise return ValueInfo().
   ValueInfo getValueInfo(GlobalValue::GUID GUID) const {
     auto I = GlobalValueMap.find(GUID);
     return ValueInfo(I == GlobalValueMap.end() ? nullptr : &*I);
   }
 
   /// Return a ValueInfo for \p GUID.

llvm/trunk/include/llvm/LTO/LTO.h

struct RegularLTOState {
     };
     std::map<std::string, CommonResolution> Commons;
 
     unsigned ParallelCodeGenParallelismLevel;
     LTOLLVMContext Ctx;
     bool HasModule = false;
     std::unique_ptr<Module> CombinedModule;
     std::unique_ptr<IRMover> Mover;
+
+    // This stores the information about a regular LTO module that we have added
+    // to the link. It will either be linked immediately (for modules without
+    // summaries) or after summary-based dead stripping (for modules with
+    // summaries).
+    struct AddedModule {
+      std::unique_ptr<Module> M;
+      std::vector<GlobalValue *> Keep;
+    };
+    std::vector<AddedModule> ModsWithSummaries;
   } RegularLTO;
 
   struct ThinLTOState {
     ThinLTOState(ThinBackend Backend);
 
     ThinBackend Backend;
     ModuleSummaryIndex CombinedIndex;
     MapVector<StringRef, BitcodeModule> ModuleMap;
     DenseMap<GlobalValue::GUID, StringRef> PrevailingModuleForGUID;
   } ThinLTO;
 
   // The global resolution for a particular (mangled) symbol name. This is in
   // particular necessary to track whether each symbol can be internalized.
   // Because any input file may introduce a new cross-partition reference, we
   // cannot make any final internalization decisions until all input files have
   // been added and the client has called run(). During run() we apply
   // internalization decisions either directly to the module (for regular LTO)
   // or to the combined index (for ThinLTO).
   struct GlobalResolution {
     /// The unmangled name of the global.
     std::string IRName;
 
-    /// Keep track if the symbol is visible outside of ThinLTO (i.e. in
-    /// either a regular object or the regular LTO partition).
-    bool VisibleOutsideThinLTO = false;
+    /// Keep track if the symbol is visible outside of a module with a summary
+    /// (i.e. in either a regular object or a regular LTO module without a
+    /// summary).
+    bool VisibleOutsideSummary = false;
 
     bool UnnamedAddr = true;
 
     /// This field keeps track of the partition number of this global. The
     /// regular LTO object is partition 0, while each ThinLTO object has its own
     /// partition number from 1 onwards.
     ///
     /// Any global that is defined or used by more than one partition, or that
enum : unsigned {
       /// The RegularLTO partition
       RegularLTO = 0,
     };
   };
 
   // Global mapping from mangled symbol names to resolutions.
   StringMap<GlobalResolution> GlobalResolutions;
 
-  void addSymbolToGlobalRes(const InputFile::Symbol &Sym, SymbolResolution Res,
-                            unsigned Partition);
+  void addModuleToGlobalRes(ArrayRef<InputFile::Symbol> Syms,
+                            ArrayRef<SymbolResolution> Res, unsigned Partition,
+                            bool InSummary);
 
   // These functions take a range of symbol resolutions [ResI, ResE) and consume
   // the resolutions used by a single input module by incrementing ResI. After
   // these functions return, [ResI, ResE) will refer to the resolution range for
   // the remaining modules in the InputFile.
   Error addModule(InputFile &Input, unsigned ModI,
                   const SymbolResolution *&ResI, const SymbolResolution *ResE);
-  Error addRegularLTO(BitcodeModule BM,
-                      ArrayRef<InputFile::Symbol> Syms,
-                      const SymbolResolution *&ResI,
-                      const SymbolResolution *ResE);
+
+  Expected<RegularLTOState::AddedModule>
+  addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
+                const SymbolResolution *&ResI, const SymbolResolution *ResE);
+  Error linkRegularLTO(RegularLTOState::AddedModule Mod,
+                       bool LivenessFromIndex);
+
   Error addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
                    const SymbolResolution *&ResI, const SymbolResolution *ResE);
 
   Error runRegularLTO(AddStreamFn AddStream);
   Error runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
                    bool HasRegularLTO);
 
   mutable bool CalledGetMaxTasks = false;

llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp

void setValueGUID(uint64_t ValueID, StringRef ValueName,
                     StringRef SourceFileName);
   Error parseValueSymbolTable(
       uint64_t Offset,
       DenseMap<unsigned, GlobalValue::LinkageTypes> &ValueIdToLinkageMap);
   std::vector<ValueInfo> makeRefList(ArrayRef<uint64_t> Record);
   std::vector<FunctionSummary::EdgeTy> makeCallList(ArrayRef<uint64_t> Record,
                                                     bool IsOldProfileFormat,
                                                     bool HasProfile);
-  Error parseEntireSummary();
+  Error parseEntireSummary(unsigned ID);
   Error parseModuleStringTable();
 
   std::pair<ValueInfo, GlobalValue::GUID>
   getValueInfoFromValueId(unsigned ValueId);
 
   ModuleSummaryIndex::ModuleInfo *addThisModule();
 };
 
case BitstreamEntry::SubBlock:
         // is no summary section.
         assert(((SeenValueSymbolTable && VSTOffset > 0) ||
                 !SeenGlobalValSummary) &&
                "Expected early VST parse via VSTOffset record");
         if (Stream.SkipBlock())
           return error("Invalid record");
         break;
       case bitc::GLOBALVAL_SUMMARY_BLOCK_ID:
+      case bitc::FULL_LTO_GLOBALVAL_SUMMARY_BLOCK_ID:
         assert(!SeenValueSymbolTable &&
                "Already read VST when parsing summary block?");
         // 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 (Error Err = parseValueSymbolTable(VSTOffset, ValueIdToLinkageMap))
             return Err;
           SeenValueSymbolTable = true;
         }
         SeenGlobalValSummary = true;
-        if (Error Err = parseEntireSummary())
+        if (Error Err = parseEntireSummary(Entry.ID))
           return Err;
         break;
       case bitc::MODULE_STRTAB_BLOCK_ID:
         if (Error Err = parseModuleStringTable())
           return Err;
         break;
       }
       continue;
if (IsOldProfileFormat) {
       Hotness = static_cast<CalleeInfo::HotnessType>(Record[++I]);
     Ret.push_back(FunctionSummary::EdgeTy{Callee, CalleeInfo{Hotness}});
   }
   return Ret;
 }
 
 // Eagerly parse the entire summary block. This populates the GlobalValueSummary
 // objects in the index.
-Error ModuleSummaryIndexBitcodeReader::parseEntireSummary() {
-  if (Stream.EnterSubBlock(bitc::GLOBALVAL_SUMMARY_BLOCK_ID))
+Error ModuleSummaryIndexBitcodeReader::parseEntireSummary(unsigned ID) {
+  if (Stream.EnterSubBlock(ID))
     return error("Invalid record");
   SmallVector<uint64_t, 64> Record;
 
   // Parse version
   {
     BitstreamEntry Entry = Stream.advanceSkippingSubblocks();
     if (Entry.Kind != BitstreamEntry::Record)
       return error("Invalid Summary Block: record for version expected");

 
 Expected<std::unique_ptr<Module>>
 BitcodeModule::getLazyModule(LLVMContext &Context, bool ShouldLazyLoadMetadata,
                              bool IsImporting) {
   return getModuleImpl(Context, false, ShouldLazyLoadMetadata, IsImporting);
 }
 
 // Parse the specified bitcode buffer and merge the index into CombinedIndex.
+// We don't use ModuleIdentifier here because the client may need to control the
+// module path used in the combined summary (e.g. when reading summaries for
+// regular LTO modules).
 Error BitcodeModule::readSummary(ModuleSummaryIndex &CombinedIndex,
-                                 unsigned ModuleId) {
+                                 StringRef ModulePath, uint64_t ModuleId) {
   BitstreamCursor Stream(Buffer);
   Stream.JumpToBit(ModuleBit);
 
   ModuleSummaryIndexBitcodeReader R(std::move(Stream), Strtab, CombinedIndex,
-                                    ModuleIdentifier, ModuleId);
+                                    ModulePath, ModuleId);
   return R.parseModule();
 }
 
 // Parse the specified bitcode buffer, returning the function info index.
 Expected<std::unique_ptr<ModuleSummaryIndex>> BitcodeModule::getSummary() {
   BitstreamCursor Stream(Buffer);
   Stream.JumpToBit(ModuleBit);
 
   auto Index = llvm::make_unique<ModuleSummaryIndex>();
   ModuleSummaryIndexBitcodeReader R(std::move(Stream), Strtab, *Index,
                                     ModuleIdentifier, 0);
 
   if (Error Err = R.parseModule())
     return std::move(Err);
 
   return std::move(Index);
 }
 
 // Check if the given bitcode buffer contains a global value summary block.
-Expected<bool> BitcodeModule::hasSummary() {
+Expected<BitcodeLTOInfo> BitcodeModule::getLTOInfo() {
   BitstreamCursor Stream(Buffer);
   Stream.JumpToBit(ModuleBit);
 
   if (Stream.EnterSubBlock(bitc::MODULE_BLOCK_ID))
     return error("Invalid record");
 
   while (true) {
     BitstreamEntry Entry = Stream.advance();
 
     switch (Entry.Kind) {
     case BitstreamEntry::Error:
       return error("Malformed block");
     case BitstreamEntry::EndBlock:
-      return false;
+      return BitcodeLTOInfo{/*IsThinLTO=*/false, /*HasSummary=*/false};
 
     case BitstreamEntry::SubBlock:
       if (Entry.ID == bitc::GLOBALVAL_SUMMARY_BLOCK_ID)
-        return true;
+        return BitcodeLTOInfo{/*IsThinLTO=*/true, /*HasSummary=*/true};
+
+      if (Entry.ID == bitc::FULL_LTO_GLOBALVAL_SUMMARY_BLOCK_ID)
+        return BitcodeLTOInfo{/*IsThinLTO=*/false, /*HasSummary=*/true};
 
       // Ignore other sub-blocks.
       if (Stream.SkipBlock())
         return error("Malformed block");
       continue;
 
     case BitstreamEntry::Record:
       Stream.skipRecord(Entry.ID);
Expected<std::string> llvm::getBitcodeProducerString(MemoryBufferRef Buffer) {
   if (!StreamOrErr)
     return StreamOrErr.takeError();
 
   return readIdentificationCode(*StreamOrErr);
 }
 
 Error llvm::readModuleSummaryIndex(MemoryBufferRef Buffer,
                                    ModuleSummaryIndex &CombinedIndex,
-                                   unsigned ModuleId) {
+                                   uint64_t ModuleId) {
   Expected<BitcodeModule> BM = getSingleModule(Buffer);
   if (!BM)
     return BM.takeError();
 
-  return BM->readSummary(CombinedIndex, ModuleId);
+  return BM->readSummary(CombinedIndex, BM->getModuleIdentifier(), ModuleId);
 }
 
 Expected<std::unique_ptr<ModuleSummaryIndex>>
 llvm::getModuleSummaryIndex(MemoryBufferRef Buffer) {
   Expected<BitcodeModule> BM = getSingleModule(Buffer);
   if (!BM)
     return BM.takeError();
 
   return BM->getSummary();
 }
 
-Expected<bool> llvm::hasGlobalValueSummary(MemoryBufferRef Buffer) {
+Expected<BitcodeLTOInfo> llvm::getBitcodeLTOInfo(MemoryBufferRef Buffer) {
   Expected<BitcodeModule> BM = getSingleModule(Buffer);
   if (!BM)
     return BM.takeError();
 
-  return BM->hasSummary();
+  return BM->getLTOInfo();
 }
 
 Expected<std::unique_ptr<ModuleSummaryIndex>>
 llvm::getModuleSummaryIndexForFile(StringRef Path,
                                    bool IgnoreEmptyThinLTOIndexFile) {
   ErrorOr<std::unique_ptr<MemoryBuffer>> FileOrErr =
       MemoryBuffer::getFileOrSTDIN(Path);
   if (!FileOrErr)
     return errorCodeToError(FileOrErr.getError());
   if (IgnoreEmptyThinLTOIndexFile && !(*FileOrErr)->getBufferSize())
     return nullptr;
   return getModuleSummaryIndex(**FileOrErr);
 }

llvm/trunk/lib/IR/ModuleSummaryIndex.cpp

ModuleSummaryIndex::getGlobalValueSummary(uint64_t ValueGUID,
                                           bool PerModuleIndex) const {
   auto VI = getValueInfo(ValueGUID);
   assert(VI && "GlobalValue not found in index");
   assert((!PerModuleIndex || VI.getSummaryList().size() == 1) &&
          "Expected a single entry per global value in per-module index");
   auto &Summary = VI.getSummaryList()[0];
   return Summary.get();
 }
+
+bool ModuleSummaryIndex::isGUIDLive(GlobalValue::GUID GUID) const {
+  auto VI = getValueInfo(GUID);
+  if (!VI)
+    return false;
+  for (auto &I : VI.getSummaryList())
+    if (isGlobalValueLive(I.get()))
+      return true;
+  return false;
+}

llvm/trunk/lib/LTO/LTO.cpp

LTO::LTO(Config Conf, ThinBackend Backend,
          unsigned ParallelCodeGenParallelismLevel)
     : Conf(std::move(Conf)),
       RegularLTO(ParallelCodeGenParallelismLevel, this->Conf),
       ThinLTO(std::move(Backend)) {}
 
 // Requires a destructor for MapVector<BitcodeModule>.
 LTO::~LTO() = default;
 
-// Add the given symbol to the GlobalResolutions map, and resolve its partition.
-void LTO::addSymbolToGlobalRes(const InputFile::Symbol &Sym,
-                               SymbolResolution Res, unsigned Partition) {
+// Add the symbols in the given module to the GlobalResolutions map, and resolve
+// their partitions.
+void LTO::addModuleToGlobalRes(ArrayRef<InputFile::Symbol> Syms,
+                               ArrayRef<SymbolResolution> Res,
+                               unsigned Partition, bool InSummary) {
+  auto *ResI = Res.begin();
+  auto *ResE = Res.end();
+  for (const InputFile::Symbol &Sym : Syms) {
+    assert(ResI != ResE);
+    SymbolResolution Res = *ResI++;
+
     auto &GlobalRes = GlobalResolutions[Sym.getName()];
     GlobalRes.UnnamedAddr &= Sym.isUnnamedAddr();
     if (Res.Prevailing)
       GlobalRes.IRName = Sym.getIRName();
 
     // Set the partition to external if we know it is re-defined by the linker
     // with -defsym or -wrap options, used elsewhere, e.g. it is visible to a
     // regular object, is referenced from llvm.compiler_used, or was already
     // recorded as being referenced from a different partition.
     if (Res.LinkerRedefined || Res.VisibleToRegularObj || Sym.isUsed() ||
         (GlobalRes.Partition != GlobalResolution::Unknown &&
          GlobalRes.Partition != Partition)) {
       GlobalRes.Partition = GlobalResolution::External;
     } else
       // First recorded reference, save the current partition.
       GlobalRes.Partition = Partition;
 
-  // Flag as visible outside of ThinLTO if visible from a regular object or
-  // if this is a reference in the regular LTO partition.
-  GlobalRes.VisibleOutsideThinLTO |=
-      (Res.VisibleToRegularObj || Sym.isUsed() ||
-       Partition == GlobalResolution::RegularLTO);
+    // Flag as visible outside of summary if visible from a regular object or
+    // from a module that does not have a summary.
+    GlobalRes.VisibleOutsideSummary |=
+        (Res.VisibleToRegularObj || Sym.isUsed() || !InSummary);
+  }
 }
 
 static void writeToResolutionFile(raw_ostream &OS, InputFile *Input,
                                   ArrayRef<SymbolResolution> Res) {
   StringRef Path = Input->getName();
   OS << Path << '\n';
   auto ResI = Res.begin();
   for (const InputFile::Symbol &Sym : Input->symbols()) {
Error LTO::add(std::unique_ptr<InputFile> Input,
 
   assert(ResI == Res.end());
   return Error::success();
 }
 
 Error LTO::addModule(InputFile &Input, unsigned ModI,
                      const SymbolResolution *&ResI,
                      const SymbolResolution *ResE) {
-  Expected<bool> HasThinLTOSummary = Input.Mods[ModI].hasSummary();
-  if (!HasThinLTOSummary)
-    return HasThinLTOSummary.takeError();
+  Expected<BitcodeLTOInfo> LTOInfo = Input.Mods[ModI].getLTOInfo();
+  if (!LTOInfo)
+    return LTOInfo.takeError();
 
+  BitcodeModule BM = Input.Mods[ModI];
   auto ModSyms = Input.module_symbols(ModI);
-  if (*HasThinLTOSummary)
-    return addThinLTO(Input.Mods[ModI], ModSyms, ResI, ResE);
-  else
-    return addRegularLTO(Input.Mods[ModI], ModSyms, ResI, ResE);
+  addModuleToGlobalRes(ModSyms, {ResI, ResE},
+                       LTOInfo->IsThinLTO ? ThinLTO.ModuleMap.size() + 1 : 0,
+                       LTOInfo->HasSummary);
+
+  if (LTOInfo->IsThinLTO)
+    return addThinLTO(BM, ModSyms, ResI, ResE);
+
+  Expected<RegularLTOState::AddedModule> ModOrErr =
+      addRegularLTO(BM, ModSyms, ResI, ResE);
+  if (!ModOrErr)
+    return ModOrErr.takeError();
+
+  if (!LTOInfo->HasSummary)
+    return linkRegularLTO(std::move(*ModOrErr), /*LivenessFromIndex=*/false);
+
+  // Regular LTO module summaries are added to a dummy module that represents
+  // the combined regular LTO module.
+  if (Error Err = BM.readSummary(ThinLTO.CombinedIndex, "", -1ull))
+    return Err;
+  RegularLTO.ModsWithSummaries.push_back(std::move(*ModOrErr));
+  return Error::success();
 }
 
 // Add a regular LTO object to the link.
-Error LTO::addRegularLTO(BitcodeModule BM,
-                         ArrayRef<InputFile::Symbol> Syms,
-                         const SymbolResolution *&ResI,
-                         const SymbolResolution *ResE) {
-  if (!RegularLTO.CombinedModule) {
-    RegularLTO.CombinedModule =
-        llvm::make_unique<Module>("ld-temp.o", RegularLTO.Ctx);
-    RegularLTO.Mover = llvm::make_unique<IRMover>(*RegularLTO.CombinedModule);
-  }
+// The resulting module needs to be linked into the combined LTO module with
+// linkRegularLTO.
+Expected<LTO::RegularLTOState::AddedModule>
+LTO::addRegularLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
+  RegularLTOState::AddedModule Mod;
   Expected<std::unique_ptr<Module>> MOrErr =
       BM.getLazyModule(RegularLTO.Ctx, /*ShouldLazyLoadMetadata*/ true,
                        /*IsImporting*/ false);
   if (!MOrErr)
     return MOrErr.takeError();
-
   Module &M = **MOrErr;
+  Mod.M = std::move(*MOrErr);
+
   if (Error Err = M.materializeMetadata())
-    return Err;
+    return std::move(Err);
   UpgradeDebugInfo(M);
 
   ModuleSymbolTable SymTab;
   SymTab.addModule(&M);
 
-  std::vector<GlobalValue *> Keep;
-
   for (GlobalVariable &GV : M.globals())
     if (GV.hasAppendingLinkage())
-      Keep.push_back(&GV);
+      Mod.Keep.push_back(&GV);
 
   DenseSet<GlobalObject *> AliasedGlobals;
   for (auto &GA : M.aliases())
     if (GlobalObject *GO = GA.getBaseObject())
       AliasedGlobals.insert(GO);
 
   // In this function we need IR GlobalValues matching the symbols in Syms
   // (which is not backed by a module), so we need to enumerate them in the same
while (MsymI != MsymE) {
       ++MsymI;
     }
   };
   Skip();
 
   for (const InputFile::Symbol &Sym : Syms) {
     assert(ResI != ResE);
     SymbolResolution Res = *ResI++;
-    addSymbolToGlobalRes(Sym, Res, 0);
 
     assert(MsymI != MsymE);
     ModuleSymbolTable::Symbol Msym = *MsymI++;
     Skip();
 
     if (GlobalValue *GV = Msym.dyn_cast<GlobalValue *>()) {
       if (Res.Prevailing) {
         if (Sym.isUndefined())
           continue;
-        Keep.push_back(GV);
+        Mod.Keep.push_back(GV);
         // For symbols re-defined with linker -wrap and -defsym options,
         // set the linkage to weak to inhibit IPO. The linkage will be
         // restored by the linker.
         if (Res.LinkerRedefined)
           GV->setLinkage(GlobalValue::WeakAnyLinkage);
 
         GlobalValue::LinkageTypes OriginalLinkage = GV->getLinkage();
         if (GlobalValue::isLinkOnceLinkage(OriginalLinkage))
           GV->setLinkage(GlobalValue::getWeakLinkage(
               GlobalValue::isLinkOnceODRLinkage(OriginalLinkage)));
       } else if (isa<GlobalObject>(GV) &&
                  (GV->hasLinkOnceODRLinkage() || GV->hasWeakODRLinkage() ||
                   GV->hasAvailableExternallyLinkage()) &&
                  !AliasedGlobals.count(cast<GlobalObject>(GV))) {
-        // Either of the above three types of linkage indicates that the
+        // Any of the above three types of linkage indicates that the
         // chosen prevailing symbol will have the same semantics as this copy of
-        // the symbol, so we can link it with available_externally linkage. We
-        // only need to do this if the symbol is undefined.
-        GlobalValue *CombinedGV =
-            RegularLTO.CombinedModule->getNamedValue(GV->getName());
-        if (!CombinedGV || CombinedGV->isDeclaration()) {
-          Keep.push_back(GV);
-          GV->setLinkage(GlobalValue::AvailableExternallyLinkage);
-          cast<GlobalObject>(GV)->setComdat(nullptr);
-        }
-      }
-    }
+        // the symbol, so we may be able to link it with available_externally
+        // linkage. We will decide later whether to do that when we link this
+        // module (in linkRegularLTO), based on whether it is undefined.
+        Mod.Keep.push_back(GV);
     // Common resolution: collect the maximum size/alignment over all commons.
     // We also record if we see an instance of a common as prevailing, so that
     // if none is prevailing we can ignore it later.
     if (Sym.isCommon()) {
       // FIXME: We should figure out what to do about commons defined by asm.
       // For now they aren't reported correctly by ModuleSymbolTable.
       auto &CommonRes = RegularLTO.Commons[Sym.getIRName()];
       CommonRes.Size = std::max(CommonRes.Size, Sym.getCommonSize());
       CommonRes.Align = std::max(CommonRes.Align, Sym.getCommonAlignment());
       CommonRes.Prevailing |= Res.Prevailing;
     }
 
     // FIXME: use proposed local attribute for FinalDefinitionInLinkageUnit.
   }
   assert(MsymI == MsymE);
+  return std::move(Mod);
+}
 
-  return RegularLTO.Mover->move(std::move(*MOrErr), Keep,
+Error LTO::linkRegularLTO(RegularLTOState::AddedModule Mod,
+                          bool LivenessFromIndex) {
+  if (!RegularLTO.CombinedModule) {
+    RegularLTO.CombinedModule =
+        llvm::make_unique<Module>("ld-temp.o", RegularLTO.Ctx);
+    RegularLTO.Mover = llvm::make_unique<IRMover>(*RegularLTO.CombinedModule);
+  }
+
+  std::vector<GlobalValue *> Keep;
+  for (GlobalValue *GV : Mod.Keep) {
+    if (LivenessFromIndex && !ThinLTO.CombinedIndex.isGUIDLive(GV->getGUID()))
+      continue;
+
+    if (!GV->hasAvailableExternallyLinkage()) {
+      Keep.push_back(GV);
+      continue;
+    }
+
+    // Only link available_externally definitions if we don't already have a
+    // definition.
+    GlobalValue *CombinedGV =
+        RegularLTO.CombinedModule->getNamedValue(GV->getName());
+    if (CombinedGV && !CombinedGV->isDeclaration())
+      continue;
+
+    Keep.push_back(GV);
+  }
+
+  return RegularLTO.Mover->move(std::move(Mod.M), Keep,
                                 [](GlobalValue &, IRMover::ValueAdder) {},
                                 /* IsPerformingImport */ false);
 }
 
-// Add a ThinLTO object to the link.
-Error LTO::addThinLTO(BitcodeModule BM,
-                      ArrayRef<InputFile::Symbol> Syms,
+// Add a ThinLTO module to the link.
+Error LTO::addThinLTO(BitcodeModule BM, ArrayRef<InputFile::Symbol> Syms,
                       const SymbolResolution *&ResI,
                       const SymbolResolution *ResE) {
   if (Error Err =
-          BM.readSummary(ThinLTO.CombinedIndex, ThinLTO.ModuleMap.size()))
+          BM.readSummary(ThinLTO.CombinedIndex, BM.getModuleIdentifier(),
+                         ThinLTO.ModuleMap.size()))
     return Err;
 
   for (const InputFile::Symbol &Sym : Syms) {
     assert(ResI != ResE);
     SymbolResolution Res = *ResI++;
-    addSymbolToGlobalRes(Sym, Res, ThinLTO.ModuleMap.size() + 1);
 
     if (Res.Prevailing) {
       if (!Sym.getIRName().empty()) {
         auto GUID = GlobalValue::getGUID(GlobalValue::getGlobalIdentifier(
             Sym.getIRName(), GlobalValue::ExternalLinkage, ""));
         ThinLTO.PrevailingModuleForGUID[GUID] = BM.getModuleIdentifier();
       }
     }
unsigned LTO::getMaxTasks() const {
   CalledGetMaxTasks = true;
   return RegularLTO.ParallelCodeGenParallelismLevel + ThinLTO.ModuleMap.size();
 }
 
 Error LTO::run(AddStreamFn AddStream, NativeObjectCache Cache) {
   // Compute "dead" symbols, we don't want to import/export these!
   DenseSet<GlobalValue::GUID> GUIDPreservedSymbols;
   for (auto &Res : GlobalResolutions) {
-    if (Res.second.VisibleOutsideThinLTO &&
+    if (Res.second.VisibleOutsideSummary &&
         // IRName will be defined if we have seen the prevailing copy of
         // this value. If not, no need to preserve any ThinLTO copies.
         !Res.second.IRName.empty())
       GUIDPreservedSymbols.insert(GlobalValue::getGUID(
           GlobalValue::dropLLVMManglingEscape(Res.second.IRName)));
   }
 
   computeDeadSymbols(ThinLTO.CombinedIndex, GUIDPreservedSymbols);
 
   // Save the status of having a regularLTO combined module, as
   // this is needed for generating the ThinLTO Task ID, and
   // the CombinedModule will be moved at the end of runRegularLTO.
-  bool HasRegularLTO = RegularLTO.CombinedModule != nullptr;
+  bool HasRegularLTO = RegularLTO.CombinedModule != nullptr ||
+                       !RegularLTO.ModsWithSummaries.empty();
   // Invoke regular LTO if there was a regular LTO module to start with.
   if (HasRegularLTO)
     if (auto E = runRegularLTO(AddStream))
       return E;
   return runThinLTO(AddStream, Cache, HasRegularLTO);
 }
 
 Error LTO::runRegularLTO(AddStreamFn AddStream) {
+  for (auto &M : RegularLTO.ModsWithSummaries)
+    if (Error Err = linkRegularLTO(std::move(M),
+                                   /*LivenessFromIndex=*/true))
+      return Err;
+
   // Make sure commons have the right size/alignment: we kept the largest from
   // all the prevailing when adding the inputs, and we apply it here.
   const DataLayout &DL = RegularLTO.CombinedModule->getDataLayout();
   for (auto &I : RegularLTO.Commons) {
     if (!I.second.Prevailing)
       // Don't do anything if no instance of this common was prevailing.
       continue;
     GlobalVariable *OldGV = RegularLTO.CombinedModule->getNamedGlobal(I.first);
return [=](Config &Conf, ModuleSummaryIndex &CombinedIndex,
              const StringMap<GVSummaryMapTy> &ModuleToDefinedGVSummaries,
              AddStreamFn AddStream, NativeObjectCache Cache) {
     return llvm::make_unique<WriteIndexesThinBackend>(
         Conf, CombinedIndex, ModuleToDefinedGVSummaries, OldPrefix, NewPrefix,
         ShouldEmitImportsFiles, LinkedObjectsFile);
   };
 }
 
-static bool IsLiveByGUID(const ModuleSummaryIndex &Index,
-                         GlobalValue::GUID GUID) {
-  auto VI = Index.getValueInfo(GUID);
-  if (!VI)
-    return false;
-  for (auto &I : VI.getSummaryList())
-    if (Index.isGlobalValueLive(I.get()))
-      return true;
-  return false;
-}
-
 Error LTO::runThinLTO(AddStreamFn AddStream, NativeObjectCache Cache,
                       bool HasRegularLTO) {
   if (ThinLTO.ModuleMap.empty())
     return Error::success();
 
   if (Conf.CombinedIndexHook && !Conf.CombinedIndexHook(ThinLTO.CombinedIndex))
     return Error::success();
 
for (auto &Res : GlobalResolutions) {
       // IRName will be defined if we have seen the prevailing copy of
       // this value. If not, no need to mark as exported from a ThinLTO
       // partition (and we can't get the GUID).
       if (Res.second.IRName.empty())
         continue;
       auto GUID = GlobalValue::getGUID(
           GlobalValue::dropLLVMManglingEscape(Res.second.IRName));
       // Mark exported unless index-based analysis determined it to be dead.
-      if (IsLiveByGUID(ThinLTO.CombinedIndex, GUID))
+      if (ThinLTO.CombinedIndex.isGUIDLive(GUID))
         ExportedGUIDs.insert(GUID);
     }
 
     auto isExported = [&](StringRef ModuleIdentifier, GlobalValue::GUID GUID) {
       const auto &ExportList = ExportLists.find(ModuleIdentifier);
       return (ExportList != ExportLists.end() &&
               ExportList->second.count(GUID)) ||
              ExportedGUIDs.count(GUID);

llvm/trunk/lib/LTO/LTOModule.cpp

if (!BufferOrErr)
     return false;
 
   ErrorOr<MemoryBufferRef> BCData = IRObjectFile::findBitcodeInMemBuffer(
       BufferOrErr.get()->getMemBufferRef());
   return bool(BCData);
 }
 
 bool LTOModule::isThinLTO() {
-  // Right now the detection is only based on the summary presence. We may want
-  // to add a dedicated flag at some point.
-  Expected<bool> Result = hasGlobalValueSummary(MBRef);
+  Expected<BitcodeLTOInfo> Result = getBitcodeLTOInfo(MBRef);
   if (!Result) {
     logAllUnhandledErrors(Result.takeError(), errs(), "");
     return false;
   }
-  return *Result;
+  return Result->IsThinLTO;
 }
 
 bool LTOModule::isBitcodeForTarget(MemoryBuffer *Buffer,
                                    StringRef TriplePrefix) {
   ErrorOr<MemoryBufferRef> BCOrErr =
       IRObjectFile::findBitcodeInMemBuffer(Buffer->getMemBufferRef());
   if (!BCOrErr)
     return false;

llvm/trunk/test/LTO/Resolution/X86/Inputs/dead-strip-fulllto.ll

+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @live1() {
+  call void @live2()
+  ret void
+}
+
+declare void @live2()
+
+define void @dead1() {
+  call void @dead2()
+  ret void
+}
+
+declare void @dead2()

llvm/trunk/test/LTO/Resolution/X86/dead-strip-fulllto.ll

+; RUN: opt -module-summary -o %t %s
+; RUN: opt -module-summary -o %t2 %S/Inputs/dead-strip-fulllto.ll
+; RUN: llvm-lto2 run %t -r %t,main,px -r %t,live1,p -r %t,live2,p -r %t,dead2,p \
+; RUN:               %t2 -r %t2,live1,p -r %t2,live2, -r %t2,dead1,p -r %t2,dead2, \
+; RUN: -save-temps -o %t3
+; RUN: llvm-nm %t3.0 | FileCheck --check-prefix=FULL %s
+; RUN: llvm-nm %t3.1 | FileCheck --check-prefix=THIN %s
+
+; FULL-NOT: dead
+; FULL: U live1
+; FULL: T live2
+; FULL: T main
+
+; THIN-NOT: dead
+; THIN: T live1
+; THIN: U live2
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define void @main() {
+  call void @live1()
+  ret void
+}
+
+declare void @live1()
+
+define void @live2() {
+  ret void
+}
+
+define void @dead2() {
+  ret void
+}
+
+!0 = !{i32 1, !"ThinLTO", i32 0}
+!llvm.module.flags = !{ !0 }