From bb6015d9c64503b5c9672e15b5e15cc8d49d50c2 Mon Sep 17 00:00:00 2001 From: Alina Sbirlea Date: Thu, 16 Jul 2020 15:46:54 -0700 Subject: [PATCH] [GraphDiff] Use class method getChildren instead of GraphTraits. Summary: Use getChildren() method in GraphDiff instead of GraphTraits. This simplifies the code and allows for refactorigns inside GraphDiff. All usecase need not have a light-weight/copyable range. Clean GraphTraits implementation. Reviewers: dblaikie Subscribers: hiraditya, llvm-commits, george.burgess.iv Tags: #llvm Differential Revision: https://reviews.llvm.org/D84562 --- .../llvm/Analysis/IteratedDominanceFrontier.h | 8 +- include/llvm/Analysis/MemorySSAUpdater.h | 2 - include/llvm/Support/CFGDiff.h | 98 +------------------ lib/Analysis/MemorySSAUpdater.cpp | 11 +-- 4 files changed, 8 insertions(+), 111 deletions(-) diff --git a/include/llvm/Analysis/IteratedDominanceFrontier.h b/include/llvm/Analysis/IteratedDominanceFrontier.h index fb660528515..8166b52aa22 100644 --- a/include/llvm/Analysis/IteratedDominanceFrontier.h +++ b/include/llvm/Analysis/IteratedDominanceFrontier.h @@ -73,13 +73,7 @@ ChildrenGetterTy::get(const NodeRef &N) { return {Children.begin(), Children.end()}; } - using SnapShotBBPairTy = - std::pair *, OrderedNodeTy>; - - ChildrenTy Ret; - for (const auto &SnapShotBBPair : children({GD, N})) - Ret.emplace_back(SnapShotBBPair.second); - return Ret; + return GD->template getChildren(N); } } // end of namespace IDFCalculatorDetail diff --git a/include/llvm/Analysis/MemorySSAUpdater.h b/include/llvm/Analysis/MemorySSAUpdater.h index 20588ef083c..d41b9320997 100644 --- a/include/llvm/Analysis/MemorySSAUpdater.h +++ b/include/llvm/Analysis/MemorySSAUpdater.h @@ -52,8 +52,6 @@ class LoopBlocksRPO; using ValueToValueMapTy = ValueMap; using PhiToDefMap = SmallDenseMap; using CFGUpdate = cfg::Update; -using GraphDiffInvBBPair = - std::pair *, Inverse>; class MemorySSAUpdater { private: diff --git a/include/llvm/Support/CFGDiff.h b/include/llvm/Support/CFGDiff.h index 269984872bf..9cbf311f680 100644 --- a/include/llvm/Support/CFGDiff.h +++ b/include/llvm/Support/CFGDiff.h @@ -30,44 +30,6 @@ // a non-inversed graph, the children are naturally the successors when // InverseEdge is false and the predecessors when InverseEdge is true. -// We define two base clases that call into GraphDiff, one for successors -// (CFGSuccessors), where InverseEdge is false, and one for predecessors -// (CFGPredecessors), where InverseEdge is true. -// FIXME: Further refactoring may merge the two base classes into a single one -// templated / parametrized on using succ_iterator/pred_iterator and false/true -// for the InverseEdge. - -// CFGViewChildren and CFGViewPredecessors, both can be parametrized to -// consider the graph inverted or not (i.e. InverseGraph). Successors -// implicitly has InverseEdge = false and Predecessors implicitly has -// InverseEdge = true (see calls to GraphDiff methods in there). The GraphTraits -// instantiations that follow define the value of InverseGraph. - -// GraphTraits instantiations: -// - GraphDiff is equivalent to InverseGraph = false -// - GraphDiff> is equivalent to InverseGraph = true -// - second pair item is BasicBlock *, then InverseEdge = false (so it inherits -// from CFGViewChildren). -// - second pair item is Inverse, then InverseEdge = true (so it -// inherits from CFGViewPredecessors). - -// The 4 GraphTraits are as follows: -// 1. std::pair *, BasicBlock *>> : -// CFGViewChildren -// Regular CFG, children means successors, InverseGraph = false, -// InverseEdge = false. -// 2. std::pair> *, BasicBlock *>> : -// CFGViewChildren -// Reverse the graph, get successors but reverse-apply updates, -// InverseGraph = true, InverseEdge = false. -// 3. std::pair *, Inverse>> : -// CFGViewPredecessors -// Regular CFG, reverse edges, so children mean predecessors, -// InverseGraph = false, InverseEdge = true. -// 4. std::pair> *, Inverse> -// : CFGViewPredecessors -// Reverse the graph and the edges, InverseGraph = true, InverseEdge = true. - namespace llvm { namespace detail { @@ -87,9 +49,9 @@ template auto reverse_if(Range &&R) { } } // namespace detail -// GraphDiff defines a CFG snapshot: given a set of Update, provide -// utilities to skip edges marked as deleted and return a set of edges marked as -// newly inserted. The current diff treats the CFG as a graph rather than a +// GraphDiff defines a CFG snapshot: given a set of Update, provides +// a getChildren method to get a Node's children based on the additional updates +// in the snapshot. The current diff treats the CFG as a graph rather than a // multigraph. Added edges are pruned to be unique, and deleted edges will // remove all existing edges between two blocks. template class GraphDiff { @@ -191,7 +153,6 @@ public: } using VectRet = SmallVector; - template VectRet getChildren(NodePtr N) const { using DirectedNodeT = std::conditional_t, NodePtr>; @@ -228,59 +189,6 @@ public: LLVM_DUMP_METHOD void dump() const { print(dbgs()); } #endif }; - -template > -struct CFGViewChildren { - using DataRef = const GraphDiff *; - using NodeRef = std::pair; - - template - static auto makeChildRange(Range &&R, DataRef DR) { - using Iter = WrappedPairNodeDataIterator(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 = detail::reverse_if(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(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(First, Second); - - // concat_range contains references to other ranges, returning it would - // leave those references dangling - the iterators contain - // other iterators by value so they're safe to return. - return make_range(CR.begin(), CR.end()); - } - - static auto child_begin(NodeRef N) { - return children(N).begin(); - } - - static auto child_end(NodeRef N) { - return children(N).end(); - } - - using ChildIteratorType = decltype(child_end(std::declval())); -}; - -template -struct GraphTraits *, T>> - : CFGViewChildren {}; -template -struct GraphTraits *, Inverse>> - : CFGViewChildren, B, true> {}; } // end namespace llvm #endif // LLVM_SUPPORT_CFGDIFF_H diff --git a/lib/Analysis/MemorySSAUpdater.cpp b/lib/Analysis/MemorySSAUpdater.cpp index 85af091772e..21cbdcd6714 100644 --- a/lib/Analysis/MemorySSAUpdater.cpp +++ b/lib/Analysis/MemorySSAUpdater.cpp @@ -832,8 +832,8 @@ void MemorySSAUpdater::applyInsertUpdates(ArrayRef Updates, // Check number of predecessors, we only care if there's more than one. unsigned Count = 0; BasicBlock *Pred = nullptr; - for (auto &Pair : children({GD, BB})) { - Pred = Pair.second; + for (auto *Pi : GD->template getChildren(BB)) { + Pred = Pi; Count++; if (Count == 2) break; @@ -926,8 +926,7 @@ void MemorySSAUpdater::applyInsertUpdates(ArrayRef Updates, auto *BB = BBPredPair.first; const auto &AddedBlockSet = BBPredPair.second.Added; auto &PrevBlockSet = BBPredPair.second.Prev; - for (auto &Pair : children({GD, BB})) { - BasicBlock *Pi = Pair.second; + for (auto *Pi : GD->template getChildren(BB)) { if (!AddedBlockSet.count(Pi)) PrevBlockSet.insert(Pi); EdgeCountMap[{Pi, BB}]++; @@ -1078,10 +1077,8 @@ void MemorySSAUpdater::applyInsertUpdates(ArrayRef Updates, for (unsigned I = 0, E = IDFPhi->getNumIncomingValues(); I < E; ++I) IDFPhi->setIncomingValue(I, GetLastDef(IDFPhi->getIncomingBlock(I))); } else { - for (auto &Pair : children({GD, BBIDF})) { - BasicBlock *Pi = Pair.second; + for (auto *Pi : GD->template getChildren(BBIDF)) IDFPhi->addIncoming(GetLastDef(Pi), Pi); - } } } }