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

[Nadav Rotem via llvm-commits]

nadav added inline comments.

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:45
@@ -39,1 +44,3 @@
 namespace {
+/// Information about Instruction's
+struct InstInfoType {
----------------
"Instruction's" -> "Instructions. "

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:48
@@ +47,3 @@
+
+  bool Live = false; ///< True if this  instruction is live.
+
----------------
Doxygen comment above the line. 

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:54
@@ +53,3 @@
+
+/// Information about basic blocks relevant to dead code elimination
+struct BlockInfoType {
----------------
Period at the end of the sentence. 

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:62
@@ +61,3 @@
+  /// Quick access to the LiveInfo for the terminator.
+  InstInfoType *TerminatorLiveInfo = nullptr; // == & InstInfo[Terminator]
+
----------------
What's "== & InstInfo[Terminator]" ?  Can you write in english that this pointer points to "InstInfo[Terminator]".

Also, it could be a good idea to say something about the invalidation of InstInfo. 

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:69
@@ +68,3 @@
+
+  TerminatorInst *Terminator = nullptr; ///< Cache of BB->getTerminator().
+};
----------------
Comment above the line. 

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:76
@@ +75,3 @@
+  /// Per basic block information.
+  std::vector<BlockInfoType> BlockInfoVec;
+
----------------
Are you using std::vector instead of SmallVector because you are expecting that most function will have a large number of basic blocks? 

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:145
@@ +144,3 @@
+  auto NumBlocks = F.size();
+  BlockInfoVec.resize(NumBlocks);
+  BlockInfo.grow(2 * NumBlocks);
----------------
Can you please add a comment that explains why  BlockInfo is twice as big as BlockInfoVec?

================
Comment at: lib/Transforms/Scalar/ADCE.cpp:159
@@ +158,3 @@
+
+  // initialize instruction map and set pointers to block info
+  InstInfo.grow(2 * NumInsts);
----------------
initialize -> Initialize. 
Period. 


https://reviews.llvm.org/D23225