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

[David Callahan via llvm-commits]

david2050 marked 6 inline comments as done.
david2050 added a comment.

Thanks!


================
Comment at: lib/Transforms/Scalar/ADCE.cpp:70
@@ +69,3 @@
+  /// Cache of BB->getTerminator()
+  TerminatorInst *Terminator = nullptr;
+};
----------------
We will had a little more state to support but it really will have no private data or sophisticated function.

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:77
@@ +76,3 @@
+  /// Per basic block information.
+  SmallVector<BlockInfoType, 32> BlockInfoVec;
+
----------------
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.

================
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
----------------
Is this comment about the resize above or the grow below?

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:263
@@ -119,1 +262,3 @@
+    // TODO -- handle PhiNodes
+  } while (!Worklist.empty());
 
----------------
In future, checkControlDependences will mark some branches as live as it determines they are needed. We then restart the data flow loop to make the data dependences of those branches as live.


https://reviews.llvm.org/D23225