[PATCH] D35334: ThinLTO Minimized Bitcode File Size Reduction

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini added inline comments.


================
Comment at: include/llvm/Bitcode/BitcodeWriter.h:89
+    ///
+    /// Index is required non-null.
+    /// 
----------------
Use a const reference then?


================
Comment at: include/llvm/Bitcode/BitcodeWriter.h:91
+    /// 
+    /// ModHash is required non-null, for use in ThinLTO incremental build, generating
+    /// while the IR bitcode file writing.
----------------
Same: if non-null required, why not using a reference?


================
Comment at: include/llvm/Bitcode/BitcodeWriter.h:132
+  ///
+  /// Index is required non-null.
+  /// 
----------------
Same?


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:4007
+
+/// Class to manage the bitcode writing for a module summary.
+class SummaryBitcodeWriter : public ModuleBitcodeWriterBase {
----------------
haojiewang wrote:
> mehdi_amini wrote:
> > Is a "module summary" described anywhere?
> I got that name from ModuleSummaryIndex, but it's not a good name. Already changed it to minimized bitcode file.
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.


https://reviews.llvm.org/D35334