[PATCH] D17473: [LoopUnroll] Avoid unnecessary DT recomputation.

[Chandler Carruth via llvm-commits]

chandlerc added a comment.

Only high-level question is whether all of these cases have to be handled when the loop is in simplified form.


================
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:115-117
@@ +114,5 @@
+      SmallVector<DomTreeNode *, 8> Children(DTN->begin(), DTN->end());
+      for (SmallVectorImpl<DomTreeNode *>::iterator DI = Children.begin(),
+                                                    DE = Children.end();
+           DI != DE; ++DI)
+        DT->changeImmediateDominator(*DI, PredDTN);
----------------
Range based for-loop?

================
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:533-534
@@ +532,4 @@
+      BasicBlock *ExitIDom = nullptr;
+      for (auto BI = pred_begin(UniqueExit), BE = pred_end(UniqueExit);
+           BI != BE; BI++) {
+        ExitIDom =
----------------
There is a range based function as well for this.

================
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:536
@@ +535,3 @@
+        ExitIDom =
+            ExitIDom ? DT->findNearestCommonDominator(ExitIDom, *BI) : *BI;
+      }
----------------
I'm surprised this much work is necessary even when the loop is in simplified form?

================
Comment at: lib/Transforms/Utils/LoopUnroll.cpp:563-566
@@ -525,5 +562,6 @@
 
-  // FIXME: Reconstruct dom info, because it is not preserved properly.
-  // Incrementally updating domtree after loop unrolling would be easy.
-  if (DT)
-    DT->recalculate(*L->getHeader()->getParent());
+  if (DT) {
+    // FIXME: Reconstruct dom info, because it is not preserved properly.
+    // Incrementally updating domtree after loop unrolling would be easy.
+    if (!CompletelyUnroll)
+      DT->recalculate(*L->getHeader()->getParent());
----------------
Merge the two ifs? And update the comment?


http://reviews.llvm.org/D17473