[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.

Cheers,
-Quentin


================
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;
----------------
Ditto.

================
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?

http://reviews.llvm.org/D9118

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list