[PATCH] D37870: Refactor collectChildrenInLoop to LoopUtils [NFC]

Sanjoy Das via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Sep 14 14:49:36 PDT 2017


sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

LGTM

I added a couple of stylistic issues that were present in the earlier function.  I'll leave it up to you if you want to fix these.



================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:1124
+  SmallVector<DomTreeNode *, 16> Worklist;
+  auto add_region_to_worklist = [&](DomTreeNode *DTN) {
+    // Only include subregions in the top level loop.
----------------
LLVM style is `AddRegionToWorklist`.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:1133
+
+  for (size_t I = 0; I < Worklist.size(); I++) {
+    DomTreeNode *DTN = Worklist[I];
----------------
For integer loop counters, I've usually seem LLVM use `i` instead of `I`.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:1135
+    DomTreeNode *DTN = Worklist[I];
+    for (DomTreeNode *Child : DTN->getChildren())
+      add_region_to_worklist(Child);
----------------
This can probably be `for (DomTreeNode *Child : Worklist[i])`, at which point the `{` braces won't be needed either.


https://reviews.llvm.org/D37870





More information about the llvm-commits mailing list