[PATCH] D48967: [Dominators] Convert existing passes and utils to use the DomTreeUpdater class

Jakub Kuderski via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 9 09:01:54 PDT 2018


kuhar added a comment.

A general high-level question/suggestion: would it be possible to split this patch such that we don't change all of the uses of DDT and plain updates at once? I'm afraid this can subtly break in many places, and the patch will have to be reverted a couple of times before you get them right. If you do it with more granularity, then it should be easier to land each part and test it in isolation.
Is it possible, or do you have a dependency on the utility functions from Local.cpp and other helper files?



================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:364
   SmallPtrSet<BasicBlock *, 16> Unreachable;
-  DominatorTree &DT = DDT->flush();
+  DominatorTree &DT = DTU->getDomTree();
   for (auto &BB : F)
----------------
Can DTU have no DomTree here? Maybe we should assert for that?


================
Comment at: lib/Transforms/Scalar/JumpThreading.cpp:409
+          TryToSimplifyUncondBranchFromEmptyBlock(&BB, DTU)) {
+        // BB is valid for cleanup here because we passed in DTU. F remains
+        // BB's parent until a DTU->getDomTree() event.
----------------
nit: I think formatting is a bit off here. Can you run clang-format on this snippet?


https://reviews.llvm.org/D48967





More information about the llvm-commits mailing list