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

[David Li via Phabricator via llvm-commits]

davidxl 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
----------------
Prefer the original name here. Also why changing bool to uint32_t?


================
Comment at: llvm/include/llvm/Analysis/BranchProbabilityInfo.h:174
+    // vector element corresponds to SCC with ID equal to i.
+    using SccBlockTypeMaps = std::vector<SccBlockTypeMap>;
+
----------------
keep original name


================
Comment at: llvm/include/llvm/Analysis/BranchProbabilityInfo.h:176
+
+    SccMap BlockToSccId;
+    SccBlockTypeMaps SccBlocks;
----------------
Prefer original name SccNums (at least for this patch). 


================
Comment at: llvm/include/llvm/Analysis/BranchProbabilityInfo.h:177
+    SccMap BlockToSccId;
+    SccBlockTypeMaps SccBlocks;
+
----------------
original name


================
Comment at: llvm/include/llvm/Analysis/BranchProbabilityInfo.h:182
+
+    int getSCCNum(const BasicBlock *BB) const;
+    bool isSCCHeader(const BasicBlock *BB, int SccNum) const {
----------------
Document the interfaces -- what return is expected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84514