[PATCH] D35334: ThinLTO Minimized Bitcode File Size Reduction

[Haojie Wang via Phabricator via llvm-commits]

haojiewang marked 3 inline comments as done.
haojiewang added inline comments.


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:100
 
-/// 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;
-
+/// Abstrac class to manage the module bitcode writing, currently subclassed for
+/// ModuleBitcodeWriter and SummaryBitcodeWriter
----------------
mehdi_amini wrote:
> s/Abstrac/Abstract/
> 
> But in fact I don't think it class is abstract, is it? 
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?


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:4007
+
+/// Class to manage the bitcode writing for a module summary.
+class SummaryBitcodeWriter : public ModuleBitcodeWriterBase {
----------------
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.


================
Comment at: lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp:426
   if (ThinLinkOS) {
     StripDebugInfo(M);
+    WriteSummaryToFile(&M, *ThinLinkOS, Index, &ModHash);
----------------
mehdi_amini wrote:
> This should be obsolete right now?
Yes, I think so. Already removed it.


https://reviews.llvm.org/D35334