[PATCH] D35334: ThinLTO Minimized Bitcode File Size Reduction
Haojie Wang via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 12 17:40:17 PDT 2017
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
More information about the llvm-commits
mailing list