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

Michael Kruse via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 9 18:43:27 PDT 2020


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


More information about the llvm-commits mailing list