[PATCH] D80403: [ThinLTO] Compute the basic block count across modules.

[Hiroshi Yamauchi via Phabricator via llvm-commits]

yamauchi added inline comments.


================
Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:1010
 
+  // The total number of basic blocks in the module.
+  uint64_t BlockCount;
----------------
tejohnson wrote:
> Also specify that this would be the total number of bbs in the LTO unit in the combined index (it is the total in the module in the per-module summary).
Done.


================
Comment at: llvm/include/llvm/IR/ModuleSummaryIndex.h:1033
   // and BitcodeWriter.cpp.
-  static constexpr uint64_t BitcodeSummaryVersion = 8;
+  static constexpr uint64_t BitcodeSummaryVersion = 9;
 
----------------
tejohnson wrote:
> I'm not convinced a version bump is required. Isn't the lack of a block count record handled naturally ( i.e. the count stays at 0 in the index)?
Yes, apparently bumping wasn't necessary. Done.


================
Comment at: llvm/lib/Bitcode/Reader/BitcodeReader.cpp:6244
+    case bitc::FS_BLOCK_COUNT:
+      // TheIndex.setBlockCount(Record[0]);
+      TheIndex.addBlockCount(Record[0]);
----------------
tejohnson wrote:
> Remove commented out line. Presumably this uses addBlockCount instead of setBlockCount so that it gets merged properly when building the combined index?
Yes. Done.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80403/new/

https://reviews.llvm.org/D80403