[PATCH] D35334: ThinLTO Minimized Bitcode File Size Reduction

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

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



================
Comment at: include/llvm/Bitcode/BitcodeWriter.h:87
 
+    /// Write the specified module summary to the buffer specified at construction
+    /// time.
----------------
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.


================
Comment at: include/llvm/Bitcode/BitcodeWriter.h:90
+    ///
+    /// ModHash is for use in ThinLTO incremental build, generating while the 
+    /// IR bitcode file writing.
----------------
nit: s/generating/generated/


================
Comment at: include/llvm/Bitcode/BitcodeWriter.h:131
+  ///
+  /// ModHash is for use in ThinLTO incremental build, generating
+  /// while the IR bitcode file writing.
----------------
ditto


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:4031
+// 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 using
+// as padding.
----------------
nit: s/using/used/


================
Comment at: lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:425
   // 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);
----------------
nit: braces can be removed now.


https://reviews.llvm.org/D35334