[PATCH] D23225: [ADCE] Modify data structures to support removing control flow

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 5 15:17:04 PDT 2016


mehdi_amini added inline comments.

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:77
@@ +76,3 @@
+  /// Per basic block information.
+  SmallVector<BlockInfoType, 32> BlockInfoVec;
+
----------------
david2050 wrote:
> Mehdi,  yes, there are places where we iterate over the blocks in order so the DenseMap itself is not suitable. The order of the vector never changes.  We could iterative of the basic blocks and index into the map if you feel strongly that it should be eliminated.
For the three current loop over `BlockInfoVec` the iteration order does not seem to matter you could iterate over the DenseMap, the question is more about future patches that may benefit from this vector.

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:147
@@ +146,3 @@
+  BlockInfoVec.resize(NumBlocks);
+
+  // We will have an entry in the map for each block so we grow the
----------------
david2050 wrote:
> Is this comment about the resize above or the grow below?
It is about `grow` below.

(Note there is an arrow on the top left of the comment next to my name with every comment that was made on a previous revision that can bring the context where the comment was made)


https://reviews.llvm.org/D23225





More information about the llvm-commits mailing list