[PATCH] Rename determineInsertionPoints to determinePHINodes, expose it so it can be reused

Quentin Colombet qcolombet at apple.com
Tue Apr 21 10:07:41 PDT 2015

Hi Daniel,

Thanks for the refactoring!

LGTM with a couple of nitpicks.


Comment at: include/llvm/Analysis/IteratedDominanceFrontier.h:45
@@ +44,3 @@
+  void setDefiningBlocks(const SmallPtrSetImpl<BasicBlock *> &Blocks) {
+    DefBlocks = &Blocks;
Add a comment that says that Blocks must be valid for the whole lifetime of the IDFCalculator (or the next call to setDefiningBlocks).

Comment at: include/llvm/Analysis/IteratedDominanceFrontier.h:49
@@ +48,3 @@
+  void setLiveInBlocks(const SmallPtrSetImpl<BasicBlock *> &Blocks) {
+    LiveInBlocks = &Blocks;

Comment at: include/llvm/Analysis/IteratedDominanceFrontier.h:58
@@ +57,3 @@
+  void calculate(SmallVectorImpl<BasicBlock *>&IDFBlocks);
Add a comment on what will IDFBlocks contain.

Edit: I see that the comment is in the cpp file. I believe it would be more convenient to have it here. Also, does doxygen pick up the comments from the .cpp?



More information about the llvm-commits mailing list