diff --git a/clang/include/clang/Analysis/Analyses/Dominators.h b/clang/include/clang/Analysis/Analyses/Dominators.h index c0cfc186c365a2..061c98137da29c 100644 --- a/clang/include/clang/Analysis/Analyses/Dominators.h +++ b/clang/include/clang/Analysis/Analyses/Dominators.h @@ -275,6 +275,76 @@ 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::ChildrenGetter< + /*Inverse=*/false>; + +template <> +template <> +inline ClangCFGDomChildrenGetter::ResultTy ClangCFGDomChildrenGetter::Get( + clang::CFGBlock *N, std::integral_constant) { + auto RChildren = reverse(children(N)); + ResultTy Ret(RChildren.begin(), RChildren.end()); + Ret.erase(std::remove(Ret.begin(), Ret.end(), nullptr), Ret.end()); + return Ret; +} + +using ClangCFGDomReverseChildrenGetter = +SemiNCAInfo::ChildrenGetter< + /*Inverse=*/true>; + +template <> +template <> +inline ClangCFGDomReverseChildrenGetter::ResultTy +ClangCFGDomReverseChildrenGetter::Get( + clang::CFGBlock *N, std::integral_constant) { + auto IChildren = inverse_children(N); + ResultTy Ret(IChildren.begin(), IChildren.end()); + Ret.erase(std::remove(Ret.begin(), Ret.end(), nullptr), Ret.end()); + return Ret; +} + +using ClangCFGPostDomChildrenGetter = +SemiNCAInfo::ChildrenGetter< + /*Inverse=*/false>; + +template <> +template <> +inline ClangCFGPostDomChildrenGetter::ResultTy +ClangCFGPostDomChildrenGetter::Get( + clang::CFGBlock *N, std::integral_constant) { + auto RChildren = reverse(children(N)); + ResultTy Ret(RChildren.begin(), RChildren.end()); + Ret.erase(std::remove(Ret.begin(), Ret.end(), nullptr), Ret.end()); + return Ret; +} + +using ClangCFGPostDomReverseChildrenGetter = +SemiNCAInfo::ChildrenGetter< + /*Inverse=*/true>; + +template <> +template <> +inline ClangCFGPostDomReverseChildrenGetter::ResultTy +ClangCFGPostDomReverseChildrenGetter::Get( + clang::CFGBlock *N, std::integral_constant) { + auto IChildren = inverse_children(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/Dominators.h b/llvm/include/llvm/IR/Dominators.h index 0c98902c72c820..6a14785a6cc30e 100644 --- a/llvm/include/llvm/IR/Dominators.h +++ b/llvm/include/llvm/IR/Dominators.h @@ -44,9 +44,6 @@ using BBPostDomTree = PostDomTreeBase; using BBUpdates = ArrayRef>; -using BBDomTreeGraphDiff = GraphDiff; -using BBPostDomTreeGraphDiff = GraphDiff; - extern template void Calculate(BBDomTree &DT); extern template void CalculateWithUpdates(BBDomTree &DT, BBUpdates U); @@ -65,10 +62,8 @@ extern template void DeleteEdge(BBPostDomTree &DT, BasicBlock *From, BasicBlock *To); -extern template void ApplyUpdates(BBDomTree &DT, - BBDomTreeGraphDiff &); -extern template void ApplyUpdates(BBPostDomTree &DT, - BBPostDomTreeGraphDiff &); +extern template void ApplyUpdates(BBDomTree &DT, BBUpdates); +extern template void ApplyUpdates(BBPostDomTree &DT, BBUpdates); extern template bool Verify(const BBDomTree &DT, BBDomTree::VerificationLevel VL); diff --git a/llvm/include/llvm/Support/CFGDiff.h b/llvm/include/llvm/Support/CFGDiff.h index 17eb3392d4ac67..94734ce70e02c0 100644 --- a/llvm/include/llvm/Support/CFGDiff.h +++ b/llvm/include/llvm/Support/CFGDiff.h @@ -194,23 +194,6 @@ template class GraphDiff { #endif }; -namespace detail { -template -auto reverse_if_helper(Range &&R, std::integral_constant) { - return std::forward(R); -} - -template -auto reverse_if_helper(Range &&R, std::integral_constant) { - return llvm::reverse(std::forward(R)); -} - -template auto reverse_if(Range &&R) { - return reverse_if_helper(std::forward(R), - std::integral_constant{}); -} -} // namespace detail - template > struct CFGViewChildren { @@ -227,10 +210,9 @@ struct CFGViewChildren { // 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) { + auto First = make_filter_range(makeChildRange(R, N.first), [N](NodeRef C) { return !C.first->ignoreChild(N.second, C.second, InverseEdge); }); diff --git a/llvm/include/llvm/Support/GenericDomTree.h b/llvm/include/llvm/Support/GenericDomTree.h index 3de0411e54834b..1e1fc1fa2486ef 100644 --- a/llvm/include/llvm/Support/GenericDomTree.h +++ b/llvm/include/llvm/Support/GenericDomTree.h @@ -29,7 +29,6 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/Support/CFGDiff.h" #include "llvm/Support/CFGUpdate.h" #include "llvm/Support/raw_ostream.h" #include @@ -206,8 +205,7 @@ void DeleteEdge(DomTreeT &DT, typename DomTreeT::NodePtr From, template void ApplyUpdates(DomTreeT &DT, - GraphDiff &PreViewCFG); + ArrayRef Updates); template bool Verify(const DomTreeT &DT, typename DomTreeT::VerificationLevel VL); @@ -517,13 +515,10 @@ class DominatorTreeBase { /// The type of updates is the same for DomTreeBase and PostDomTreeBase /// with the same template parameter T. /// - /// \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. + /// \param Updates An unordered sequence of updates to perform. /// void applyUpdates(ArrayRef Updates) { - GraphDiff PreViewCFG( - Updates, /*ReverseApplyUpdates=*/true); - DomTreeBuilder::ApplyUpdates(*this, PreViewCFG); + DomTreeBuilder::ApplyUpdates(*this, Updates); } /// 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 1d0dd09e91ce56..a344f66c548e27 100644 --- a/llvm/include/llvm/Support/GenericDomTreeConstruction.h +++ b/llvm/include/llvm/Support/GenericDomTreeConstruction.h @@ -58,13 +58,6 @@ struct SemiNCAInfo { using TreeNodePtr = DomTreeNodeBase *; using RootsT = decltype(DomTreeT::Roots); static constexpr bool IsPostDom = DomTreeT::IsPostDominator; - using GraphDiffT = GraphDiff; - using GraphDiffNodePair = std::pair; - using GraphDiffInvNodePair = std::pair>; - using GraphDiffNodePairIfDomT = - std::conditional_t; - using GraphDiffNodePairIfPDomT = - std::conditional_t; // Information record used by Semi-NCA during tree construction. struct InfoRec { @@ -84,27 +77,28 @@ struct SemiNCAInfo { using UpdateT = typename DomTreeT::UpdateType; using UpdateKind = typename DomTreeT::UpdateKind; struct BatchUpdateInfo { - // Note: Updates inside PreViewCFG are aleady legalized. - BatchUpdateInfo(GraphDiffT &PreViewCFG) - : PreViewCFG(PreViewCFG), - NumLegalized(PreViewCFG.getNumLegalizedUpdates()) {} - + SmallVector Updates; + using NodePtrAndKind = PointerIntPair; + + // 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> FutureSuccessors; + DenseMap> FuturePredecessors; // 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 EmptyGD; // If BUI is a nullptr, then there's no batch update in progress. - SemiNCAInfo(BatchUpdatePtr BUI) : BatchUpdates(BUI) { - if (!BatchUpdates) - EmptyGD = std::make_unique(); - } + SemiNCAInfo(BatchUpdatePtr BUI) : BatchUpdates(BUI) {} void clear() { NumToNode = {nullptr}; // Restore to initial state with a dummy start node. @@ -113,6 +107,67 @@ struct SemiNCAInfo { // in progress, we need this information to continue it. } + template + struct ChildrenGetter { + using ResultTy = SmallVector; + + static ResultTy Get(NodePtr N, std::integral_constant) { + auto RChildren = reverse(children(N)); + return ResultTy(RChildren.begin(), RChildren.end()); + } + + static ResultTy Get(NodePtr N, std::integral_constant) { + auto IChildren = inverse_children(N); + return ResultTy(IChildren.begin(), IChildren.end()); + } + + using Tag = std::integral_constant; + + // 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; @@ -180,12 +235,8 @@ struct SemiNCAInfo { NumToNode.push_back(BB); constexpr bool Direction = IsReverse != IsPostDom; // XOR. - using DirectedNodeT = - std::conditional_t, NodePtr>; - using GraphDiffBBPair = std::pair; - const auto *GD = BatchUpdates ? &BatchUpdates->PreViewCFG : EmptyGD.get(); - for (auto &Pair : children({GD, BB})) { - const NodePtr Succ = Pair.second; + for (const NodePtr Succ : + ChildrenGetter::Get(BB, BatchUpdates)) { const auto SIT = NodeToInfo.find(Succ); // Don't visit nodes more than once but remember to collect // ReverseChildren. @@ -320,9 +371,7 @@ struct SemiNCAInfo { // to CFG nodes within infinite loops. static bool HasForwardSuccessors(const NodePtr N, BatchUpdatePtr BUI) { assert(N && "N must be a valid node"); - GraphDiffT EmptyGD; - auto &GD = BUI ? BUI->PreViewCFG : EmptyGD; - return !llvm::empty(children({&GD, N})); + return !ChildrenGetter::Get(N, BUI).empty(); } static NodePtr GetEntryNode(const DomTreeT &DT) { @@ -746,11 +795,8 @@ struct SemiNCAInfo { // // Invariant: there is an optimal path from `To` to TN with the minimum // depth being CurrentLevel. - GraphDiffT EmptyGD; - auto &GD = BUI ? BUI->PreViewCFG : EmptyGD; - for (auto &Pair : - children({&GD, TN->getBlock()})) { - const NodePtr Succ = Pair.second; + for (const NodePtr Succ : + ChildrenGetter::Get(TN->getBlock(), BUI)) { const TreeNodePtr SuccTN = DT.getNode(Succ); assert(SuccTN && "Unreachable successor found at reachable insertion"); @@ -880,12 +926,8 @@ 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) { - GraphDiffT EmptyGD; - auto &GD = BUI ? BUI->PreViewCFG : EmptyGD; - for (auto &Pair : children({&GD, Of})) - if (Pair.second == SuccCandidate) - return true; - return false; + auto Successors = ChildrenGetter::Get(Of, BUI); + return llvm::find(Successors, SuccCandidate) != Successors.end(); }; (void)IsSuccessor; assert(!IsSuccessor(To, From) && "Deleted edge still exists in the CFG!"); @@ -971,17 +1013,15 @@ struct SemiNCAInfo { const TreeNodePtr TN) { LLVM_DEBUG(dbgs() << "IsReachableFromIDom " << BlockNamePrinter(TN) << "\n"); - auto TNB = TN->getBlock(); - GraphDiffT EmptyGD; - auto &GD = BUI ? BUI->PreViewCFG : EmptyGD; - for (auto &Pair : children({&GD, TNB})) { - const NodePtr Pred = Pair.second; + for (const NodePtr Pred : + ChildrenGetter::Get(TN->getBlock(), BUI)) { LLVM_DEBUG(dbgs() << "\tPred " << BlockNamePrinter(Pred) << "\n"); if (!DT.getNode(Pred)) continue; - const NodePtr Support = DT.findNearestCommonDominator(TNB, Pred); + const NodePtr Support = + DT.findNearestCommonDominator(TN->getBlock(), Pred); LLVM_DEBUG(dbgs() << "\tSupport " << BlockNamePrinter(Support) << "\n"); - if (Support != TNB) { + if (Support != TN->getBlock()) { LLVM_DEBUG(dbgs() << "\t" << BlockNamePrinter(TN) << " is reachable from support " << BlockNamePrinter(Support) << "\n"); @@ -1112,23 +1152,53 @@ struct SemiNCAInfo { //===--------------------- DomTree Batch Updater --------------------------=== //~~ - static void ApplyUpdates(DomTreeT &DT, GraphDiffT &PreViewCFG) { - const size_t NumUpdates = PreViewCFG.getNumLegalizedUpdates(); + static void ApplyUpdates(DomTreeT &DT, ArrayRef Updates) { + const size_t NumUpdates = Updates.size(); if (NumUpdates == 0) return; // Take the fast path for a single update and avoid running the batch update // machinery. if (NumUpdates == 1) { - UpdateT Update = PreViewCFG.popUpdateForIncrementalUpdates(); + const auto &Update = Updates.front(); if (Update.getKind() == UpdateKind::Insert) - InsertEdge(DT, /*BUI=*/nullptr, Update.getFrom(), Update.getTo()); + DT.insertEdge(Update.getFrom(), Update.getTo()); else - DeleteEdge(DT, /*BUI=*/nullptr, Update.getFrom(), Update.getTo()); + DT.deleteEdge(Update.getFrom(), Update.getTo()); + return; } - BatchUpdateInfo BUI(PreViewCFG); + BatchUpdateInfo BUI; + LLVM_DEBUG(dbgs() << "Legalizing " << BUI.Updates.size() << " updates\n"); + cfg::LegalizeUpdates(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 + // 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 @@ -1138,21 +1208,21 @@ struct SemiNCAInfo { // Make unittests of the incremental algorithm work if (DT.DomTreeNodes.size() <= 100) { - if (BUI.NumLegalized > DT.DomTreeNodes.size()) + if (NumLegalized > DT.DomTreeNodes.size()) CalculateFromScratch(DT, &BUI); - } else if (BUI.NumLegalized > DT.DomTreeNodes.size() / 40) + } else if (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 < BUI.NumLegalized && !BUI.IsRecalculated; ++i) + for (size_t i = 0; i < NumLegalized && !BUI.IsRecalculated; ++i) ApplyNextUpdate(DT, BUI); } static void ApplyNextUpdate(DomTreeT &DT, BatchUpdateInfo &BUI) { - // Popping the next update, will move the PreViewCFG to the next snapshot. - UpdateT CurrentUpdate = BUI.PreViewCFG.popUpdateForIncrementalUpdates(); + assert(!BUI.Updates.empty() && "No updates to apply!"); + UpdateT CurrentUpdate = BUI.Updates.pop_back_val(); #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 @@ -1161,6 +1231,21 @@ 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 @@ -1518,11 +1603,19 @@ void Calculate(DomTreeT &DT) { template void CalculateWithUpdates(DomTreeT &DT, ArrayRef Updates) { - // FIXME: Updated to use the PreViewCFG and behave the same as until now. - // This behavior is however incorrect; this actually needs the PostViewCFG. - GraphDiff PreViewCFG( - Updates, /*ReverseApplyUpdates=*/true); - typename SemiNCAInfo::BatchUpdateInfo BUI(PreViewCFG); + // TODO: Move BUI creation in common method, reuse in ApplyUpdates. + typename SemiNCAInfo::BatchUpdateInfo BUI; + LLVM_DEBUG(dbgs() << "Legalizing " << BUI.Updates.size() << " updates\n"); + cfg::LegalizeUpdates(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()}); + } + SemiNCAInfo::CalculateFromScratch(DT, &BUI); } @@ -1542,9 +1635,8 @@ void DeleteEdge(DomTreeT &DT, typename DomTreeT::NodePtr From, template void ApplyUpdates(DomTreeT &DT, - GraphDiff &PreViewCFG) { - SemiNCAInfo::ApplyUpdates(DT, PreViewCFG); + ArrayRef Updates) { + SemiNCAInfo::ApplyUpdates(DT, Updates); } template diff --git a/llvm/lib/IR/Dominators.cpp b/llvm/lib/IR/Dominators.cpp index a58380ff1f702f..4062ade645ba8c 100644 --- a/llvm/lib/IR/Dominators.cpp +++ b/llvm/lib/IR/Dominators.cpp @@ -90,10 +90,9 @@ template void llvm::DomTreeBuilder::DeleteEdge( DomTreeBuilder::BBPostDomTree &DT, BasicBlock *From, BasicBlock *To); template void llvm::DomTreeBuilder::ApplyUpdates( - DomTreeBuilder::BBDomTree &DT, DomTreeBuilder::BBDomTreeGraphDiff &); + DomTreeBuilder::BBDomTree &DT, DomTreeBuilder::BBUpdates); template void llvm::DomTreeBuilder::ApplyUpdates( - DomTreeBuilder::BBPostDomTree &DT, - DomTreeBuilder::BBPostDomTreeGraphDiff &); + DomTreeBuilder::BBPostDomTree &DT, DomTreeBuilder::BBUpdates); template bool llvm::DomTreeBuilder::Verify( const DomTreeBuilder::BBDomTree &DT,