[llvm] a903749 - [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.

Alina Sbirlea via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 9 20:49:49 PDT 2020


I'm hoping rG5da1671bf823 fixed them. Please let me know if not and I will
revert.

Alina

On Thu, Apr 9, 2020, 6:44 PM Michael Kruse <llvm-commits at meinersbur.de>
wrote:

> This broke the Windows build:
> http://lab.llvm.org:8011/builders/llvm-clang-x86_64-win-fast/builds/14172
>
> Michael
>
> Am Do., 9. Apr. 2020 um 20:09 Uhr schrieb Alina Sbirlea via
> llvm-commits <llvm-commits at lists.llvm.org>:
> >
> >
> > Author: Alina Sbirlea
> > Date: 2020-04-09T18:08:39-07:00
> > New Revision: a90374988e4eb8c50d91e11f4e61cdbd5debb235
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/a90374988e4eb8c50d91e11f4e61cdbd5debb235
> > DIFF:
> https://github.com/llvm/llvm-project/commit/a90374988e4eb8c50d91e11f4e61cdbd5debb235.diff
> >
> > LOG: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.
> >
> > Summary:
> > This replaces the ChildrenGetter inside the DominatorTree with
> > GraphTraits over a GraphDiff object, an object which encapsulated the
> > view of the previous CFG.
> > This also simplifies the extentions in clang which use DominatorTree, as
> > GraphDiff also filters nullptrs.
> >
> > Reviewers: kuhar, dblaikie, NutshellySima
> >
> > Subscribers: hiraditya, cfe-commits, llvm-commits
> >
> > Tags: #clang
> >
> > Differential Revision: https://reviews.llvm.org/D77341
> >
> > Added:
> >
> >
> > Modified:
> >     clang/include/clang/Analysis/Analyses/Dominators.h
> >     llvm/include/llvm/IR/CFGDiff.h
> >     llvm/include/llvm/IR/Dominators.h
> >     llvm/include/llvm/Support/GenericDomTree.h
> >     llvm/include/llvm/Support/GenericDomTreeConstruction.h
> >     llvm/lib/IR/Dominators.cpp
> >
> > Removed:
> >
> >
> >
> >
> ################################################################################
> > diff  --git a/clang/include/clang/Analysis/Analyses/Dominators.h
> b/clang/include/clang/Analysis/Analyses/Dominators.h
> > index 061c98137da2..c0cfc186c365 100644
> > --- a/clang/include/clang/Analysis/Analyses/Dominators.h
> > +++ b/clang/include/clang/Analysis/Analyses/Dominators.h
> > @@ -275,76 +275,6 @@ class ControlDependencyCalculator : public
> ManagedAnalysis {
> >
> >  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.
> >
> > diff  --git a/llvm/include/llvm/IR/CFGDiff.h
> b/llvm/include/llvm/IR/CFGDiff.h
> > index c50db0de79a3..5f2b5c709324 100644
> > --- a/llvm/include/llvm/IR/CFGDiff.h
> > +++ b/llvm/include/llvm/IR/CFGDiff.h
> > @@ -195,6 +195,23 @@ template <typename NodePtr, bool InverseGraph =
> false> class GraphDiff {
> >  #endif
> >  };
> >
> > +namespace detail {
> > +template <typename Range>
> > +auto reverse_if_helper(Range &&R, std::integral_constant<bool, false>) {
> > +  return std::forward<Range>(R);
> > +}
> > +
> > +template <typename Range>
> > +auto reverse_if_helper(Range &&R, std::integral_constant<bool, true>) {
> > +  return llvm::reverse(std::forward<Range>(R));
> > +}
> > +
> > +template <bool B, typename Range> auto reverse_if(Range &&R) {
> > +  return reverse_if_helper(std::forward<Range>(R),
> > +                           std::integral_constant<bool, B>{});
> > +}
> > +} // namespace detail
> > +
> >  template <typename GraphT, bool InverseGraph = false, bool InverseEdge
> = false,
> >            typename GT = GraphTraits<GraphT>>
> >  struct CFGViewChildren {
> > @@ -211,9 +228,10 @@ struct CFGViewChildren {
> >
> >      // filter iterator init:
> >      auto R = make_range(GT::child_begin(N.second),
> GT::child_end(N.second));
> > +    auto RR = detail::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);
> >      });
> >
> >
> > diff  --git a/llvm/include/llvm/IR/Dominators.h
> b/llvm/include/llvm/IR/Dominators.h
> > index 6a14785a6cc3..0c98902c72c8 100644
> > --- a/llvm/include/llvm/IR/Dominators.h
> > +++ b/llvm/include/llvm/IR/Dominators.h
> > @@ -44,6 +44,9 @@ 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);
> > @@ -62,8 +65,10 @@ 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);
> >
> > diff  --git a/llvm/include/llvm/Support/GenericDomTree.h
> b/llvm/include/llvm/Support/GenericDomTree.h
> > index 1e1fc1fa2486..4159baa50dad 100644
> > --- a/llvm/include/llvm/Support/GenericDomTree.h
> > +++ b/llvm/include/llvm/Support/GenericDomTree.h
> > @@ -29,6 +29,7 @@
> >  #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>
> > @@ -205,7 +206,8 @@ void DeleteEdge(DomTreeT &DT, typename
> DomTreeT::NodePtr From,
> >
> >  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);
> > @@ -515,10 +517,13 @@ class DominatorTreeBase {
> >    /// 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.
> >
> > diff  --git a/llvm/include/llvm/Support/GenericDomTreeConstruction.h
> b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
> > index a344f66c548e..2e848358bb5b 100644
> > --- a/llvm/include/llvm/Support/GenericDomTreeConstruction.h
> > +++ b/llvm/include/llvm/Support/GenericDomTreeConstruction.h
> > @@ -58,6 +58,13 @@ struct SemiNCAInfo {
> >    using TreeNodePtr = DomTreeNodeBase<NodeT> *;
> >    using RootsT = decltype(DomTreeT::Roots);
> >    static constexpr bool IsPostDom = DomTreeT::IsPostDominator;
> > +  using GraphDiffT = GraphDiff<NodePtr, IsPostDom>;
> > +  using GraphDiffNodePair = std::pair<const GraphDiffT *, NodePtr>;
> > +  using GraphDiffInvNodePair = std::pair<const GraphDiffT *,
> Inverse<NodePtr>>;
> > +  using GraphDiffNodePairIfDomT =
> > +      std::conditional_t<!IsPostDom, GraphDiffInvNodePair,
> GraphDiffNodePair>;
> > +  using GraphDiffNodePairIfPDomT =
> > +      std::conditional_t<IsPostDom, GraphDiffInvNodePair,
> GraphDiffNodePair>;
> >
> >    // Information record used by Semi-NCA during tree construction.
> >    struct InfoRec {
> > @@ -77,28 +84,27 @@ struct SemiNCAInfo {
> >    using UpdateT = typename DomTreeT::UpdateType;
> >    using UpdateKind = typename DomTreeT::UpdateKind;
> >    struct BatchUpdateInfo {
> > -    SmallVector<UpdateT, 4> Updates;
> > -    using NodePtrAndKind = PointerIntPair<NodePtr, 1, UpdateKind>;
> > -
> > -    // 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;
> > +    // Note: Updates inside PreViewCFG are aleady legalized.
> > +    BatchUpdateInfo(GraphDiffT &PreViewCFG)
> > +        : PreViewCFG(PreViewCFG),
> > +          NumLegalized(PreViewCFG.getNumLegalizedUpdates()) {}
> > +
> >      // Remembers if the whole tree was recalculated at some point
> during the
> >      // current batch update.
> >      bool IsRecalculated = false;
> > +    GraphDiffT &PreViewCFG;
> > +    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.
> > @@ -107,67 +113,6 @@ struct SemiNCAInfo {
> >      // 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;
> > @@ -235,8 +180,12 @@ struct SemiNCAInfo {
> >        NumToNode.push_back(BB);
> >
> >        constexpr bool Direction = IsReverse != IsPostDom;  // XOR.
> > -      for (const NodePtr Succ :
> > -           ChildrenGetter<Direction>::Get(BB, BatchUpdates)) {
> > +      using DirectedNodeT =
> > +          std::conditional_t<Direction, Inverse<NodePtr>, NodePtr>;
> > +      using GraphDiffBBPair = std::pair<const GraphDiffT *,
> DirectedNodeT>;
> > +      const auto *GD = BatchUpdates ? &BatchUpdates->PreViewCFG :
> EmptyGD.get();
> > +      for (auto &Pair : children<GraphDiffBBPair>({GD, BB})) {
> > +        const NodePtr Succ = Pair.second;
> >          const auto SIT = NodeToInfo.find(Succ);
> >          // Don't visit nodes more than once but remember to collect
> >          // ReverseChildren.
> > @@ -371,7 +320,9 @@ struct SemiNCAInfo {
> >    // 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;
> > +    auto &GD = BUI ? BUI->PreViewCFG : EmptyGD;
> > +    return !empty(children<GraphDiffNodePair>({&GD, N}));
> >    }
> >
> >    static NodePtr GetEntryNode(const DomTreeT &DT) {
> > @@ -795,8 +746,11 @@ struct SemiNCAInfo {
> >          //
> >          // 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;
> > +        auto &GD = BUI ? BUI->PreViewCFG : EmptyGD;
> > +        for (auto &Pair :
> > +             children<GraphDiffNodePairIfPDomT>({&GD, TN->getBlock()}))
> {
> > +          const NodePtr Succ = Pair.second;
> >            const TreeNodePtr SuccTN = DT.getNode(Succ);
> >            assert(SuccTN &&
> >                   "Unreachable successor found at reachable insertion");
> > @@ -926,8 +880,12 @@ struct SemiNCAInfo {
> >      // 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;
> > +      auto &GD = BUI ? BUI->PreViewCFG : EmptyGD;
> > +      for (auto &Pair : children<GraphDiffNodePairIfPDomT>({&GD, Of}))
> > +        if (Pair.second == SuccCandidate)
> > +          return true;
> > +      return false;
> >      };
> >      (void)IsSuccessor;
> >      assert(!IsSuccessor(To, From) && "Deleted edge still exists in the
> CFG!");
> > @@ -1013,15 +971,17 @@ struct SemiNCAInfo {
> >                                 const TreeNodePtr TN) {
> >      LLVM_DEBUG(dbgs() << "IsReachableFromIDom " << BlockNamePrinter(TN)
> >                        << "\n");
> > -    for (const NodePtr Pred :
> > -         ChildrenGetter<!IsPostDom>::Get(TN->getBlock(), BUI)) {
> > +    auto TNB = TN->getBlock();
> > +    GraphDiffT EmptyGD;
> > +    auto &GD = BUI ? BUI->PreViewCFG : EmptyGD;
> > +    for (auto &Pair : children<GraphDiffNodePairIfDomT>({&GD, TNB})) {
> > +      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);
> > +      const NodePtr Support = DT.findNearestCommonDominator(TNB, Pred);
> >        LLVM_DEBUG(dbgs() << "\tSupport " << BlockNamePrinter(Support) <<
> "\n");
> > -      if (Support != TN->getBlock()) {
> > +      if (Support != TNB) {
> >          LLVM_DEBUG(dbgs() << "\t" << BlockNamePrinter(TN)
> >                            << " is reachable from support "
> >                            << BlockNamePrinter(Support) << "\n");
> > @@ -1152,53 +1112,23 @@ struct SemiNCAInfo {
> >    //===--------------------- 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, /*BUI=*/nullptr, Update.getFrom(),
> Update.getTo());
> >        else
> > -        DT.deleteEdge(Update.getFrom(), Update.getTo());
> > -
> > +        DeleteEdge(DT, /*BUI=*/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
> > @@ -1208,21 +1138,21 @@ struct SemiNCAInfo {
> >
> >      // 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 PreViewCFG to the next
> snapshot.
> > +    UpdateT CurrentUpdate =
> BUI.PreViewCFG.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
> > @@ -1231,21 +1161,6 @@ struct SemiNCAInfo {
> >      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
> > @@ -1603,19 +1518,11 @@ void Calculate(DomTreeT &DT) {
> >  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);
> >  }
> >
> > @@ -1635,8 +1542,9 @@ void DeleteEdge(DomTreeT &DT, typename
> DomTreeT::NodePtr From,
> >
> >  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>
> >
> > diff  --git a/llvm/lib/IR/Dominators.cpp b/llvm/lib/IR/Dominators.cpp
> > index 4062ade645ba..a58380ff1f70 100644
> > --- a/llvm/lib/IR/Dominators.cpp
> > +++ b/llvm/lib/IR/Dominators.cpp
> > @@ -90,9 +90,10 @@ 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,
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20200409/8845ba3f/attachment.html>


More information about the llvm-commits mailing list