ThinLTO Minimized Bitcode File Size Reduction

[haojiewang]

Summary

Currently the ThinLTO minimized bitcode file only strip the debug info, but there is still a lot of information in the minimized bit code file that will be not used for thin linker. In this patch, most of the extra information is striped to reduce the minimized bitcode file. Now only ModuleVersion, ModuleInfo, ModuleGlobalValueSummary, ModuleHash, Symtab and Strtab are left. Now the minimized bitcode file size is reduced to 15%-30% of the debug info stripped bitcode file size.

[mehdi_amini]

include/llvm/Bitcode/BitcodeWriter.h

87

s/constrction/construction/

lib/Bitcode/Writer/BitcodeWriter.cpp

100–103

s/Abstrac/Abstract/

But in fact I don't think it class is abstract, is it?

4007

Is a "module summary" described anywhere?

lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

424–425

This should be obsolete right now?

[haojiewang]

Change some comment. Remove StripDebugInfo function while writing the minimized bitcode file.

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

4007

I got that name from ModuleSummaryIndex, but it's not a good name. Already changed it to minimized bitcode file.

lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

424–425

Yes, I think so. Already removed it.

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

100–103

Yes, I also think so. I described it this way because the BitcodeWriterBase class's comment also says it is a abstract class, though I don't think it is. I was confused by this. Should I change both of them?

[mehdi_amini]

include/llvm/Bitcode/BitcodeWriter.h

89

Use a const reference then?

91

Same: if non-null required, why not using a reference?

144

Same?

lib/Bitcode/Writer/BitcodeWriter.cpp

4007

I have the same question for "minimized bitcode file" though, it may be some doc debt if this is already used in some place and not defined, just trying to flag these, not blocking your patch.

[haojiewang]

include/llvm/Bitcode/BitcodeWriter.h

89

But M is actually also required non-null. I just used the same args list with writeModule. So here using const references instead of pointer is better?

[pcc]

Please add a test that shows that the thinlink files can be read by llvm-lto2.

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

With this call you're emitting a lot of unnecessary information into the minimized bitcode file. As I was suggesting on the other thread I wonder whether you can just emit "short" module-level records that just contain the name and the linkage (maybe see if you can drop the linkage, as it should already be available in the summary).

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

49–68

This part of the test is no longer relevant.

50

I'd also check what we *are* emitting, not just what we're not emitting.

[mehdi_amini]

include/llvm/Bitcode/BitcodeWriter.h

89

I believe references carry the non_null information explicitly as part of the API.

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

Yes, this is what I'm going to do after this patch released. So do you suggest I do it in this patch or leave it to next patch?

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

49–68

You mean that I'd better write a new test file, right? Along with the llvm-lto2 test.

[haojiewang]

include/llvm/Bitcode/BitcodeWriter.h

89

Sure, I mean should I also change pointer M to reference or just change the Index and ModHash? And also for pointer M in writeModule?

[pcc]

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

I would suggest writing an initial patch that changes the index bitcode reader to read the linkage from the summary instead of from the module, and then once that lands do what I suggested in this patch.

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

49–68

I just mean that if you're now reading the file with llvm-bcanalyzer instead of llvm-dis you wouldn't expect the output to contain !llvm.dbg.cu either with or without your change, so it doesn't make sense to test for it. You can keep the rest of the test in this file.

Regarding llvm-lto2, you could either create a new test file or convert this one to use llvm-lto2 instead of llvm-lto.

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

If it is ok that we do that after this patch? Because I think module info have too much redundancy info for minimized bitcode file, most of which is necessary for IR bitcode file. We can add a new writeModuleInfo for minimized bitcode file after this patch, and then also change the bitcode reader. That can get 5%-10% benefit on size reduction.

[pcc]

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

Up to you but keep in mind that your code will be easier to review if you do things in the order I suggested, because we won't have to think about the design that we know you're going to replace, just the one that you're going to replace it with.

[tejohnson]

lib/Bitcode/Writer/BitcodeWriter.cpp

4007

The FE option is -fthin-link-bitcode, so perhaps ThinLinkBitcodeWriter? Admittedly I need to add documentation for this option.

4031

That change seems like an incremental follow on to this one. I.e. it affects the reader, whereas this patch just takes a first cut at reducing what we emit in the writer. So I am personally happy with this one going in first. Peter, are there things here that would be throwaway work if this went in first and that was a follow on?

[pcc]

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

I can accept that it may be easier to develop the patches in this order, but as I mentioned it makes things harder to review (which I guess is the "throwaway work" -- if we go with this first I would need to carefully look at the body of writeModuleInfo to make sure that running it wouldn't cause problems in this new flow, even though we know that it won't make a difference in the end).

Generally when I write patches I try to make things easier for the reviewer. But as I say, up to you folks.

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

I checked the code and found a problem for getting linkage from the summary instead of module info. The current logic is that we first insert the ValueID-ValueInfo map, which requires GUID(getting from name, linkage and source file name) and ValueID. This will be done after reading the module info. Then when reading summary, we can get GUID from that map. If we want to get the linkage for a value when reading summary, we cannot get the GUID until we read that value's summary. But this value may appear before this in a ref list or call list of other value, which requires the value info for it, while we don't have that yet. So if we want to do it this way, the logic may become more complex: we have to store all the ref list and call list to a ValueID-XXList map, and update them to SummaryIndex when we finish reading all the summary part. Is there another way to solve this problem?

[pcc]

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

I see. In that case I think it makes sense to forget about moving the linkage as an initial step and change this patch so that it creates short module records containing just the name and the linkage. That would address my main design concern with this patch.

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

I have two questions for shortening the module records:

1. If we only record the name and linkage, the index of linkage in module record will be changed(from 5 to 2). Should we change it only for summary or also change the original module info(put linkage right after the name)? We have to also support reading summary from full IR bitcode file, so I guess we should let them have the same format.
2. Currently there are two versions of module record(one is getting name from VST and another is getting name from strtab). Should we add a third version? Or obsolete the old ones?

[pcc]

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

1. I would just keep the linkage at offset 5 and use zeroes as padding.
2. If you do that there should be no need for a new version.

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

Oh, I see. Thank you. I will upload the update tomorrow.

[haojiewang]

Add writeSimplifiedModuleInfo to ThinLinkBitcodeWriter, now only the source filename, global value name and linkage will be writen while writing module info for a thin link bitcode file.
All the comments, names that contain "minimized bitcode file" and "module summary" are changed to "thin link bitcode".
Change the non-null pointer to reference.
Update the test for thin link bitcode file.

[haojiewang]

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

49–68

Current thin link bitcode cannot be read by llvm-lto2. I'm tracing the reason and trying to find how to fix it. Do we need fix this in this patch?

[pcc]

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

49–68

How are you calling llvm-lto2? You're passing -thinlto-distributed-indexes, right?

[haojiewang]

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

49–68

Actually no, I just used the same command I saw in other test because I'm now familiar with llvm-lto2. What test command should I use?

[pcc]

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

49–68

Probably something like http://llvm-cs.pcc.me.uk/test/ThinLTO/X86/distributed_import.ll#10 with resolutions for f and g.

[haojiewang]

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

49–68

llvm-lto2 works, but this test cannot past. Because the symtab will not be built in this test case. And the full IR bitcode file for this test also cannot pass llvm-lto2 because there is no data layout in this test. Should I change this test to a more complete one or just leave it here and add another test?

[pcc]

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

49–68

I'd add a triple and datalayout to this test, it probably ought to have one anyway.

[haojiewang]

Update test for llvm-lto2.

[haojiewang]

Update the test for llvm-lto2.

[haojiewang]

Friendly ping

[tejohnson]

This looks pretty good to me now, a few nits below. Please wait to see if Peter has any more comments.

include/llvm/Bitcode/BitcodeWriter.h

87

Elsewhere we describe this as the minimized bitcode file used for the thin link, can you mention that for your 2 interfaces? Specifically what is missing is that these are writing just what is necessary for the thin link.

90

nit: s/generating/generated/

133

ditto

lib/Bitcode/Writer/BitcodeWriter.cpp

4031

nit: s/using/used/

lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

424–425

nit: braces can be removed now.

[tejohnson]

lib/Bitcode/Writer/BitcodeWriter.cpp

101

Just noticed, this should be ThinLinkBitcodeWriter now.

[haojiewang]

Update some comments.

[pcc]

include/llvm/Bitcode/BitcodeWriter.h

93

clang-format

136

clang-format

lib/Bitcode/Writer/BitcodeWriter.cpp

114

I think we can move this into the derived class now, together with Hasher and addToStrtab.

124

I think this change removes the need to support writing a module with a precomputed hash from ModuleBitcodeWriter. Can you please remove that functionality?

142

clang-format

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

49–68

Sorry, I clearly wasn't paying attention when I referred you to the distributed_import.ll test above, as that test is testing for pretty much exactly what I thought wasn't already being tested for. May I ask you to remove the llvm-lto2 part and move the llvm-bcanalyzer part of this test to distributed_import.ll?

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

114

Oh, sure. But I think this change will require a new addToStrtab for ThinLinkBitcodeWriter, which only add the str to StrtabBuilder without update GenerateHash.

124

Writing a module with a precomputed hash is still needed while split writing a module(splitAndWriteThinLTOBitcode), because I didn't touch the split writing in this patch. Maybe I should also change the writer in split writing?

[pcc]

lib/Bitcode/Writer/BitcodeWriter.cpp

114

You can just change the callers to call StrtabBuilder.add(), that's what we did before we needed to add addToStrtab to fix the caching problem in ModuleBitcodeWriter.

124

You mean here?
http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp#386

Yes, I think you can change that to call writeThinLinkBitcode.

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

124

I just tried to change the split writing, but there were bunch of errors in test after I did that. There will be 11 test files should be updated if we change this, and most of them are because llvm-dis cannot read a thin link bitcode file. Should we do it in this patch or leave it the next patch? How should we update them, just changing llvm-dis to llvm-bcanalyzer or also adding the CHECKs and CHECK-NOTs as in the test file of this patch?

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

124

I'm just not sure if I would affect other people's code if I do so much changes on the test files.

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

124

Sorry, I just made a terrible mistake on this one. Actually there would be only 1 test file should be updated, which is for the llvm-dis problem. Besides, for the "NODEBUG" checks, I can just remove them because they are not needed after this patch, right? And after I add the CHECKs and CHECK-NOTs in distributed-import.ll, I don't have to do the check again for the split.ll test file, right?

[pcc]

lib/Bitcode/Writer/BitcodeWriter.cpp

124

Yes, you can remove the NODEBUG checks from that file, and no other changes should be required there since we're already making sure we can read the resulting files with llvm-lto (ideally it should be changed to use llvm-lto2, but that can be done some other time).

[haojiewang]

1. Format the code using clang-format.
2. Move GenerateHash, Hasher, addToStrtab from ModuleBitcodeWriterBase to ModuleBitcodeWriter since they are not really used in ThinLinkBitcodeWriter. Now ThinLinkBitcodeWriter just use StrtabBuilder.add() directly to write symbol names to Strtab.
3. Update splitAndWriteThinLTOBitcode so that only the necessary information will be written into ThinLTO parts.
4. Functionally remove writing a module with a precomputed module hash.
5. Update the test files. Remove the NODEBUG tests in split.ll and no-type-md.ll. Thin link bitcode file content check is moved into distributed-import.ll.

[pcc]

lib/Bitcode/Writer/BitcodeWriter.cpp

119

This comment doesn't really make sense anymore in the context of the base class. I'd replace the ModHash field in this class with a ModHash field in both derived classes with their own comments, since they really serve different purposes in each class.

4021

Inline into caller.

lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

380

Update comment

424–425

Update comment

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

48–68

You can remove everything from this line onwards I think.

test/Transforms/ThinLTOBitcodeWriter/split.ll

46 ↗

(On Diff #107227)

Likewise.

[haojiewang]

1. Move ModHash to derived classes.
2. Inline a one line function.
3. Comments update.
4. Tests update.

[pcc]

LGTM

lib/Bitcode/Writer/BitcodeWriter.cpp

212

I'd remove the second sentence.

4010

I'd reword as "generated while writing the module bitcode file".

4011

This could be const ModuleHash &ModHash;.

4059

Remove blank line.

4124

This comment isn't very useful, it just says the same thing as the code. I'd remove it.

I think the other comments in this function are self evident from the code as well, but up to you if you want to keep them.

[haojiewang]

1. Change ModHash in ThinLinkBitcodeWriter, writeThinLinkBitcode and WriteThinLinkBitcodeToFile to const.
2. Update some comments.

[haojiewang]

lib/Bitcode/Writer/BitcodeWriter.cpp

4124

I just keep the module hash comment, since I inline that function.

include/llvm/Bitcode/BitcodeWriter.h

public:
     /// is used as the hash instead of computing from the generated bitcode.
     /// Can be used to produce the same module hash for a minimized bitcode
     /// used just for the thin link as in the regular full bitcode that will
     /// be used in the backend.
     void writeModule(const Module *M, bool ShouldPreserveUseListOrder = false,
                      const ModuleSummaryIndex *Index = nullptr,
                      bool GenerateHash = false, ModuleHash *ModHash = nullptr);
 
+    /// Write the specified thin link bitcode file (i.e., the minimized bitcode 
+    /// file) to the buffer specified at construction time. The thin link 
+    /// bitcode file is used for thin link, and it only contains the necessary
+    /// information for thin link.
+    /// 
+    /// ModHash is for use in ThinLTO incremental build, generated while the 
+    /// IR bitcode file writing.
+    void writeThinLinkBitcode(const Module *M, const ModuleSummaryIndex &Index, 
+                      ModuleHash &ModHash);
+
     void writeIndex(
         const ModuleSummaryIndex *Index,
         const std::map<std::string, GVSummaryMapTy> *ModuleToSummariesForIndex);
   };
 
   /// \brief Write the specified module to the specified raw output stream.
   ///
   /// For streams where it matters, the given stream should be in "binary"
namespace llvm {
   /// used just for the thin link as in the regular full bitcode that will
   /// be used in the backend.
   void WriteBitcodeToFile(const Module *M, raw_ostream &Out,
                           bool ShouldPreserveUseListOrder = false,
                           const ModuleSummaryIndex *Index = nullptr,
                           bool GenerateHash = false,
                           ModuleHash *ModHash = nullptr);
 
+  /// Write the specified thin link bitcode file (i.e., the minimized bitcode
+  /// file) to the given raw output stream, where it will be written in a new
+  /// bitcode block. The thin link bitcode file is used for thin link, and it
+  /// only contains the necessary information for thin link.
+  /// 
+  /// ModHash is for use in ThinLTO incremental build, generated while the IR
+  /// bitcode file writing.
+  void WriteThinLinkBitcodeToFile(const Module *M, raw_ostream &Out,
+                          const ModuleSummaryIndex &Index,
+                          ModuleHash &ModHash);
+
   /// Write the specified module summary index to the given raw output stream,
   /// where it will be written in a new bitcode block. This is used when
   /// writing the combined index file for ThinLTO. When writing a subset of the
   /// index for a distributed backend, provide the \p ModuleToSummariesForIndex
   /// map.
   void WriteIndexToFile(const ModuleSummaryIndex &Index, raw_ostream &Out,
                         const std::map<std::string, GVSummaryMapTy>
                             *ModuleToSummariesForIndex = nullptr);
 } // End llvm namespace
 
 #endif

lib/Bitcode/Writer/BitcodeWriter.cpp

protected:
   void writeModuleVersion();
 };
 
 void BitcodeWriterBase::writeModuleVersion() {
   // VERSION: [version#]
   Stream.EmitRecord(bitc::MODULE_CODE_VERSION, ArrayRef<uint64_t>{2});
 }
 
-/// Class to manage the bitcode writing for a module.
-class ModuleBitcodeWriter : public BitcodeWriterBase {
-  /// Pointer to the buffer allocated by caller for bitcode writing.
-  const SmallVectorImpl<char> &Buffer;
-
+/// Base class to manage the module bitcode writing, currently subclassed for
+/// ModuleBitcodeWriter and ThinLinkBitcodeWriter.
+class ModuleBitcodeWriterBase : public BitcodeWriterBase {
+protected:
   /// The Module to write to bitcode.
   const Module &M;
 
   /// Enumerates ids for all values in the module.
   ValueEnumerator VE;
 
   /// Optional per-module index to write for ThinLTO.
   const ModuleSummaryIndex *Index;
 
   /// True if a module hash record should be written.
   bool GenerateHash;
 
   SHA1 Hasher;
 
   /// If non-null, when GenerateHash is true, the resulting hash is written
   /// into ModHash. When GenerateHash is false, that specified value
   /// is used as the hash instead of computing from the generated bitcode.
   /// Can be used to produce the same module hash for a minimized bitcode
   /// used just for the thin link as in the regular full bitcode that will
   /// be used in the backend.
   ModuleHash *ModHash;
 
-  /// The start bit of the identification block.
-  uint64_t BitcodeStartBit;
-
   /// Map that holds the correspondence between GUIDs in the summary index,
   /// that came from indirect call profiles, and a value id generated by this
   /// class to use in the VST and summary block records.
   std::map<GlobalValue::GUID, unsigned> GUIDToValueIdMap;
 
   /// Tracks the last value id recorded in the GUIDToValueMap.
   unsigned GlobalValueId;
 
   /// Saves the offset of the VSTOffset record that must eventually be
   /// backpatched with the offset of the actual VST.
   uint64_t VSTOffsetPlaceholder = 0;
 
 public:
-  /// Constructs a ModuleBitcodeWriter object for the given Module,
+  /// Constructs a ModuleBitcodeWriterBase object for the given Module,
   /// writing to the provided \p Buffer.
-  ModuleBitcodeWriter(const Module *M, SmallVectorImpl<char> &Buffer,
-                      StringTableBuilder &StrtabBuilder,
+  ModuleBitcodeWriterBase(const Module *M, StringTableBuilder &StrtabBuilder,
                       BitstreamWriter &Stream, bool ShouldPreserveUseListOrder,
                       const ModuleSummaryIndex *Index, bool GenerateHash,
                       ModuleHash *ModHash = nullptr)
-      : BitcodeWriterBase(Stream, StrtabBuilder), Buffer(Buffer), M(*M),
+      : BitcodeWriterBase(Stream, StrtabBuilder), M(*M),
         VE(*M, ShouldPreserveUseListOrder), Index(Index),
-        GenerateHash(GenerateHash), ModHash(ModHash),
-        BitcodeStartBit(Stream.GetCurrentBitNo()) {
+        GenerateHash(GenerateHash), ModHash(ModHash) {
     // Assign ValueIds to any callee values in the index that came from
     // indirect call profiles and were recorded as a GUID not a Value*
     // (which would have been assigned an ID by the ValueEnumerator).
     // The starting ValueId is just after the number of values in the
     // ValueEnumerator, so that they can be emitted in the VST.
     GlobalValueId = VE.getValues().size();
     if (!Index)
       return;
     for (const auto &GUIDSummaryLists : *Index)
       // Examine all summaries for this GUID.
       for (auto &Summary : GUIDSummaryLists.second.SummaryList)
         if (auto FS = dyn_cast<FunctionSummary>(Summary.get()))
           // For each call in the function summary, see if the call
           // is to a GUID (which means it is for an indirect call,
           // otherwise we would have a Value for it). If so, synthesize
           // a value id.
           for (auto &CallEdge : FS->calls())
             if (!CallEdge.first.getValue())
               assignValueId(CallEdge.first.getGUID());
   }
 
+protected:
+  size_t addToStrtab(StringRef Str);
+
+  void writePerModuleGlobalValueSummary();
+
+private:
+  void writePerModuleFunctionSummaryRecord(SmallVector<uint64_t, 64> &NameVals,
+                                           GlobalValueSummary *Summary,
+                                           unsigned ValueID,
+                                           unsigned FSCallsAbbrev,
+                                           unsigned FSCallsProfileAbbrev,
+                                           const Function &F);
+  void writeModuleLevelReferences(const GlobalVariable &V,
+                                  SmallVector<uint64_t, 64> &NameVals,
+                                  unsigned FSModRefsAbbrev);
+
+  void assignValueId(GlobalValue::GUID ValGUID) {
+    GUIDToValueIdMap[ValGUID] = ++GlobalValueId;
+  }
+  unsigned getValueId(GlobalValue::GUID ValGUID) {
+    const auto &VMI = GUIDToValueIdMap.find(ValGUID);
+    // Expect that any GUID value had a value Id assigned by an
+    // earlier call to assignValueId.
+    assert(VMI != GUIDToValueIdMap.end() &&
+           "GUID does not have assigned value Id");
+    return VMI->second;
+  }
+  // Helper to get the valueId for the type of value recorded in VI.
+  unsigned getValueId(ValueInfo VI) {
+    if (!VI.getValue())
+      return getValueId(VI.getGUID());
+    return VE.getValueID(VI.getValue());
+  }
+  std::map<GlobalValue::GUID, unsigned> &valueIds() { return GUIDToValueIdMap; }
+};
+
+/// Class to manage the bitcode writing for a module.
+class ModuleBitcodeWriter : public ModuleBitcodeWriterBase {
+  /// Pointer to the buffer allocated by caller for bitcode writing.
+  const SmallVectorImpl<char> &Buffer;
+
+  /// The start bit of the identification block.
+  uint64_t BitcodeStartBit;
+
+public:
+  /// Constructs a ModuleBitcodeWriter object for the given Module,
+  /// writing to the provided \p Buffer.
+  ModuleBitcodeWriter(const Module *M, SmallVectorImpl<char> &Buffer,
+                      StringTableBuilder &StrtabBuilder,
+                      BitstreamWriter &Stream, bool ShouldPreserveUseListOrder,
+                      const ModuleSummaryIndex *Index, bool GenerateHash,
+                      ModuleHash *ModHash = nullptr)
+      : ModuleBitcodeWriterBase(M, StrtabBuilder, Stream, ShouldPreserveUseListOrder,
+                                Index, GenerateHash, ModHash), 
+        Buffer(Buffer), BitcodeStartBit(Stream.GetCurrentBitNo()) { }
+
   /// Emit the current module to the bitstream.
   void write();
 
 private:
   uint64_t bitcodeStartBit() { return BitcodeStartBit; }
 
-  size_t addToStrtab(StringRef Str);
-
   void writeAttributeGroupTable();
   void writeAttributeTable();
   void writeTypeTable();
   void writeComdats();
   void writeValueSymbolTableForwardDecl();
   void writeModuleInfo();
   void writeValueAsMetadata(const ValueAsMetadata *MD,
                             SmallVectorImpl<uint64_t> &Record);
private:
   void writeGlobalValueSymbolTable(
       DenseMap<const Function *, uint64_t> &FunctionToBitcodeIndex);
   void writeUseList(UseListOrder &&Order);
   void writeUseListBlock(const Function *F);
   void
   writeFunction(const Function &F,
                 DenseMap<const Function *, uint64_t> &FunctionToBitcodeIndex);
   void writeBlockInfo();
-  void writePerModuleFunctionSummaryRecord(SmallVector<uint64_t, 64> &NameVals,
-                                           GlobalValueSummary *Summary,
-                                           unsigned ValueID,
-                                           unsigned FSCallsAbbrev,
-                                           unsigned FSCallsProfileAbbrev,
-                                           const Function &F);
-  void writeModuleLevelReferences(const GlobalVariable &V,
-                                  SmallVector<uint64_t, 64> &NameVals,
-                                  unsigned FSModRefsAbbrev);
-  void writePerModuleGlobalValueSummary();
   void writeModuleHash(size_t BlockStartPos);
 
-  void assignValueId(GlobalValue::GUID ValGUID) {
-    GUIDToValueIdMap[ValGUID] = ++GlobalValueId;
-  }
-  unsigned getValueId(GlobalValue::GUID ValGUID) {
-    const auto &VMI = GUIDToValueIdMap.find(ValGUID);
-    // Expect that any GUID value had a value Id assigned by an
-    // earlier call to assignValueId.
-    assert(VMI != GUIDToValueIdMap.end() &&
-           "GUID does not have assigned value Id");
-    return VMI->second;
-  }
-  // Helper to get the valueId for the type of value recorded in VI.
-  unsigned getValueId(ValueInfo VI) {
-    if (!VI.getValue())
-      return getValueId(VI.getGUID());
-    return VE.getValueID(VI.getValue());
-  }
-  std::map<GlobalValue::GUID, unsigned> &valueIds() { return GUIDToValueIdMap; }
-
   unsigned getEncodedSyncScopeID(SyncScope::ID SSID) {
     return unsigned(SSID);
   }
 };
 
 /// Class to manage the bitcode writing for a combined index.
 class IndexBitcodeWriter : public BitcodeWriterBase {
   /// The combined index to write to bitcode.
static unsigned getEncodedUnnamedAddr(const GlobalValue &GV) {
   switch (GV.getUnnamedAddr()) {
   case GlobalValue::UnnamedAddr::None:   return 0;
   case GlobalValue::UnnamedAddr::Local:  return 2;
   case GlobalValue::UnnamedAddr::Global: return 1;
   }
   llvm_unreachable("Invalid unnamed_addr");
 }
 
-size_t ModuleBitcodeWriter::addToStrtab(StringRef Str) {
+size_t ModuleBitcodeWriterBase::addToStrtab(StringRef Str) {
   if (GenerateHash)
     Hasher.update(Str);
   return StrtabBuilder.add(Str);
 }
 
 void ModuleBitcodeWriter::writeComdats() {
   SmallVector<unsigned, 64> Vals;
   for (const Comdat *C : VE.getComdats()) {
static void writeFunctionTypeMetadataRecords(BitstreamWriter &Stream,
 
   WriteConstVCallVec(bitc::FS_TYPE_TEST_ASSUME_CONST_VCALL,
                      FS->type_test_assume_const_vcalls());
   WriteConstVCallVec(bitc::FS_TYPE_CHECKED_LOAD_CONST_VCALL,
                      FS->type_checked_load_const_vcalls());
 }
 
 // Helper to emit a single function summary record.
-void ModuleBitcodeWriter::writePerModuleFunctionSummaryRecord(
+void ModuleBitcodeWriterBase::writePerModuleFunctionSummaryRecord(
     SmallVector<uint64_t, 64> &NameVals, GlobalValueSummary *Summary,
     unsigned ValueID, unsigned FSCallsAbbrev, unsigned FSCallsProfileAbbrev,
     const Function &F) {
   NameVals.push_back(ValueID);
 
   FunctionSummary *FS = cast<FunctionSummary>(Summary);
   writeFunctionTypeMetadataRecords(Stream, FS);
 
void ModuleBitcodeWriterBase::writePerModuleFunctionSummaryRecord(
 
   // Emit the finished record.
   Stream.EmitRecord(Code, NameVals, FSAbbrev);
   NameVals.clear();
 }
 
 // Collect the global value references in the given variable's initializer,
 // and emit them in a summary record.
-void ModuleBitcodeWriter::writeModuleLevelReferences(
+void ModuleBitcodeWriterBase::writeModuleLevelReferences(
     const GlobalVariable &V, SmallVector<uint64_t, 64> &NameVals,
     unsigned FSModRefsAbbrev) {
   auto VI = Index->getValueInfo(GlobalValue::getGUID(V.getName()));
   if (!VI || VI.getSummaryList().empty()) {
     // Only declarations should not have a summary (a declaration might however
     // have a summary if the def was in module level asm).
     assert(V.isDeclaration());
     return;

 
 // Current version for the summary.
 // This is bumped whenever we introduce changes in the way some record are
 // interpreted, like flags for instance.
 static const uint64_t INDEX_VERSION = 3;
 
 /// Emit the per-module summary section alongside the rest of
 /// the module's bitcode.
-void ModuleBitcodeWriter::writePerModuleGlobalValueSummary() {
+void ModuleBitcodeWriterBase::writePerModuleGlobalValueSummary() {
   // By default we compile with ThinLTO if the module has a summary, but the
   // client can request full LTO with a module flag.
   bool IsThinLTO = true;
   if (auto *MD =
           mdconst::extract_or_null<ConstantInt>(M.getModuleFlag("ThinLTO")))
     IsThinLTO = MD->getZExtValue();
   Stream.EnterSubblock(IsThinLTO ? bitc::GLOBALVAL_SUMMARY_BLOCK_ID
                                  : bitc::FULL_LTO_GLOBALVAL_SUMMARY_BLOCK_ID,
void llvm::WriteIndexToFile(
   Buffer.reserve(256 * 1024);
 
   BitcodeWriter Writer(Buffer);
   Writer.writeIndex(&Index, ModuleToSummariesForIndex);
   Writer.writeStrtab();
 
   Out.write((char *)&Buffer.front(), Buffer.size());
 }
+
+/// Class to manage the bitcode writing for a thin link bitcode file.
+class ThinLinkBitcodeWriter : public ModuleBitcodeWriterBase {
+public:
+  ThinLinkBitcodeWriter(const Module *M, StringTableBuilder &StrtabBuilder,
+                          BitstreamWriter &Stream, const ModuleSummaryIndex &Index, 
+                          ModuleHash &ModHash) 
+        : ModuleBitcodeWriterBase(M, StrtabBuilder, Stream, 
+                                  /*ShouldPreserveUseListOrder=*/ false,
+                                  &Index, 
+                                  /*GenerateHash=*/ false, &ModHash) { }
+
+  void write();
+
+private:
+  void writeModuleHash() {
+    Stream.EmitRecord(bitc::MODULE_CODE_HASH, ArrayRef<uint32_t>(*ModHash));
+  }
+
+  void writeSimplifiedModuleInfo();
+};
+
+// This function writes a simpilified module info for thin link bitcode file.
+// It only contains the source file name along with the name(the offset and
+// size in strtab) and linkage for global values. For the global value info
+// entry, in order to keep linkage at offset 5, there are three zeros used
+// as padding.
+void ThinLinkBitcodeWriter::writeSimplifiedModuleInfo() {
+  SmallVector<unsigned, 64> Vals;
+  // Emit the module's source file name.
+  {
+    StringEncoding Bits = getStringEncoding(M.getSourceFileName());
+    BitCodeAbbrevOp AbbrevOpToUse = BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 8);
+    if (Bits == SE_Char6)
+      AbbrevOpToUse = BitCodeAbbrevOp(BitCodeAbbrevOp::Char6);
+    else if (Bits == SE_Fixed7)
+      AbbrevOpToUse = BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 7);
+
+    // MODULE_CODE_SOURCE_FILENAME: [namechar x N]
+    auto Abbv = std::make_shared<BitCodeAbbrev>();
+    Abbv->Add(BitCodeAbbrevOp(bitc::MODULE_CODE_SOURCE_FILENAME));
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
+    Abbv->Add(AbbrevOpToUse);
+    unsigned FilenameAbbrev = Stream.EmitAbbrev(std::move(Abbv));
+
+    for (const auto P : M.getSourceFileName())
+      Vals.push_back((unsigned char)P);
+
+    Stream.EmitRecord(bitc::MODULE_CODE_SOURCE_FILENAME, Vals, FilenameAbbrev);
+    Vals.clear();
+  }
+
+  // Emit the global variable information.
+  for (const GlobalVariable &GV : M.globals()) {
+
+    // GLOBALVAR: [strtab offset, strtab size, 0, 0, 0, linkage]
+    Vals.push_back(addToStrtab(GV.getName()));
+    Vals.push_back(GV.getName().size());
+    Vals.push_back(0);
+    Vals.push_back(0);
+    Vals.push_back(0);
+    Vals.push_back(getEncodedLinkage(GV));
+
+    Stream.EmitRecord(bitc::MODULE_CODE_GLOBALVAR, Vals);
+    Vals.clear();
+  }
+
+  // Emit the function proto information.
+  for (const Function &F : M) {
+    // FUNCTION:  [strtab offset, strtab size, 0, 0, 0, linkage]
+    Vals.push_back(addToStrtab(F.getName()));
+    Vals.push_back(F.getName().size());
+    Vals.push_back(0);
+    Vals.push_back(0);
+    Vals.push_back(0);
+    Vals.push_back(getEncodedLinkage(F));
+
+    Stream.EmitRecord(bitc::MODULE_CODE_FUNCTION, Vals);
+    Vals.clear();
+  }
+
+  // Emit the alias information.
+  for (const GlobalAlias &A : M.aliases()) {
+    // ALIAS: [strtab offset, strtab size, 0, 0, 0, linkage]
+    Vals.push_back(addToStrtab(A.getName()));
+    Vals.push_back(A.getName().size());
+    Vals.push_back(0);
+    Vals.push_back(0);
+    Vals.push_back(0);
+    Vals.push_back(getEncodedLinkage(A));
+
+    Stream.EmitRecord(bitc::MODULE_CODE_ALIAS, Vals);
+    Vals.clear();
+  }
+
+  // Emit the ifunc information.
+  for (const GlobalIFunc &I : M.ifuncs()) {
+    // IFUNC: [strtab offset, strtab size, 0, 0, 0, linkage]
+    Vals.push_back(addToStrtab(I.getName()));
+    Vals.push_back(I.getName().size());
+    Vals.push_back(0);
+    Vals.push_back(0);
+    Vals.push_back(0);
+    Vals.push_back(getEncodedLinkage(I));
+
+    Stream.EmitRecord(bitc::MODULE_CODE_IFUNC, Vals);
+    Vals.clear();
+  }
+}
+
+void ThinLinkBitcodeWriter::write() {
+  Stream.EnterSubblock(bitc::MODULE_BLOCK_ID, 3);
+
+  writeModuleVersion();
+
+  writeSimplifiedModuleInfo();
+
+  writePerModuleGlobalValueSummary();
+
+  writeModuleHash();
+
+  Stream.ExitBlock();
+}
+
+void BitcodeWriter::writeThinLinkBitcode(const Module *M,const ModuleSummaryIndex &Index,
+                                 ModuleHash &ModHash) {
+  assert(!WroteStrtab);
+
+  // The Mods vector is used by irsymtab::build, which requires non-const
+  // Modules in case it needs to materialize metadata. But the bitcode writer
+  // requires that the module is materialized, so we can cast to non-const here,
+  // after checking that it is in fact materialized.
+  assert(M->isMaterialized());
+  Mods.push_back(const_cast<Module *>(M));
+
+  ThinLinkBitcodeWriter ThinLinkWriter(M, StrtabBuilder, *Stream, Index, ModHash);
+  ThinLinkWriter.write();
+}
+
+// Write the specified thin link bitcode file to the given raw output stream,
+// where it will be written in a new bitcode block. This is used when
+// writing the per-module index file for ThinLTO.
+void llvm::WriteThinLinkBitcodeToFile(const Module *M, raw_ostream &Out,
+                              const ModuleSummaryIndex &Index,
+                              ModuleHash &ModHash) {
+  SmallVector<char, 0> Buffer;
+  Buffer.reserve(256 * 1024);
+
+  BitcodeWriter Writer(Buffer);
+  Writer.writeThinLinkBitcode(M, Index, ModHash);
+  Writer.writeSymtab();
+  Writer.writeStrtab();
+
+  Out.write((char *)&Buffer.front(), Buffer.size());
+}

lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp

W.writeModule(&M, /*ShouldPreserveUseListOrder=*/false, &Index,
                 /*GenerateHash=*/true, &ModHash);
   W.writeModule(MergedM.get(), /*ShouldPreserveUseListOrder=*/false,
                 &MergedMIndex);
   W.writeSymtab();
   W.writeStrtab();
   OS << Buffer;
 
   // If a minimized bitcode module was requested for the thin link,
   // strip the debug info (the merged module was already stripped above)
   // and write it to the given OS.
   if (ThinLinkOS) {
     Buffer.clear();
     BitcodeWriter W2(Buffer);
     StripDebugInfo(M);
     W2.writeModule(&M, /*ShouldPreserveUseListOrder=*/false, &Index,
                    /*GenerateHash=*/false, &ModHash);
     W2.writeModule(MergedM.get(), /*ShouldPreserveUseListOrder=*/false,
void writeThinLTOBitcode(raw_ostream &OS, raw_ostream *ThinLinkOS,
 
   // Save the module hash produced for the full bitcode, which will
   // be used in the backends, and use that in the minimized bitcode
   // produced for the full link.
   ModuleHash ModHash = {{0}};
   WriteBitcodeToFile(&M, OS, /*ShouldPreserveUseListOrder=*/false, Index,
                      /*GenerateHash=*/true, &ModHash);
   // If a minimized bitcode module was requested for the thin link,
   // strip the debug info and write it to the given OS.
-  if (ThinLinkOS) {
-    StripDebugInfo(M);
-    WriteBitcodeToFile(&M, *ThinLinkOS, /*ShouldPreserveUseListOrder=*/false,
-                       Index,
-                       /*GenerateHash=*/false, &ModHash);
-  }
+  if (ThinLinkOS && Index) 
+    WriteThinLinkBitcodeToFile(&M, *ThinLinkOS, *Index, ModHash);
 }
 
 class WriteThinLTOBitcode : public ModulePass {
   raw_ostream &OS; // raw_ostream to print on
   // The output stream on which to emit a minimized module for use
   // just in the thin link, if requested.
   raw_ostream *ThinLinkOS;
 

test/Transforms/ThinLTOBitcodeWriter/no-type-md.ll

 ; Generate bitcode files with summary, as well as minimized bitcode without
 ; the debug metadata for the thin link.
 ; RUN: opt -thinlto-bc -thin-link-bitcode-file=%t.thinlink.bc -o %t.bc %s
 ; RUN: llvm-dis -o - %t.bc | FileCheck %s
-; RUN: llvm-dis -o - %t.thinlink.bc | FileCheck --check-prefix=NODEBUG %s
+; RUN: llvm-bcanalyzer -dump %t.thinlink.bc | FileCheck --check-prefix=NODEBUG %s
 ; RUN: llvm-bcanalyzer -dump %t.bc | FileCheck --check-prefix=BCA %s
 
 ; Make sure the combined index files produced by both the normal and the
 ; thin link bitcode files are identical
 ; RUN: llvm-lto -thinlto -o %t3 %t.bc
 ; Copy the minimized bitcode to the regular bitcode path so the module
 ; paths in the index are the same (save and restore the regular bitcode
 ; for use again further down).
 ; RUN: mv %t.bc %t.bc.sv
 ; RUN: cp %t.thinlink.bc %t.bc
 ; RUN: llvm-lto -thinlto -o %t4 %t.bc
 ; RUN: mv %t.bc.sv %t.bc
 ; RUN: diff %t3.thinlto.bc %t4.thinlto.bc
 
 ; Try again using -thinlto-action to produce combined index
 ; RUN: rm -f %t3.thinlto.bc %t4.thinlto.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t3.thinlto.bc %t.bc
 ; Copy the minimized bitcode to the regular bitcode path so the module
 ; paths in the index are the same.
 ; RUN: cp %t.thinlink.bc %t.bc
 ; RUN: llvm-lto -thinlto-action=thinlink -o %t4.thinlto.bc %t.bc
 ; RUN: diff %t3.thinlto.bc %t4.thinlto.bc
 
+; Make sure llvm-lto2 can read the thin link bitcode file
+; RUN: llvm-lto2 run %t.thinlink.bc -o %t.o -save-temps \
+; RUN:     -thinlto-distributed-indexes \
+; RUN:     -r=%t.thinlink.bc,f,px \
+; RUN:     -r=%t.thinlink.bc,g,px
+
 ; BCA: <GLOBALVAL_SUMMARY_BLOCK
 
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
 ; CHECK: @g = global i8 42
 @g = global i8 42
 
 ; CHECK: define void @f()
 define void @f() {
   ret void
 }
 
 ; CHECK: !llvm.dbg.cu
-; NODEBUG-NOT: !llvm.dbg.cu
+; NODEBUG-NOT: IDENTIFICATION_BLOCK_ID
+; NODEBUG-NOT: BLOCKINFO_BLOCK
+; NODEBUG-NOT: TYPE_BLOCK_ID
+; NODEBUG-NOT: VSTOFFSET
+; NODEBUG-NOT: CONSTANTS_BLOCK
+; NODEBUG-NOT: METADATA_KIND_BLOCK
+; NODEBUG-NOT: METADATA_BLOCK
+; NODEBUG-NOT: OPERAND_BUNDLE_TAGS_BLOCK
+; NODEBUG-NOT: UnknownBlock26
+; NODEBUG-NOT: FUNCTION_BLOCK
+; NODEBUG-NOT: VALUE_SYMTAB
+; NODEBUG: MODULE_BLOCK
+; NODEBUG: VERSION
+; NODEBUG: SOURCE_FILENAME
+; NODEBUG: <GLOBALVAR op0=0
+; NODEBUG: <FUNCTION op0=1
+; NODEBUG: GLOBALVAL_SUMMARY_BLOCK
+; NODEBUG: HASH
+; NODEBUG: SYMTAB_BLOCK
+; NODEBUG: STRTAB_BLOCK
 !llvm.dbg.cu = !{}
 
 !1 = !{i32 2, !"Debug Info Version", i32 3}
 !llvm.module.flags = !{!1}