[llvm] [NFCI][BitcodeReader]Read real GUID from VI as opposed to storing it in map (PR #107735)
Mingming Liu via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 8 00:10:44 PDT 2024
minglotus-6 wrote:
> * We always create an instance of `ValueInfo` with the same GUID as `std::get<2>`.
Yup this is the case for all ([first](https://github.com/llvm/llvm-project/blob/caebb4562ce634a22f7b13480b19cffc2a6a6730/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L7130-L7133), [second](https://github.com/llvm/llvm-project/blob/caebb4562ce634a22f7b13480b19cffc2a6a6730/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L7221-L7222), [third](https://github.com/llvm/llvm-project/blob/caebb4562ce634a22f7b13480b19cffc2a6a6730/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L7622-L7623)) insertions.
> Once an instance of ValueInfo is created, its pointer portion never changes.
> The pointer portion of ValueInfo points to an key-value pair of GlobalValueSummaryMapTy, so the key portion, GUID, shouldn't change.
This is the case as well.
Basically, `ValueIdToValueInfoMap` is owned and used by the per module-summary bitcode reader, to associate ([serialized](https://github.com/llvm/llvm-project/blob/cf11eb62e1d0fa41f68b4ca3150eac854ac2e991/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp#L4364), [per-module](https://github.com/llvm/llvm-project/blob/cf11eb62e1d0fa41f68b4ca3150eac854ac2e991/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp#L259)) 'value-id' with the (in-memory) value summary (from combined summary).
So the ValueInfo's pointer portion essentially points to the [global-value summary entry](https://github.com/llvm/llvm-project/blob/cf11eb62e1d0fa41f68b4ca3150eac854ac2e991/llvm/include/llvm/IR/ModuleSummaryIndex.h#L176), which is owned by [CombinedIndex](https://github.com/llvm/llvm-project/blob/cf11eb62e1d0fa41f68b4ca3150eac854ac2e991/llvm/include/llvm/LTO/LTO.h#L341).
Since bitcode readers are [(reasonably) destructed](https://github.com/llvm/llvm-project/blob/bc59b638ae35594b55a439a5986519a3f9cc903d/llvm/lib/Bitcode/Reader/BitcodeReader.cpp#L8411-L8413) after the per-module summary content are merged into combined index, this change doesn't help reduce peak memory usage. This change is not complex anyway, so I just thought about sending it out.
https://github.com/llvm/llvm-project/pull/107735
More information about the llvm-commits
mailing list