[PATCH] D43176: [LoopInterchange] Incrementally update the dominator tree.

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:407
+    DT->applyUpdates(DTUpdates);
+    DTUpdates.clear();
+  }
----------------
Is this necessary? I guess you want to make the vector look clean in case a crash happens, is that it?


================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:588
       Changed |= Interchanged;
     }
     return Changed;
----------------
Is there a reason why the updates are applied int the destructor instead of at the end of this loop?


================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:1279
+                            std::vector<DominatorTree::UpdateType> &DTUpdates) {
+  unsigned NumSucc = BI->getNumSuccessors();
+  for (unsigned i = 0; i < NumSucc; ++i) {
----------------
`const unsigned NumSucc`? 


================
Comment at: lib/Transforms/Scalar/LoopInterchange.cpp:1285
+      DTUpdates.push_back(
+          {DominatorTree::UpdateKind::Insert, BI->getParent(), NewBB});
+      DTUpdates.push_back(
----------------
Can you have multiple successors that are the same BB? If so, you would have to exclude the redundant ones here.


https://reviews.llvm.org/D43176