[PATCH] D35334: ThinLTO Minimized Bitcode File Size Reduction

[Peter Collingbourne via Phabricator via llvm-commits]

pcc accepted this revision.
pcc added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:198
+  /// If non-null, when GenerateHash is true, the resulting hash is written
+  /// into ModHash. If null, when GenerateHash is true, ModHash will be
+  /// generated for this module.
----------------
I'd remove the second sentence.


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:4006
+  /// ModHash is for use in ThinLTO incremental build, generated while the
+  /// IR bitcode file writing.
+  ModuleHash *ModHash;
----------------
I'd reword as "generated while writing the module bitcode file".


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:4007
+  /// IR bitcode file writing.
+  ModuleHash *ModHash;
+
----------------
This could be `const ModuleHash &ModHash;`.


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:4055
+  for (const GlobalVariable &GV : M.globals()) {
+
+    // GLOBALVAR: [strtab offset, strtab size, 0, 0, 0, linkage]
----------------
Remove blank line.


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:4120
+
+  // Write per-module global value summary.
+  writePerModuleGlobalValueSummary();
----------------
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.


https://reviews.llvm.org/D35334