[DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

[asbirlea]

Summary

This replaces the logic inChildrenGetter inside the DominatorTree with
GraphTraits over a GraphDiff object, an object which encapsulated the
view of the previous CFG.

The simplification step removing the extensions in clang which use DominatorTree and also filters for nullptr are deferred to a subsequent cleanup patch.

[asbirlea]

I sent this out to get some general feedback, but I'd like to be able to simplify the fact that this is replacing a for over ChildrenGetter with a for over children<type>(pair), plus 5 lines to set up the type and pair; one simplification is around getting the type inside children (GraphDiffBBPair) and another around having the BUI or not; for the latter I'm considering updating a few places such that BUI is never nullptr.

[dblaikie]

Couple of small nits, but I'll leave most of the review to someoen else here - I /think/ it's beyond my context/experience (but if necessary, poke me and I can look more/ask more questions/etc)

llvm/include/llvm/Support/GenericDomTreeConstruction.h

324–325

No need for make_pair if you're going to specify the types on the LHS, could write this as:

std::pair<...> GDNodePair(GDTmp, N);

(similarly two instances further down in this patch - make_pair or explicit type, not both)

Or you could use auto on the LHS given the types of the two parameters seem close enough that it's not too surprising what make_pair produces, maybe even just roll it all in together:

return !empty(children<GraphDiffBBPair>(std::make_pair(GDTmp, N)));

326–330

Probably write this as (if I'm understanding the code/intent correctly):

return !empty(children<GraphDiffBBPair>(GDNodePair));

[kuhar]

llvm/include/llvm/IR/CFGDiff.h

199

You can use two helper functions taking integral_constant<bool, true/false> tags -- should be less verbose

llvm/include/llvm/Support/GenericDomTreeConstruction.h

89

nit: Why pointer instead of keeping a reference?

177

nit: use using instead of typefef.

178

nit: can we use std::conditional_t<...> in c++14 to get rid of typename and ::type? Or does it make some buildbots unhappy?

179

nit: I don't find this name very informative. Maybe something like DirectedBB?

183

std::pair<const GraphDiffT *, NodePtr> GDNodePair(GDTmp, BB)

185

Won't children infer the template parameter based on the passes value? I don't get why the template argument type is a pair where the second argument is a directed nodeptr, but the runtime value is always a plain nodeptr. Couldn't the second pair type also be directed for GDNodePair?

186

Could this new code be a hoisted into a helper function?

1146

Could you add a comment next to the nullptr with the argument name?

1543

Does this care about the direction (pre- or post-) at all, or does it need some CFG view? Why is pre-view incorrect?

[kuhar]

llvm/include/llvm/Support/GenericDomTreeConstruction.h

186

Or alternatively, could the old ChildrenGetter be implemented with these 5 magic lines?

[asbirlea]

Address comments.

llvm/include/llvm/Support/GenericDomTreeConstruction.h

185

The directed nodeptr is the equivalent of a bool inside GraphDiff translating to what kind of children do you want; the second pair argument bears no info of that kind, only the actual NodePtr for which we desire the children.

186

I think it looks much cleaner after the latest update, let me know what you think.

1543

The purpose of this method was to update the DT assuming an additional list of updates (the argument given). In practice, the BUI argument is ignored in the CalculateFromScratch call below. Hence we need the additional changes to support this kind of updates inside CalculateFromScratch and the other methods.

[kuhar]

In D77341#1960854, @asbirlea wrote:

Address comments.

Thanks for the changes and explanations. It think with a few more tweaks this will be a good refactoring step towards phasing out BUI.

llvm/include/llvm/IR/CFGDiff.h

198

What benefit does an anonymous namespace in a header have over a named one, e.g., detail/impl? Doesn't it make it more difficult to deduplicate symbols across different translation units? Not sure what the best practices are in C++ now in this area.

llvm/include/llvm/Support/GenericDomTreeConstruction.h

323

This pattern repeats a few times. Could it be pushed into a helper function to get something like this?

return !empty(children<GraphDiffNodePair>(GetGraphDiffNodePair(BUI, N)));

[kuhar]

llvm/include/llvm/IR/CFGDiff.h

198

Found an article on this matter: https://wiki.sei.cmu.edu/confluence/display/cplusplus/DCL59-CPP.+Do+not+define+an+unnamed+namespace+in+a+header+file

[dblaikie]

llvm/include/llvm/IR/CFGDiff.h

198

Oh, yeah, sorry I didn't spot that - yeah, anonymous namespaces in header files are a no-go. This should be a detail (detail outnumbers impl 361 to 41 in the LLVM project) namespace within the llvm namespace as suggested.

[asbirlea]

llvm/include/llvm/Support/GenericDomTreeConstruction.h

323

My dilemma here is how to not allocate a GraphDiffT instance. There are 4 cases where the pattern is inside a static method and once in a class method. I used a stack var for the 4 cases and a unique_ptr for the class method.

Sure, I can do:

static auto getGDNPair(BUI, EmptyGD, N) {
return std::make_pair<>(BUI ? BUI->PreViewCFG : EmptyGD, N);
}
{
...
GraphDiffT EmptyGD;
return !empty(children<GraphDiffNodePair>(getGDNPair(BUI, &EmptyGD, N)));
}

But it felt like moving one line at the callsite to a oneline helper is not making things much better.

I was hoping for something more along the lines of:

return !empty(children<GraphDiffNodePair>({getGD(BUI), N});

Or, ensure BUI is always non-null, and simplify to:

return !empty(children<GraphDiffNodePair>({BUI->PreViewCFG, N});

Thoughts?

[asbirlea]

Name anonymous namespace.

[kuhar]

Looks good to me overall. I don't want to block it over the cosmetic issues like allocating the empty GD object.

llvm/include/llvm/Support/GenericDomTreeConstruction.h

323

Does allocating a GD have a big cost?

I think you could get away with returning a temporary GD object that would die as soon as the expression evaluation ends -- should be no different than placing it on the stack just before the function call.

Overall, I still don't understand why you try to avoid creating a wrapper function / struct that returns children, and call childredn<...>(...) directly. Either way, I being able to write:

return !empty(children<GraphDiffNodePair>({getGD(BUI), N});

or

return !empty(getChildren<GraphDiffNodePair>(BUI, N));

would definitely be concise and readable enough IMO.

[asbirlea]

Thank you for all the comments.

llvm/include/llvm/Support/GenericDomTreeConstruction.h

323

Does allocating a GD have a big cost?

AFAIU, dynamically allocating the object does, vs having a stack variable.

I'll do a follow-up cleanup attempt.

[stella.stamenova]

It looks like this broke the windows lldb buildbot:

lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/15542

It already had a test failure, so you probably didn’t get the email.

[bondhugula]

@asbirlea This has broken the MLIR build. Could you please build with -DLLVM_ENABLE_PROJECTS=mlir?

FAILED: tools/mlir/lib/Analysis/CMakeFiles/MLIRAnalysis.dir/Dominance.cpp.o 
/usr/lib64/ccache/clang++  -DGTEST_HAS_RTTI=0 -DMLIR_CUDA_CONVERSIONS_ENABLED=1 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/mlir/lib/Analysis -I/home/uday/llvm-project/mlir/lib/Analysis -I/usr/include/libxml2 -Iinclude -I/home/uday/llvm-project/llvm/include -I/home/uday/llvm-project/mlir/include -Itools/mlir/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -ffunction-sections -fdata-sections -O2     -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT tools/mlir/lib/Analysis/CMakeFiles/MLIRAnalysis.dir/Dominance.cpp.o -MF tools/mlir/lib/Analysis/CMakeFiles/MLIRAnalysis.dir/Dominance.cpp.o.d -o tools/mlir/lib/Analysis/CMakeFiles/MLIRAnalysis.dir/Dominance.cpp.o -c /home/uday/llvm-project/mlir/lib/Analysis/Dominance.cpp
In file included from /home/uday/llvm-project/mlir/lib/Analysis/Dominance.cpp:14:
In file included from /home/uday/llvm-project/mlir/include/mlir/Analysis/Dominance.h:12:
In file included from /home/uday/llvm-project/mlir/include/mlir/IR/RegionGraphTraits.h:18:
In file included from /home/uday/llvm-project/mlir/include/mlir/IR/Region.h:16:
In file included from /home/uday/llvm-project/mlir/include/mlir/IR/Block.h:16:
In file included from /home/uday/llvm-project/mlir/include/mlir/IR/BlockSupport.h:16:
In file included from /home/uday/llvm-project/mlir/include/mlir/IR/Value.h:16:
In file included from /home/uday/llvm-project/mlir/include/mlir/IR/Types.h:12:
In file included from /home/uday/llvm-project/mlir/include/mlir/IR/TypeSupport.h:17:
In file included from /home/uday/llvm-project/mlir/include/mlir/IR/StorageUniquerSupport.h:17:
In file included from /home/uday/llvm-project/mlir/include/mlir/Support/STLExtras.h:18:
In file included from /home/uday/llvm-project/llvm/include/llvm/ADT/STLExtras.h:20:
/home/uday/llvm-project/llvm/include/llvm/ADT/iterator.h:366:17: error: indirection requires pointer operand ('const mlir::PredecessorIterator' invalid)
    NR.second = *this->I;

[mehdi_amini]

llvm/include/llvm/Support/GenericDomTree.h

32

This looks like a layering violation to me here: I wouldn't expect a generic utility in Support to introduce a dependency on IR.

I speculatively reverted in https://github.com/llvm/llvm-project/commit/57d2d48399b63c0ef1dda490fdaf28efbb80c2c0 while we can figure it out.

[mehdi_amini]

In D77341#1973827, @bondhugula wrote:

@asbirlea This has broken the MLIR build. Could you please build with -DLLVM_ENABLE_PROJECTS=mlir?

If you use arcanist, you get pre-merge testing on Phabricator :)

[dblaikie]

llvm/include/llvm/Support/GenericDomTree.h

32

Yep - agreed on the layering violation. Thanks for the catch/revert!

I /think/ CFGDiff.h is independent of IR & shouldn't include IR/CFG.h and then can move into Support. Ah, yeah, when I did 30804d0a3fb23325c4b455108e59d00213b83891 & related work, that's what made it independent of IR.

Moved it over to Support in a838aadae3f20b9644e2ad553d3d558a91fd8fd7

[mehdi_amini]

llvm/include/llvm/Support/GenericDomTree.h

32

Thanks! Re-landed in 0445c64998d14b81f0d3a3182011fc5eae47fa71

[mehdi_amini]

llvm/include/llvm/Support/GenericDomTree.h

32

Actually reverted again as it breaks the MLIR build in a different way, I won't be able to look into this in more details right now.

(I ran ninja check but I forgot that check is not check-all...)

[nikic]

This change causes a 2.5% compile-time regression across the board, see http://llvm-compile-time-tracker.com/compare.php?from=37bcf2df01cfa47e4509a5d225a23e2ca95005e6&to=a90374988e4eb8c50d91e11f4e61cdbd5debb235&stat=instructions. Can you please try to find a cheaper way to approach this issue?

[asbirlea]

I'm working to update this revision to address the compile-time regression.
Re-opening so I can rebase and add misc fixes and will mark as changes planned until I address the compile-time issue.

[asbirlea]

Rebase.

[asbirlea]

Updated to include the part of the patch that's moving the Updates to a CFGDiff object. Splitting off from the clean-up work merging the two branches when BUI is null.
This patch does not exhibit the compile-time regression which caused it to be reverted previously.

[asbirlea]

Nit: re-add consts

[nikic]

Numbers for the new patch: https://llvm-compile-time-tracker.com/compare.php?from=c0308fd154f9a945608bd42630dc81dce5edfb40&to=e6e3534e77961cfa65d36828de5c75f36a25d009&stat=instructions

The regression is definitely smaller now, but still fairly large. E.g. > 2% on mafft.

[asbirlea]

Thank you for the testing. Could you help with with instructions on how to run the tracker myself?
My local testing showed a negligible regression for mafft and a negligible improvement on other benchmarks, so it looked like noise on average.

[nikic]

In D77341#2144974, @asbirlea wrote:

Thank you for the testing. Could you help with with instructions on how to run the tracker myself?
My local testing showed a negligible regression for mafft and a negligible improvement on other benchmarks, so it looked like noise on average.

The tracker just compiles test-suite under perf stat using the cached cmake configs. If I pick out the file with the largest regression (mafft constants.c in ReleaseThinLTO config with 18% regression) I can reproduce this locally as follows:

perf stat /home/nikic/llvm-project/build/bin/clang -DNDEBUG  -O3 -fomit-frame-pointer -flto=thin -DNDEBUG   -w -Werror=date-time -DLLVM -MD -MT MultiSource/Benchmarks/mafft/CMakeFiles/pairlocalalign.dir/constants.c.o -MF MultiSource/Benchmarks/mafft/CMakeFiles/pairlocalalign.dir/constants.c.o.d -o MultiSource/Benchmarks/mafft/CMakeFiles/pairlocalalign.dir/constants.c.o   -c ../MultiSource/Benchmarks/mafft/constants.c

This gives me 3.5M instructions before and 4.2M instructions after. Those particular numbers are for an assertion-enabled build (the numbers on the website are without assertions.)

[asbirlea]

The increase in number of instructions and cycles was caused by reversing the order in which the updates are applied by GraphDiff.
I'll look into re-adding some of the original cleanups in a follow-up patch.

[dblaikie]

In D77341#2171305, @asbirlea wrote:

The increase in number of instructions and cycles was caused by reversing the order in which the updates are applied by GraphDiff.
I'll look into re-adding some of the original cleanups in a follow-up patch.

Fascinating - and you were able to reproduce this by just reversing the order in the old code (using ChildrenGetter/building a vector and returning it - not using all the iterator adapters, etc) and that the performance problem was no longer present in the new code if you changed its order to match?

Huh. (again, sorry this has been such a slog :/)

[asbirlea]

Yes. The difference in perf in instructions and cycles count is due to reversing the update order.
I'm still not seeing meaningful changes in wall-time even with the update order reversed, but reliable wall-time testing is hard.

I do have changes that

• drop the GraphTraits usage entirely in favor of the getChildren() method in GraphDiff
• refactor the hashmaps in GraphDiff

but I plan to send those separately.

[nikic]

Numbers for the newest version: https://llvm-compile-time-tracker.com/compare.php?from=183342c0a9850e60dd7a004b651c83dfb3a7d25e&to=eabcf534fe8760d14c95b40f07c61450c819d643&stat=instructions

This is now a geomean 0.2% regressions, and I don't see any large outliers for individual files either (everything is below 2%). So this should be fine now, at least where compile-time impact is concerned :)

[asbirlea]

Great, thank you for confirming :-)

clang/include/clang/Analysis/Analyses/Dominators.h

 //- Dominators.h - Implementation of dominators tree for Clang CFG -*- C++ -*-//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
LLVM_DUMP_METHOD void dump() {
     }
   }
 };
 
 } // namespace clang
 
 namespace llvm {
 
-/// Clang's CFG contains nullpointers for unreachable succesors, e.g. when an
-/// if statement's condition is always false, it's 'then' branch is represented
-/// with a nullptr. This however will result in a nullpointer derefernece for
-/// dominator tree calculation.
-///
-/// To circumvent this, let's just crudely specialize the children getters
-/// used in LLVM's dominator tree builder.
-namespace DomTreeBuilder {
-
-using ClangCFGDomChildrenGetter =
-SemiNCAInfo<clang::CFGDomTree::DominatorTreeBase>::ChildrenGetter<
-                                                             /*Inverse=*/false>;
-
-template <>
-template <>
-inline ClangCFGDomChildrenGetter::ResultTy ClangCFGDomChildrenGetter::Get(
-    clang::CFGBlock *N, std::integral_constant<bool, /*Inverse=*/false>) {
-  auto RChildren = reverse(children<NodePtr>(N));
-  ResultTy Ret(RChildren.begin(), RChildren.end());
-  Ret.erase(std::remove(Ret.begin(), Ret.end(), nullptr), Ret.end());
-  return Ret;
-}
-
-using ClangCFGDomReverseChildrenGetter =
-SemiNCAInfo<clang::CFGDomTree::DominatorTreeBase>::ChildrenGetter<
-                                                              /*Inverse=*/true>;
-
-template <>
-template <>
-inline ClangCFGDomReverseChildrenGetter::ResultTy
-ClangCFGDomReverseChildrenGetter::Get(
-    clang::CFGBlock *N, std::integral_constant<bool, /*Inverse=*/true>) {
-  auto IChildren = inverse_children<NodePtr>(N);
-  ResultTy Ret(IChildren.begin(), IChildren.end());
-  Ret.erase(std::remove(Ret.begin(), Ret.end(), nullptr), Ret.end());
-  return Ret;
-}
-
-using ClangCFGPostDomChildrenGetter =
-SemiNCAInfo<clang::CFGPostDomTree::DominatorTreeBase>::ChildrenGetter<
-                                                             /*Inverse=*/false>;
-
-template <>
-template <>
-inline ClangCFGPostDomChildrenGetter::ResultTy
-ClangCFGPostDomChildrenGetter::Get(
-    clang::CFGBlock *N, std::integral_constant<bool, /*Inverse=*/false>) {
-  auto RChildren = reverse(children<NodePtr>(N));
-  ResultTy Ret(RChildren.begin(), RChildren.end());
-  Ret.erase(std::remove(Ret.begin(), Ret.end(), nullptr), Ret.end());
-  return Ret;
-}
-
-using ClangCFGPostDomReverseChildrenGetter =
-SemiNCAInfo<clang::CFGPostDomTree::DominatorTreeBase>::ChildrenGetter<
-                                                              /*Inverse=*/true>;
-
-template <>
-template <>
-inline ClangCFGPostDomReverseChildrenGetter::ResultTy
-ClangCFGPostDomReverseChildrenGetter::Get(
-    clang::CFGBlock *N, std::integral_constant<bool, /*Inverse=*/true>) {
-  auto IChildren = inverse_children<NodePtr>(N);
-  ResultTy Ret(IChildren.begin(), IChildren.end());
-  Ret.erase(std::remove(Ret.begin(), Ret.end(), nullptr), Ret.end());
-  return Ret;
-}
-
-} // end of namespace DomTreeBuilder
-
 //===-------------------------------------
 /// DominatorTree GraphTraits specialization so the DominatorTree can be
 /// iterable by generic graph iterators.
 ///
 template <> struct GraphTraits<clang::DomTreeNode *> {
   using NodeRef = ::clang::DomTreeNode *;
   using ChildIteratorType = ::clang::DomTreeNode::iterator;
 

llvm/include/llvm/IR/CFGDiff.h

 //===- CFGDiff.h - Define a CFG snapshot. -----------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
void print(raw_ostream &OS) const {
     OS << "\n";
   }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
   LLVM_DUMP_METHOD void dump() const { print(dbgs()); }
 #endif
 };
 
+namespace {
+template <bool B> struct reverse_if_helper;
+template <> struct reverse_if_helper<true> {
+  template <typename Range> auto operator()(Range &&R) {
+    return llvm::reverse(std::forward<Range>(R));
+  }
+};
+
+template <> struct reverse_if_helper<false> {
+  template <typename Range> auto operator()(Range &&R) {
+    return std::forward<Range>(R);
+  }
+};
+
+template <bool B, typename Range> auto reverse_if(Range &&R) {
+  return reverse_if_helper<B>()(std::forward<Range>(R));
+}
+} // namespace
+
 template <typename GraphT, bool InverseGraph = false, bool InverseEdge = false,
           typename GT = GraphTraits<GraphT>>
 struct CFGViewChildren {
   using DataRef = const GraphDiff<typename GT::NodeRef, InverseGraph> *;
   using NodeRef = std::pair<DataRef, typename GT::NodeRef>;
 
   template<typename Range>
   static auto makeChildRange(Range &&R, DataRef DR) {
     using Iter = WrappedPairNodeDataIterator<decltype(std::forward<Range>(R).begin()), NodeRef, DataRef>;
     return make_range(Iter(R.begin(), DR), Iter(R.end(), DR));
   }
 
   static auto children(NodeRef N) {
 
     // filter iterator init:
     auto R = make_range(GT::child_begin(N.second), GT::child_end(N.second));
+    auto RR = reverse_if<!InverseEdge>(R);
     // This lambda is copied into the iterators and persists to callers, ensure
     // captures are by value or otherwise have sufficient lifetime.
-    auto First = make_filter_range(makeChildRange(R, N.first), [N](NodeRef C) {
+    auto First = make_filter_range(makeChildRange(RR, N.first), [N](NodeRef C) {
       return !C.first->ignoreChild(N.second, C.second, InverseEdge);
     });
 
     // new inserts iterator init:
     auto InsertVec = N.first->getAddedChildren(N.second, InverseEdge);
     auto Second = makeChildRange(InsertVec, N.first);
 
     auto CR = concat<NodeRef>(First, Second);

llvm/include/llvm/IR/Dominators.h

 //===- Dominators.h - Dominator Info Calculation ----------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //

 extern template class cfg::Update<BasicBlock *>;
 
 namespace DomTreeBuilder {
 using BBDomTree = DomTreeBase<BasicBlock>;
 using BBPostDomTree = PostDomTreeBase<BasicBlock>;
 
 using BBUpdates = ArrayRef<llvm::cfg::Update<BasicBlock *>>;
 
+using BBDomTreeGraphDiff = GraphDiff<BasicBlock *, false>;
+using BBPostDomTreeGraphDiff = GraphDiff<BasicBlock *, true>;
+
 extern template void Calculate<BBDomTree>(BBDomTree &DT);
 extern template void CalculateWithUpdates<BBDomTree>(BBDomTree &DT,
                                                      BBUpdates U);
 
 extern template void Calculate<BBPostDomTree>(BBPostDomTree &DT);
 
 extern template void InsertEdge<BBDomTree>(BBDomTree &DT, BasicBlock *From,
                                            BasicBlock *To);
 extern template void InsertEdge<BBPostDomTree>(BBPostDomTree &DT,
                                                BasicBlock *From,
                                                BasicBlock *To);
 
 extern template void DeleteEdge<BBDomTree>(BBDomTree &DT, BasicBlock *From,
                                            BasicBlock *To);
 extern template void DeleteEdge<BBPostDomTree>(BBPostDomTree &DT,
                                                BasicBlock *From,
                                                BasicBlock *To);
 
-extern template void ApplyUpdates<BBDomTree>(BBDomTree &DT, BBUpdates);
-extern template void ApplyUpdates<BBPostDomTree>(BBPostDomTree &DT, BBUpdates);
+extern template void ApplyUpdates<BBDomTree>(BBDomTree &DT,
+                                             BBDomTreeGraphDiff &);
+extern template void ApplyUpdates<BBPostDomTree>(BBPostDomTree &DT,
+                                                 BBPostDomTreeGraphDiff &);
 
 extern template bool Verify<BBDomTree>(const BBDomTree &DT,
                                        BBDomTree::VerificationLevel VL);
 extern template bool Verify<BBPostDomTree>(const BBPostDomTree &DT,
                                            BBPostDomTree::VerificationLevel VL);
 }  // namespace DomTreeBuilder
 
 using DomTreeNode = DomTreeNodeBase<BasicBlock>;

llvm/include/llvm/Support/GenericDomTree.h

 //===- GenericDomTree.h - Generic dominator trees for graphs ----*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 /// \file

 #define LLVM_SUPPORT_GENERICDOMTREE_H
 
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/GraphTraits.h"
 #include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/IR/CFGDiff.h"
 #include "llvm/Support/CFGUpdate.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstddef>
 #include <iterator>
 #include <memory>
 #include <type_traits>
void InsertEdge(DomTreeT &DT, typename DomTreeT::NodePtr From,
                 typename DomTreeT::NodePtr To);
 
 template <typename DomTreeT>
 void DeleteEdge(DomTreeT &DT, typename DomTreeT::NodePtr From,
                 typename DomTreeT::NodePtr To);
 
 template <typename DomTreeT>
 void ApplyUpdates(DomTreeT &DT,
-                  ArrayRef<typename DomTreeT::UpdateType> Updates);
+                  GraphDiff<typename DomTreeT::NodePtr,
+                            DomTreeT::IsPostDominator> &PreViewCFG);
 
 template <typename DomTreeT>
 bool Verify(const DomTreeT &DT, typename DomTreeT::VerificationLevel VL);
 }  // namespace DomTreeBuilder
 
 /// Core dominator tree base class.
 ///
 /// This class is a generic template over graph nodes. It is instantiated for
#endif
   /// cases.
   ///
   /// Note that for postdominators it automatically takes care of applying
   /// updates on reverse edges internally (so there's no need to swap the
   /// From and To pointers when constructing DominatorTree::UpdateType).
   /// The type of updates is the same for DomTreeBase<T> and PostDomTreeBase<T>
   /// with the same template parameter T.
   ///
-  /// \param Updates An unordered sequence of updates to perform.
+  /// \param Updates An unordered sequence of updates to perform. The current
+  /// CFG and the reverse of these updates provides the pre-view of the CFG.
   ///
   void applyUpdates(ArrayRef<UpdateType> Updates) {
-    DomTreeBuilder::ApplyUpdates(*this, Updates);
+    GraphDiff<NodePtr, IsPostDominator> PreViewCFG(
+        Updates, /*ReverseApplyUpdates=*/true);
+    DomTreeBuilder::ApplyUpdates(*this, PreViewCFG);
   }
 
   /// Inform the dominator tree about a CFG edge insertion and update the tree.
   ///
   /// This function has to be called just before or just after making the update
   /// on the actual CFG. There cannot be any other updates that the dominator
   /// tree doesn't know about.
   ///

llvm/include/llvm/Support/GenericDomTreeConstruction.h

 //===- GenericDomTreeConstruction.h - Dominator Calculation ------*- C++ -*-==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 /// \file

 
 template <typename DomTreeT>
 struct SemiNCAInfo {
   using NodePtr = typename DomTreeT::NodePtr;
   using NodeT = typename DomTreeT::NodeType;
   using TreeNodePtr = DomTreeNodeBase<NodeT> *;
   using RootsT = decltype(DomTreeT::Roots);
   static constexpr bool IsPostDom = DomTreeT::IsPostDominator;
+  using GraphDiffT = GraphDiff<NodePtr, IsPostDom>;
 
   // Information record used by Semi-NCA during tree construction.
   struct InfoRec {
     unsigned DFSNum = 0;
     unsigned Parent = 0;
     unsigned Semi = 0;
     NodePtr Label = nullptr;
     NodePtr IDom = nullptr;
     SmallVector<NodePtr, 2> ReverseChildren;
   };
 
   // Number to node mapping is 1-based. Initialize the mapping to start with
   // a dummy element.
   std::vector<NodePtr> NumToNode = {nullptr};
   DenseMap<NodePtr, InfoRec> NodeToInfo;
 
   using UpdateT = typename DomTreeT::UpdateType;
   using UpdateKind = typename DomTreeT::UpdateKind;
   struct BatchUpdateInfo {
-    SmallVector<UpdateT, 4> Updates;
-    using NodePtrAndKind = PointerIntPair<NodePtr, 1, UpdateKind>;
+    // Note: Updates inside PreViewOfCFG are aleady legalized.
+    BatchUpdateInfo(GraphDiffT &PreViewOfCFG)
+        : PreViewOfCFG(&PreViewOfCFG),
+          NumLegalized(PreViewOfCFG.getNumLegalizedUpdates()) {}
 
-    // In order to be able to walk a CFG that is out of sync with the CFG
-    // DominatorTree last knew about, use the list of updates to reconstruct
-    // previous CFG versions of the current CFG. For each node, we store a set
-    // of its virtually added/deleted future successors and predecessors.
-    // Note that these children are from the future relative to what the
-    // DominatorTree knows about -- using them to gets us some snapshot of the
-    // CFG from the past (relative to the state of the CFG).
-    DenseMap<NodePtr, SmallVector<NodePtrAndKind, 4>> FutureSuccessors;
-    DenseMap<NodePtr, SmallVector<NodePtrAndKind, 4>> FuturePredecessors;
     // Remembers if the whole tree was recalculated at some point during the
     // current batch update.
     bool IsRecalculated = false;
+    GraphDiffT *PreViewOfCFG;
+    const size_t NumLegalized;
   };
 
   BatchUpdateInfo *BatchUpdates;
   using BatchUpdatePtr = BatchUpdateInfo *;
+  std::unique_ptr<GraphDiffT> EmptyGD;
 
   // If BUI is a nullptr, then there's no batch update in progress.
-  SemiNCAInfo(BatchUpdatePtr BUI) : BatchUpdates(BUI) {}
+  SemiNCAInfo(BatchUpdatePtr BUI) : BatchUpdates(BUI) {
+    if (!BatchUpdates)
+      EmptyGD = std::make_unique<GraphDiffT>();
+  }
 
   void clear() {
     NumToNode = {nullptr}; // Restore to initial state with a dummy start node.
     NodeToInfo.clear();
     // Don't reset the pointer to BatchUpdateInfo here -- if there's an update
     // in progress, we need this information to continue it.
   }
 
-  template <bool Inverse>
-  struct ChildrenGetter {
-    using ResultTy = SmallVector<NodePtr, 8>;
-
-    static ResultTy Get(NodePtr N, std::integral_constant<bool, false>) {
-      auto RChildren = reverse(children<NodePtr>(N));
-      return ResultTy(RChildren.begin(), RChildren.end());
-    }
-
-    static ResultTy Get(NodePtr N, std::integral_constant<bool, true>) {
-      auto IChildren = inverse_children<NodePtr>(N);
-      return ResultTy(IChildren.begin(), IChildren.end());
-    }
-
-    using Tag = std::integral_constant<bool, Inverse>;
-
-    // The function below is the core part of the batch updater. It allows the
-    // Depth Based Search algorithm to perform incremental updates in lockstep
-    // with updates to the CFG. We emulated lockstep CFG updates by getting its
-    // next snapshots by reverse-applying future updates.
-    static ResultTy Get(NodePtr N, BatchUpdatePtr BUI) {
-      ResultTy Res = Get(N, Tag());
-      // If there's no batch update in progress, simply return node's children.
-      if (!BUI) return Res;
-
-      // CFG children are actually its *most current* children, and we have to
-      // reverse-apply the future updates to get the node's children at the
-      // point in time the update was performed.
-      auto &FutureChildren = (Inverse != IsPostDom) ? BUI->FuturePredecessors
-                                                    : BUI->FutureSuccessors;
-      auto FCIt = FutureChildren.find(N);
-      if (FCIt == FutureChildren.end()) return Res;
-
-      for (auto ChildAndKind : FCIt->second) {
-        const NodePtr Child = ChildAndKind.getPointer();
-        const UpdateKind UK = ChildAndKind.getInt();
-
-        // Reverse-apply the future update.
-        if (UK == UpdateKind::Insert) {
-          // If there's an insertion in the future, it means that the edge must
-          // exist in the current CFG, but was not present in it before.
-          assert(llvm::find(Res, Child) != Res.end()
-                 && "Expected child not found in the CFG");
-          Res.erase(std::remove(Res.begin(), Res.end(), Child), Res.end());
-          LLVM_DEBUG(dbgs() << "\tHiding edge " << BlockNamePrinter(N) << " -> "
-                            << BlockNamePrinter(Child) << "\n");
-        } else {
-          // If there's an deletion in the future, it means that the edge cannot
-          // exist in the current CFG, but existed in it before.
-          assert(llvm::find(Res, Child) == Res.end() &&
-                 "Unexpected child found in the CFG");
-          LLVM_DEBUG(dbgs() << "\tShowing virtual edge " << BlockNamePrinter(N)
-                            << " -> " << BlockNamePrinter(Child) << "\n");
-          Res.push_back(Child);
-        }
-      }
-
-      return Res;
-    }
-  };
-
   NodePtr getIDom(NodePtr BB) const {
     auto InfoIt = NodeToInfo.find(BB);
     if (InfoIt == NodeToInfo.end()) return nullptr;
 
     return InfoIt->second.IDom;
   }
 
   TreeNodePtr getNodeForBlock(NodePtr BB, DomTreeT &DT) {
while (!WorkList.empty()) {
 
       // Visited nodes always have positive DFS numbers.
       if (BBInfo.DFSNum != 0) continue;
       BBInfo.DFSNum = BBInfo.Semi = ++LastNum;
       BBInfo.Label = BB;
       NumToNode.push_back(BB);
 
       constexpr bool Direction = IsReverse != IsPostDom;  // XOR.
-      for (const NodePtr Succ :
-           ChildrenGetter<Direction>::Get(BB, BatchUpdates)) {
+      typedef
+          typename std::conditional<Direction, Inverse<NodePtr>, NodePtr>::type
+              IsRevType;
+      using GraphDiffBBPair = std::pair<const GraphDiffT *, IsRevType>;
+      const GraphDiffT *GDTmp =
+          BatchUpdates ? BatchUpdates->PreViewOfCFG : EmptyGD.get();
+      std::pair<const GraphDiffT *, NodePtr> GDNodePair =
+          std::make_pair(GDTmp, BB);
+      for (auto &Pair : children<GraphDiffBBPair>(GDNodePair)) {
+        const NodePtr Succ = Pair.second;
         const auto SIT = NodeToInfo.find(Succ);
         // Don't visit nodes more than once but remember to collect
         // ReverseChildren.
         if (SIT != NodeToInfo.end() && SIT->second.DFSNum != 0) {
           if (Succ != BB) SIT->second.ReverseChildren.push_back(BB);
           continue;
         }
 
void addVirtualRoot() {
     NumToNode.push_back(nullptr);  // NumToNode[1] = nullptr;
   }
 
   // For postdominators, nodes with no forward successors are trivial roots that
   // are always selected as tree roots. Roots with forward successors correspond
   // to CFG nodes within infinite loops.
   static bool HasForwardSuccessors(const NodePtr N, BatchUpdatePtr BUI) {
     assert(N && "N must be a valid node");
-    return !ChildrenGetter<false>::Get(N, BUI).empty();
+    GraphDiffT EmptyGD;
+    using GraphDiffBBPair = std::pair<const GraphDiffT *, NodePtr>;
+    const GraphDiffT *GDTmp = BUI ? BUI->PreViewOfCFG : &EmptyGD;
+    std::pair<const GraphDiffT *, NodePtr> GDNodePair =
+        std::make_pair(GDTmp, N);
+    for (auto &Pair : children<GraphDiffBBPair>(GDNodePair)) {
+      (void)Pair;
+      return true;
+    }
+    return false;
   }
 
   static NodePtr GetEntryNode(const DomTreeT &DT) {
     assert(DT.Parent && "Parent not set");
     return GraphTraits<typename DomTreeT::ParentPtr>::getEntryNode(DT.Parent);
   }
 
   // Finds all roots without relaying on the set of roots already stored in the
while (!II.Bucket.empty()) {
         // Unlike regular Dijkstra, we have an inner loop to expand more
         // vertices. The first iteration is for the (affected) vertex popped
         // from II.Bucket and the rest are for vertices in
         // UnaffectedOnCurrentLevel, which may eventually expand to affected
         // vertices.
         //
         // Invariant: there is an optimal path from `To` to TN with the minimum
         // depth being CurrentLevel.
-        for (const NodePtr Succ :
-             ChildrenGetter<IsPostDom>::Get(TN->getBlock(), BUI)) {
+        GraphDiffT EmptyGD;
+        typedef typename std::conditional<IsPostDom, Inverse<NodePtr>,
+                                          NodePtr>::type IsRevType;
+        using GraphDiffBBPair = std::pair<const GraphDiffT *, IsRevType>;
+        const GraphDiffT *GDTmp = BUI ? BUI->PreViewOfCFG : &EmptyGD;
+        std::pair<const GraphDiffT *, NodePtr> GDNodePair =
+            std::make_pair(GDTmp, TN->getBlock());
+        for (auto &Pair : children<GraphDiffBBPair>(GDNodePair)) {
+          const NodePtr Succ = Pair.second;
           const TreeNodePtr SuccTN = DT.getNode(Succ);
           assert(SuccTN &&
                  "Unreachable successor found at reachable insertion");
           const unsigned SuccLevel = SuccTN->getLevel();
 
           LLVM_DEBUG(dbgs() << "\tSuccessor " << BlockNamePrinter(Succ)
                             << ", level = " << SuccLevel << "\n");
 
static void DeleteEdge(DomTreeT &DT, const BatchUpdatePtr BUI,
     LLVM_DEBUG(dbgs() << "Deleting edge " << BlockNamePrinter(From) << " -> "
                       << BlockNamePrinter(To) << "\n");
 
 #ifndef NDEBUG
     // Ensure that the edge was in fact deleted from the CFG before informing
     // the DomTree about it.
     // The check is O(N), so run it only in debug configuration.
     auto IsSuccessor = [BUI](const NodePtr SuccCandidate, const NodePtr Of) {
-      auto Successors = ChildrenGetter<IsPostDom>::Get(Of, BUI);
-      return llvm::find(Successors, SuccCandidate) != Successors.end();
+      GraphDiffT EmptyGD;
+      typedef
+          typename std::conditional<IsPostDom, Inverse<NodePtr>, NodePtr>::type
+              IsRevType;
+      using GraphDiffBBPair = std::pair<const GraphDiffT *, IsRevType>;
+      const GraphDiffT *GDTmp = BUI ? BUI->PreViewOfCFG : &EmptyGD;
+      std::pair<const GraphDiffT *, NodePtr> GDNodePair =
+          std::make_pair(GDTmp, Of);
+      for (auto &Pair : children<GraphDiffBBPair>(GDNodePair))
+        if (Pair.second == SuccCandidate)
+          return true;
+      return false;
     };
     (void)IsSuccessor;
     assert(!IsSuccessor(To, From) && "Deleted edge still exists in the CFG!");
 #endif
 
     const TreeNodePtr FromTN = DT.getNode(From);
     // Deletion in an unreachable subtree -- nothing to do.
     if (!FromTN) return;
#endif
   }
 
   // Checks if a node has proper support, as defined on the page 3 and later
   // explained on the page 7 of [2].
   static bool HasProperSupport(DomTreeT &DT, const BatchUpdatePtr BUI,
                                const TreeNodePtr TN) {
     LLVM_DEBUG(dbgs() << "IsReachableFromIDom " << BlockNamePrinter(TN)
                       << "\n");
-    for (const NodePtr Pred :
-         ChildrenGetter<!IsPostDom>::Get(TN->getBlock(), BUI)) {
+    GraphDiffT EmptyGD;
+    typedef
+        typename std::conditional<!IsPostDom, Inverse<NodePtr>, NodePtr>::type
+            IsRevType;
+    using GraphDiffBBPair = std::pair<const GraphDiffT *, IsRevType>;
+    const GraphDiffT *GDTmp = BUI ? BUI->PreViewOfCFG : &EmptyGD;
+    std::pair<const GraphDiffT *, NodePtr> GDNodePair =
+        std::make_pair(GDTmp, TN->getBlock());
+    for (auto &Pair : children<GraphDiffBBPair>(GDNodePair)) {
+      const NodePtr Pred = Pair.second;
       LLVM_DEBUG(dbgs() << "\tPred " << BlockNamePrinter(Pred) << "\n");
       if (!DT.getNode(Pred)) continue;
 
       const NodePtr Support =
           DT.findNearestCommonDominator(TN->getBlock(), Pred);
       LLVM_DEBUG(dbgs() << "\tSupport " << BlockNamePrinter(Support) << "\n");
       if (Support != TN->getBlock()) {
         LLVM_DEBUG(dbgs() << "\t" << BlockNamePrinter(TN)
static void EraseNode(DomTreeT &DT, const TreeNodePtr TN) {
 
     DT.DomTreeNodes.erase(TN->getBlock());
   }
 
   //~~
   //===--------------------- DomTree Batch Updater --------------------------===
   //~~
 
-  static void ApplyUpdates(DomTreeT &DT, ArrayRef<UpdateT> Updates) {
-    const size_t NumUpdates = Updates.size();
+  static void ApplyUpdates(DomTreeT &DT, GraphDiffT &PreViewCFG) {
+    const size_t NumUpdates = PreViewCFG.getNumLegalizedUpdates();
     if (NumUpdates == 0)
       return;
 
     // Take the fast path for a single update and avoid running the batch update
     // machinery.
     if (NumUpdates == 1) {
-      const auto &Update = Updates.front();
+      UpdateT Update = PreViewCFG.popUpdateForIncrementalUpdates();
       if (Update.getKind() == UpdateKind::Insert)
-        DT.insertEdge(Update.getFrom(), Update.getTo());
+        InsertEdge(DT, nullptr, Update.getFrom(), Update.getTo());
       else
-        DT.deleteEdge(Update.getFrom(), Update.getTo());
-
+        DeleteEdge(DT, nullptr, Update.getFrom(), Update.getTo());
       return;
     }
 
-    BatchUpdateInfo BUI;
-    LLVM_DEBUG(dbgs() << "Legalizing " << BUI.Updates.size() << " updates\n");
-    cfg::LegalizeUpdates<NodePtr>(Updates, BUI.Updates, IsPostDom);
-
-    const size_t NumLegalized = BUI.Updates.size();
-    BUI.FutureSuccessors.reserve(NumLegalized);
-    BUI.FuturePredecessors.reserve(NumLegalized);
-
-    // Use the legalized future updates to initialize future successors and
-    // predecessors. Note that these sets will only decrease size over time, as
-    // the next CFG snapshots slowly approach the actual (current) CFG.
-    for (UpdateT &U : BUI.Updates) {
-      BUI.FutureSuccessors[U.getFrom()].push_back({U.getTo(), U.getKind()});
-      BUI.FuturePredecessors[U.getTo()].push_back({U.getFrom(), U.getKind()});
-    }
-
-#if 0
-    // FIXME: The LLVM_DEBUG macro only plays well with a modular
-    // build of LLVM when the header is marked as textual, but doing
-    // so causes redefinition errors.
-    LLVM_DEBUG(dbgs() << "About to apply " << NumLegalized << " updates\n");
-    LLVM_DEBUG(if (NumLegalized < 32) for (const auto &U
-                                           : reverse(BUI.Updates)) {
-      dbgs() << "\t";
-      U.dump();
-      dbgs() << "\n";
-    });
-    LLVM_DEBUG(dbgs() << "\n");
-#endif
-
+    BatchUpdateInfo BUI(PreViewCFG);
     // Recalculate the DominatorTree when the number of updates
     // exceeds a threshold, which usually makes direct updating slower than
     // recalculation. We select this threshold proportional to the
     // size of the DominatorTree. The constant is selected
     // by choosing the one with an acceptable performance on some real-world
     // inputs.
 
     // Make unittests of the incremental algorithm work
     if (DT.DomTreeNodes.size() <= 100) {
-      if (NumLegalized > DT.DomTreeNodes.size())
+      if (BUI.NumLegalized > DT.DomTreeNodes.size())
         CalculateFromScratch(DT, &BUI);
-    } else if (NumLegalized > DT.DomTreeNodes.size() / 40)
+    } else if (BUI.NumLegalized > DT.DomTreeNodes.size() / 40)
       CalculateFromScratch(DT, &BUI);
 
     // If the DominatorTree was recalculated at some point, stop the batch
     // updates. Full recalculations ignore batch updates and look at the actual
     // CFG.
-    for (size_t i = 0; i < NumLegalized && !BUI.IsRecalculated; ++i)
+    for (size_t i = 0; i < BUI.NumLegalized && !BUI.IsRecalculated; ++i)
       ApplyNextUpdate(DT, BUI);
   }
 
   static void ApplyNextUpdate(DomTreeT &DT, BatchUpdateInfo &BUI) {
-    assert(!BUI.Updates.empty() && "No updates to apply!");
-    UpdateT CurrentUpdate = BUI.Updates.pop_back_val();
+    // Popping the next update, will move the PreViewOfCFG to the next snapshot.
+    UpdateT CurrentUpdate = BUI.PreViewOfCFG->popUpdateForIncrementalUpdates();
 #if 0
     // FIXME: The LLVM_DEBUG macro only plays well with a modular
     // build of LLVM when the header is marked as textual, but doing
     // so causes redefinition errors.
     LLVM_DEBUG(dbgs() << "Applying update: ");
     LLVM_DEBUG(CurrentUpdate.dump(); dbgs() << "\n");
 #endif
 
-    // Move to the next snapshot of the CFG by removing the reverse-applied
-    // current update. Since updates are performed in the same order they are
-    // legalized it's sufficient to pop the last item here.
-    auto &FS = BUI.FutureSuccessors[CurrentUpdate.getFrom()];
-    assert(FS.back().getPointer() == CurrentUpdate.getTo() &&
-           FS.back().getInt() == CurrentUpdate.getKind());
-    FS.pop_back();
-    if (FS.empty()) BUI.FutureSuccessors.erase(CurrentUpdate.getFrom());
-
-    auto &FP = BUI.FuturePredecessors[CurrentUpdate.getTo()];
-    assert(FP.back().getPointer() == CurrentUpdate.getFrom() &&
-           FP.back().getInt() == CurrentUpdate.getKind());
-    FP.pop_back();
-    if (FP.empty()) BUI.FuturePredecessors.erase(CurrentUpdate.getTo());
-
     if (CurrentUpdate.getKind() == UpdateKind::Insert)
       InsertEdge(DT, &BUI, CurrentUpdate.getFrom(), CurrentUpdate.getTo());
     else
       DeleteEdge(DT, &BUI, CurrentUpdate.getFrom(), CurrentUpdate.getTo());
   }
 
   //~~
   //===--------------- DomTree correctness verification ---------------------===

 template <class DomTreeT>
 void Calculate(DomTreeT &DT) {
   SemiNCAInfo<DomTreeT>::CalculateFromScratch(DT, nullptr);
 }
 
 template <typename DomTreeT>
 void CalculateWithUpdates(DomTreeT &DT,
                           ArrayRef<typename DomTreeT::UpdateType> Updates) {
-  // TODO: Move BUI creation in common method, reuse in ApplyUpdates.
-  typename SemiNCAInfo<DomTreeT>::BatchUpdateInfo BUI;
-  LLVM_DEBUG(dbgs() << "Legalizing " << BUI.Updates.size() << " updates\n");
-  cfg::LegalizeUpdates<typename DomTreeT::NodePtr>(Updates, BUI.Updates,
-                                                   DomTreeT::IsPostDominator);
-  const size_t NumLegalized = BUI.Updates.size();
-  BUI.FutureSuccessors.reserve(NumLegalized);
-  BUI.FuturePredecessors.reserve(NumLegalized);
-  for (auto &U : BUI.Updates) {
-    BUI.FutureSuccessors[U.getFrom()].push_back({U.getTo(), U.getKind()});
-    BUI.FuturePredecessors[U.getTo()].push_back({U.getFrom(), U.getKind()});
-  }
-
+  // FIXME: Updated to use the PreViewCFG and behave the same as until now.
+  // This behavior is however incorrect; this actually needs the PostViewCFG.
+  GraphDiff<typename DomTreeT::NodePtr, DomTreeT::IsPostDominator> PreViewCFG(
+      Updates, /*ReverseApplyUpdates=*/true);
+  typename SemiNCAInfo<DomTreeT>::BatchUpdateInfo BUI(PreViewCFG);
   SemiNCAInfo<DomTreeT>::CalculateFromScratch(DT, &BUI);
 }
 
 template <class DomTreeT>
 void InsertEdge(DomTreeT &DT, typename DomTreeT::NodePtr From,
                 typename DomTreeT::NodePtr To) {
   if (DT.isPostDominator()) std::swap(From, To);
   SemiNCAInfo<DomTreeT>::InsertEdge(DT, nullptr, From, To);
 }
 
 template <class DomTreeT>
 void DeleteEdge(DomTreeT &DT, typename DomTreeT::NodePtr From,
                 typename DomTreeT::NodePtr To) {
   if (DT.isPostDominator()) std::swap(From, To);
   SemiNCAInfo<DomTreeT>::DeleteEdge(DT, nullptr, From, To);
 }
 
 template <class DomTreeT>
 void ApplyUpdates(DomTreeT &DT,
-                  ArrayRef<typename DomTreeT::UpdateType> Updates) {
-  SemiNCAInfo<DomTreeT>::ApplyUpdates(DT, Updates);
+                  GraphDiff<typename DomTreeT::NodePtr,
+                            DomTreeT::IsPostDominator> &PreViewCFG) {
+  SemiNCAInfo<DomTreeT>::ApplyUpdates(DT, PreViewCFG);
 }
 
 template <class DomTreeT>
 bool Verify(const DomTreeT &DT, typename DomTreeT::VerificationLevel VL) {
   SemiNCAInfo<DomTreeT> SNCA(nullptr);
 
   // Simplist check is to compare against a new tree. This will also
   // usefully print the old and new trees, if they are different.

llvm/lib/IR/Dominators.cpp

 //===- Dominators.cpp - Dominator Calculation -----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
template void llvm::DomTreeBuilder::InsertEdge<DomTreeBuilder::BBPostDomTree>(
     DomTreeBuilder::BBPostDomTree &DT, BasicBlock *From, BasicBlock *To);
 
 template void llvm::DomTreeBuilder::DeleteEdge<DomTreeBuilder::BBDomTree>(
     DomTreeBuilder::BBDomTree &DT, BasicBlock *From, BasicBlock *To);
 template void llvm::DomTreeBuilder::DeleteEdge<DomTreeBuilder::BBPostDomTree>(
     DomTreeBuilder::BBPostDomTree &DT, BasicBlock *From, BasicBlock *To);
 
 template void llvm::DomTreeBuilder::ApplyUpdates<DomTreeBuilder::BBDomTree>(
-    DomTreeBuilder::BBDomTree &DT, DomTreeBuilder::BBUpdates);
+    DomTreeBuilder::BBDomTree &DT, DomTreeBuilder::BBDomTreeGraphDiff &);
 template void llvm::DomTreeBuilder::ApplyUpdates<DomTreeBuilder::BBPostDomTree>(
-    DomTreeBuilder::BBPostDomTree &DT, DomTreeBuilder::BBUpdates);
+    DomTreeBuilder::BBPostDomTree &DT,
+    DomTreeBuilder::BBPostDomTreeGraphDiff &);
 
 template bool llvm::DomTreeBuilder::Verify<DomTreeBuilder::BBDomTree>(
     const DomTreeBuilder::BBDomTree &DT,
     DomTreeBuilder::BBDomTree::VerificationLevel VL);
 template bool llvm::DomTreeBuilder::Verify<DomTreeBuilder::BBPostDomTree>(
     const DomTreeBuilder::BBPostDomTree &DT,
     DomTreeBuilder::BBPostDomTree::VerificationLevel VL);