[PATCH] D84514: [BPI][NFC] Consolidate code to deal with SCCs under a dedicated data structure.

[Evgeniy via Phabricator via llvm-commits]

ebrevnov marked 3 inline comments as done.
ebrevnov added inline comments.


================
Comment at: llvm/include/llvm/Analysis/BranchProbabilityInfo.h:171
+    // SccBlockType.
+    using SccBlockTypeMap = DenseMap<const BasicBlock *, uint32_t>;
+    // Vector containing classification of basic blocks for all  SCCs where i'th
----------------
davidxl wrote:
> Prefer the original name here. Also why changing bool to uint32_t?
Old code accounted for header blocks only by keeping a map between basic block to a boolean value. New code associates basic blocks with one or several values from SccBlockType. That's why 'bool' was replaced with 'uint32_t'.

In other words we are able to classify basic blocks as either Inner or Header and/or Exiting. That's why keeping "Header" in the type name has now sense now.


================
Comment at: llvm/include/llvm/Analysis/BranchProbabilityInfo.h:174
+    // vector element corresponds to SCC with ID equal to i.
+    using SccBlockTypeMaps = std::vector<SccBlockTypeMap>;
+
----------------
davidxl wrote:
> keep original name
"Header" has no sense now. It should be changed to something. Please suggest alternatives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84514