[PATCH] D35334: ThinLTO Minimized Bitcode File Size Reduction

[Peter Collingbourne via Phabricator via llvm-commits]

pcc added inline comments.


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:114
   /// True if a module hash record should be written.
   bool GenerateHash;
 
----------------
haojiewang wrote:
> pcc wrote:
> > I think we can move this into the derived class now, together with Hasher and addToStrtab.
> Oh, sure. But I think this change will require a new addToStrtab for ThinLinkBitcodeWriter, which only add the str to StrtabBuilder without update GenerateHash.
You can just change the callers to call `StrtabBuilder.add()`, that's what we did before we needed to add `addToStrtab` to fix the caching problem in ModuleBitcodeWriter.


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:124
   /// be used in the backend.
   ModuleHash *ModHash;
 
----------------
haojiewang wrote:
> pcc wrote:
> > I think this change removes the need to support writing a module with a precomputed hash from ModuleBitcodeWriter. Can you please remove that functionality?
> Writing a module with a precomputed hash is still needed while split writing a module(splitAndWriteThinLTOBitcode), because I didn't touch the split writing in this patch. Maybe I should also change the writer in split writing? 
You mean here?
http://llvm-cs.pcc.me.uk/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp#386

Yes, I think you can change that to call writeThinLinkBitcode.


https://reviews.llvm.org/D35334