[PATCH] D37528: [JumpThreading] Preserve dominance across the pass.

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a comment.

This patch uses the "!find-push_back" pattern a lot:

  DominatorTree::UpdateType UT = {DominatorTree::Delete, *PI, BB};
  if (std::find(Updates.begin(), Updates.end(), UT) == Updates.end()) {
    Updates.push_back(UT);

which made me think that maybe it would be better to offer some better interface for applying updates that would automatically get rid of the duplicate ones? Or maybe it would make sense to just have a second overload that accepts set-like containers.



================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:1951
 
+  std::vector<DominatorTree::UpdateType> Updates = {
+      {DominatorTree::Insert, NewBB, SuccBB},
----------------
If there are exactly 3 updates every time, maybe it'd make sense to use `SmallVector<DT::UT, 3>` or just an array here?


================
Comment at: llvm/lib/Transforms/Scalar/JumpThreading.cpp:2518
+  // of the above calls.
+  std::vector<DominatorTree::UpdateType> Updates = {
+      // Guarded block split.
----------------
(Same us above)
Maybe worth using SmallVector/array?


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:183
+              deleteEdge = false;
+              break;
+            }
----------------
Why not some standard algorithm? I.E. `std::find` or `std::none_of`


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:1826
+
+  if (DT && !Updates.empty())
+    DT->applyUpdates(Updates);
----------------
`applyUpdates` should work with empty update set, but I guess it makes no real difference here.


Repository:
  rL LLVM

https://reviews.llvm.org/D37528