[PATCH] D35334: ThinLTO Minimized Bitcode File Size Reduction

Haojie Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 18 17:44:18 PDT 2017


haojiewang added inline comments.


================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:124
   /// be used in the backend.
   ModuleHash *ModHash;
 
----------------
haojiewang wrote:
> haojiewang wrote:
> > pcc wrote:
> > > 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.
> > I just tried to change the split writing, but there were bunch of errors in test after I did that. There will be 11 test files should be updated if we change this, and most of them are because llvm-dis cannot read a thin link bitcode file. Should we do it in this patch or leave it the next patch? How should we update them, just changing llvm-dis to llvm-bcanalyzer or also adding the CHECKs and CHECK-NOTs as in the test file of this patch?
> I'm just not sure if I would affect other people's code if I do so much changes on the test files.
Sorry, I just made a terrible mistake on this one. Actually there would be only 1 test file should be updated, which is for the llvm-dis problem. Besides, for the "NODEBUG" checks, I can just remove them because they are not needed after this patch, right? And after I add the CHECKs and CHECK-NOTs in distributed-import.ll, I don't have to do the check again for the split.ll test file, right?


https://reviews.llvm.org/D35334





More information about the llvm-commits mailing list