[DomTreeUpdater] Split implementations #97027
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Move implementations of `GenericDomTreeUpdater` to GenericDomTreeUpdaterImpl.h
|
@llvm/pr-subscribers-llvm-analysis Author: None (paperchalice) ChangesMove implementations of Patch is 40.37 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/97027.diff 6 Files Affected:
diff --git a/llvm/include/llvm/Analysis/DomTreeUpdater.h b/llvm/include/llvm/Analysis/DomTreeUpdater.h
index 2b838a311440e..9ffd4948c1750 100644
--- a/llvm/include/llvm/Analysis/DomTreeUpdater.h
+++ b/llvm/include/llvm/Analysis/DomTreeUpdater.h
@@ -16,7 +16,6 @@
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/Analysis/GenericDomTreeUpdater.h"
-#include "llvm/Analysis/PostDominators.h"
#include "llvm/IR/Dominators.h"
#include "llvm/IR/ValueHandle.h"
#include "llvm/Support/Compiler.h"
@@ -26,6 +25,8 @@
namespace llvm {
+class PostDominatorTree;
+
class DomTreeUpdater
: public GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
PostDominatorTree> {
@@ -110,27 +111,15 @@ class DomTreeUpdater
bool forceFlushDeletedBB();
/// Debug method to help view the internal state of this class.
- LLVM_DUMP_METHOD void dump() const {
- Base::dump();
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
- raw_ostream &OS = dbgs();
- OS << "Pending Callbacks:\n";
- int Index = 0;
- for (const auto &BB : Callbacks) {
- OS << " " << Index << " : ";
- ++Index;
- if (BB->hasName())
- OS << BB->getName() << "(";
- else
- OS << "(no_name)(";
- OS << BB << ")\n";
- }
-#endif
- }
+ LLVM_DUMP_METHOD void dump() const;
};
extern template class GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
PostDominatorTree>;
+
+extern template void
+GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
+ PostDominatorTree>::recalculate(Function &F);
} // namespace llvm
#endif // LLVM_ANALYSIS_DOMTREEUPDATER_H
diff --git a/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h b/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h
index 7092c67083a67..1a86398e4fc68 100644
--- a/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h
+++ b/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h
@@ -16,91 +16,67 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallSet.h"
-#include "llvm/Support/Debug.h"
-#include "llvm/Support/raw_ostream.h"
+#include "llvm/Support/Compiler.h"
namespace llvm {
template <typename DerivedT, typename DomTreeT, typename PostDomTreeT>
class GenericDomTreeUpdater {
- DerivedT &derived() { return *static_cast<DerivedT *>(this); }
- const DerivedT &derived() const {
- return *static_cast<const DerivedT *>(this);
- }
+ DerivedT &derived();
+ const DerivedT &derived() const;
public:
enum class UpdateStrategy : unsigned char { Eager = 0, Lazy = 1 };
using BasicBlockT = typename DomTreeT::NodeType;
- explicit GenericDomTreeUpdater(UpdateStrategy Strategy_)
- : Strategy(Strategy_) {}
- GenericDomTreeUpdater(DomTreeT &DT_, UpdateStrategy Strategy_)
- : DT(&DT_), Strategy(Strategy_) {}
- GenericDomTreeUpdater(DomTreeT *DT_, UpdateStrategy Strategy_)
- : DT(DT_), Strategy(Strategy_) {}
- GenericDomTreeUpdater(PostDomTreeT &PDT_, UpdateStrategy Strategy_)
- : PDT(&PDT_), Strategy(Strategy_) {}
- GenericDomTreeUpdater(PostDomTreeT *PDT_, UpdateStrategy Strategy_)
- : PDT(PDT_), Strategy(Strategy_) {}
+ explicit GenericDomTreeUpdater(UpdateStrategy Strategy_);
+ GenericDomTreeUpdater(DomTreeT &DT_, UpdateStrategy Strategy_);
+ GenericDomTreeUpdater(DomTreeT *DT_, UpdateStrategy Strategy_);
+ GenericDomTreeUpdater(PostDomTreeT &PDT_, UpdateStrategy Strategy_);
+ GenericDomTreeUpdater(PostDomTreeT *PDT_, UpdateStrategy Strategy_);
GenericDomTreeUpdater(DomTreeT &DT_, PostDomTreeT &PDT_,
- UpdateStrategy Strategy_)
- : DT(&DT_), PDT(&PDT_), Strategy(Strategy_) {}
+ UpdateStrategy Strategy_);
GenericDomTreeUpdater(DomTreeT *DT_, PostDomTreeT *PDT_,
- UpdateStrategy Strategy_)
- : DT(DT_), PDT(PDT_), Strategy(Strategy_) {}
+ UpdateStrategy Strategy_);
- ~GenericDomTreeUpdater() { flush(); }
+ ~GenericDomTreeUpdater();
/// Returns true if the current strategy is Lazy.
- bool isLazy() const { return Strategy == UpdateStrategy::Lazy; };
+ bool isLazy() const;
/// Returns true if the current strategy is Eager.
- bool isEager() const { return Strategy == UpdateStrategy::Eager; };
+ bool isEager() const;
/// Returns true if it holds a DomTreeT.
- bool hasDomTree() const { return DT != nullptr; }
+ bool hasDomTree() const;
/// Returns true if it holds a PostDomTreeT.
- bool hasPostDomTree() const { return PDT != nullptr; }
+ bool hasPostDomTree() const;
/// Returns true if there is BasicBlockT awaiting deletion.
/// The deletion will only happen until a flush event and
/// all available trees are up-to-date.
/// Returns false under Eager UpdateStrategy.
- bool hasPendingDeletedBB() const { return !DeletedBBs.empty(); }
+ bool hasPendingDeletedBB() const;
/// Returns true if DelBB is awaiting deletion.
/// Returns false under Eager UpdateStrategy.
- bool isBBPendingDeletion(BasicBlockT *DelBB) const {
- if (Strategy == UpdateStrategy::Eager || DeletedBBs.empty())
- return false;
- return DeletedBBs.contains(DelBB);
- }
+ bool isBBPendingDeletion(BasicBlockT *DelBB) const;
/// Returns true if either of DT or PDT is valid and the tree has at
/// least one update pending. If DT or PDT is nullptr it is treated
/// as having no pending updates. This function does not check
/// whether there is MachineBasicBlock awaiting deletion.
/// Returns false under Eager UpdateStrategy.
- bool hasPendingUpdates() const {
- return hasPendingDomTreeUpdates() || hasPendingPostDomTreeUpdates();
- }
+ bool hasPendingUpdates() const;
/// Returns true if there are DomTreeT updates queued.
/// Returns false under Eager UpdateStrategy or DT is nullptr.
- bool hasPendingDomTreeUpdates() const {
- if (!DT)
- return false;
- return PendUpdates.size() != PendDTUpdateIndex;
- }
+ bool hasPendingDomTreeUpdates() const;
/// Returns true if there are PostDomTreeT updates queued.
/// Returns false under Eager UpdateStrategy or PDT is nullptr.
- bool hasPendingPostDomTreeUpdates() const {
- if (!PDT)
- return false;
- return PendUpdates.size() != PendPDTUpdateIndex;
- }
+ bool hasPendingPostDomTreeUpdates() const;
///@{
/// \name Mutation APIs
@@ -126,34 +102,7 @@ class GenericDomTreeUpdater {
/// Notify DTU that the entry block was replaced.
/// Recalculate all available trees and flush all BasicBlocks
/// awaiting deletion immediately.
- template <typename FuncT> void recalculate(FuncT &F) {
- if (Strategy == UpdateStrategy::Eager) {
- if (DT)
- DT->recalculate(F);
- if (PDT)
- PDT->recalculate(F);
- return;
- }
-
- // There is little performance gain if we pend the recalculation under
- // Lazy UpdateStrategy so we recalculate available trees immediately.
-
- // Prevent forceFlushDeletedBB() from erasing DomTree or PostDomTree nodes.
- IsRecalculatingDomTree = IsRecalculatingPostDomTree = true;
-
- // Because all trees are going to be up-to-date after recalculation,
- // flush awaiting deleted BasicBlocks.
- derived().forceFlushDeletedBB();
- if (DT)
- DT->recalculate(F);
- if (PDT)
- PDT->recalculate(F);
-
- // Resume forceFlushDeletedBB() to erase DomTree or PostDomTree nodes.
- IsRecalculatingDomTree = IsRecalculatingPostDomTree = false;
- PendDTUpdateIndex = PendPDTUpdateIndex = PendUpdates.size();
- dropOutOfDateUpdates();
- }
+ template <typename FuncT> void recalculate(FuncT &F);
/// Submit updates to all available trees.
/// The Eager Strategy flushes updates immediately while the Lazy Strategy
@@ -170,24 +119,7 @@ class GenericDomTreeUpdater {
/// 2. It is illegal to submit any update that has already been submitted,
/// i.e., you are supposed not to insert an existent edge or delete a
/// nonexistent edge.
- void applyUpdates(ArrayRef<typename DomTreeT::UpdateType> Updates) {
- if (!DT && !PDT)
- return;
-
- if (Strategy == UpdateStrategy::Lazy) {
- PendUpdates.reserve(PendUpdates.size() + Updates.size());
- for (const auto &U : Updates)
- if (!isSelfDominance(U))
- PendUpdates.push_back(U);
-
- return;
- }
-
- if (DT)
- DT->applyUpdates(Updates);
- if (PDT)
- PDT->applyUpdates(Updates);
- }
+ void applyUpdates(ArrayRef<typename DomTreeT::UpdateType> Updates);
/// Submit updates to all available trees. It will also
/// 1. discard duplicated updates,
@@ -210,58 +142,7 @@ class GenericDomTreeUpdater {
/// 3. It is only legal to submit updates to an edge in the order CFG changes
/// are made. The order you submit updates on different edges is not
/// restricted.
- void applyUpdatesPermissive(ArrayRef<typename DomTreeT::UpdateType> Updates) {
- if (!DT && !PDT)
- return;
-
- SmallSet<std::pair<BasicBlockT *, BasicBlockT *>, 8> Seen;
- SmallVector<typename DomTreeT::UpdateType, 8> DeduplicatedUpdates;
- for (const auto &U : Updates) {
- auto Edge = std::make_pair(U.getFrom(), U.getTo());
- // Because it is illegal to submit updates that have already been applied
- // and updates to an edge need to be strictly ordered,
- // it is safe to infer the existence of an edge from the first update
- // to this edge.
- // If the first update to an edge is "Delete", it means that the edge
- // existed before. If the first update to an edge is "Insert", it means
- // that the edge didn't exist before.
- //
- // For example, if the user submits {{Delete, A, B}, {Insert, A, B}},
- // because
- // 1. it is illegal to submit updates that have already been applied,
- // i.e., user cannot delete an nonexistent edge,
- // 2. updates to an edge need to be strictly ordered,
- // So, initially edge A -> B existed.
- // We can then safely ignore future updates to this edge and directly
- // inspect the current CFG:
- // a. If the edge still exists, because the user cannot insert an existent
- // edge, so both {Delete, A, B}, {Insert, A, B} actually happened and
- // resulted in a no-op. DTU won't submit any update in this case.
- // b. If the edge doesn't exist, we can then infer that {Delete, A, B}
- // actually happened but {Insert, A, B} was an invalid update which never
- // happened. DTU will submit {Delete, A, B} in this case.
- if (!isSelfDominance(U) && Seen.count(Edge) == 0) {
- Seen.insert(Edge);
- // If the update doesn't appear in the CFG, it means that
- // either the change isn't made or relevant operations
- // result in a no-op.
- if (isUpdateValid(U)) {
- if (isLazy())
- PendUpdates.push_back(U);
- else
- DeduplicatedUpdates.push_back(U);
- }
- }
- }
-
- if (Strategy == UpdateStrategy::Lazy)
- return;
-
- if (DT)
- DT->applyUpdates(DeduplicatedUpdates);
- if (PDT)
- PDT->applyUpdates(DeduplicatedUpdates);
- }
+ void applyUpdatesPermissive(ArrayRef<typename DomTreeT::UpdateType> Updates);
///@}
@@ -275,127 +156,22 @@ class GenericDomTreeUpdater {
/// Flush DomTree updates and return DomTree.
/// It flushes Deleted BBs if both trees are up-to-date.
/// It must only be called when it has a DomTree.
- DomTreeT &getDomTree() {
- assert(DT && "Invalid acquisition of a null DomTree");
- applyDomTreeUpdates();
- dropOutOfDateUpdates();
- return *DT;
- }
+ DomTreeT &getDomTree();
/// Flush PostDomTree updates and return PostDomTree.
/// It flushes Deleted BBs if both trees are up-to-date.
/// It must only be called when it has a PostDomTree.
- PostDomTreeT &getPostDomTree() {
- assert(PDT && "Invalid acquisition of a null PostDomTree");
- applyPostDomTreeUpdates();
- dropOutOfDateUpdates();
- return *PDT;
- }
+ PostDomTreeT &getPostDomTree();
/// Apply all pending updates to available trees and flush all BasicBlocks
/// awaiting deletion.
- void flush() {
- applyDomTreeUpdates();
- applyPostDomTreeUpdates();
- dropOutOfDateUpdates();
- }
+ void flush();
///@}
/// Debug method to help view the internal state of this class.
- LLVM_DUMP_METHOD void dump() const {
-#if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
- raw_ostream &OS = llvm::dbgs();
-
- OS << "Available Trees: ";
- if (DT || PDT) {
- if (DT)
- OS << "DomTree ";
- if (PDT)
- OS << "PostDomTree ";
- OS << "\n";
- } else
- OS << "None\n";
-
- OS << "UpdateStrategy: ";
- if (Strategy == UpdateStrategy::Eager) {
- OS << "Eager\n";
- return;
- } else
- OS << "Lazy\n";
- int Index = 0;
-
- auto printUpdates =
- [&](typename ArrayRef<typename DomTreeT::UpdateType>::const_iterator
- begin,
- typename ArrayRef<typename DomTreeT::UpdateType>::const_iterator
- end) {
- if (begin == end)
- OS << " None\n";
- Index = 0;
- for (auto It = begin, ItEnd = end; It != ItEnd; ++It) {
- auto U = *It;
- OS << " " << Index << " : ";
- ++Index;
- if (U.getKind() == DomTreeT::Insert)
- OS << "Insert, ";
- else
- OS << "Delete, ";
- BasicBlockT *From = U.getFrom();
- if (From) {
- auto S = From->getName();
- if (!From->hasName())
- S = "(no name)";
- OS << S << "(" << From << "), ";
- } else {
- OS << "(badref), ";
- }
- BasicBlockT *To = U.getTo();
- if (To) {
- auto S = To->getName();
- if (!To->hasName())
- S = "(no_name)";
- OS << S << "(" << To << ")\n";
- } else {
- OS << "(badref)\n";
- }
- }
- };
-
- if (DT) {
- const auto I = PendUpdates.begin() + PendDTUpdateIndex;
- assert(PendUpdates.begin() <= I && I <= PendUpdates.end() &&
- "Iterator out of range.");
- OS << "Applied but not cleared DomTreeUpdates:\n";
- printUpdates(PendUpdates.begin(), I);
- OS << "Pending DomTreeUpdates:\n";
- printUpdates(I, PendUpdates.end());
- }
-
- if (PDT) {
- const auto I = PendUpdates.begin() + PendPDTUpdateIndex;
- assert(PendUpdates.begin() <= I && I <= PendUpdates.end() &&
- "Iterator out of range.");
- OS << "Applied but not cleared PostDomTreeUpdates:\n";
- printUpdates(PendUpdates.begin(), I);
- OS << "Pending PostDomTreeUpdates:\n";
- printUpdates(I, PendUpdates.end());
- }
-
- OS << "Pending DeletedBBs:\n";
- Index = 0;
- for (const auto *BB : DeletedBBs) {
- OS << " " << Index << " : ";
- ++Index;
- if (BB->hasName())
- OS << BB->getName() << "(";
- else
- OS << "(no_name)(";
- OS << BB << ")\n";
- }
-#endif
- }
+ LLVM_DUMP_METHOD void dump() const;
protected:
SmallVector<typename DomTreeT::UpdateType, 16> PendUpdates;
@@ -409,115 +185,30 @@ class GenericDomTreeUpdater {
bool IsRecalculatingPostDomTree = false;
/// Returns true if the update is self dominance.
- bool isSelfDominance(typename DomTreeT::UpdateType Update) const {
- // Won't affect DomTree and PostDomTree.
- return Update.getFrom() == Update.getTo();
- }
+ bool isSelfDominance(typename DomTreeT::UpdateType Update) const;
/// Helper function to apply all pending DomTree updates.
- void applyDomTreeUpdates() {
- // No pending DomTreeUpdates.
- if (Strategy != UpdateStrategy::Lazy || !DT)
- return;
-
- // Only apply updates not are applied by DomTree.
- if (hasPendingDomTreeUpdates()) {
- const auto I = PendUpdates.begin() + PendDTUpdateIndex;
- const auto E = PendUpdates.end();
- assert(I < E &&
- "Iterator range invalid; there should be DomTree updates.");
- DT->applyUpdates(ArrayRef<typename DomTreeT::UpdateType>(I, E));
- PendDTUpdateIndex = PendUpdates.size();
- }
- }
+ void applyDomTreeUpdates();
/// Helper function to apply all pending PostDomTree updates.
- void applyPostDomTreeUpdates() {
- // No pending PostDomTreeUpdates.
- if (Strategy != UpdateStrategy::Lazy || !PDT)
- return;
-
- // Only apply updates not are applied by PostDomTree.
- if (hasPendingPostDomTreeUpdates()) {
- const auto I = PendUpdates.begin() + PendPDTUpdateIndex;
- const auto E = PendUpdates.end();
- assert(I < E &&
- "Iterator range invalid; there should be PostDomTree updates.");
- PDT->applyUpdates(ArrayRef<typename DomTreeT::UpdateType>(I, E));
- PendPDTUpdateIndex = PendUpdates.size();
- }
- }
+ void applyPostDomTreeUpdates();
/// Returns true if the update appears in the LLVM IR.
/// It is used to check whether an update is valid in
/// insertEdge/deleteEdge or is unnecessary in the batch update.
- bool isUpdateValid(typename DomTreeT::UpdateType Update) const {
- const auto *From = Update.getFrom();
- const auto *To = Update.getTo();
- const auto Kind = Update.getKind();
-
- // Discard updates by inspecting the current state of successors of From.
- // Since isUpdateValid() must be called *after* the Terminator of From is
- // altered we can determine if the update is unnecessary for batch updates
- // or invalid for a single update.
- const bool HasEdge = llvm::is_contained(successors(From), To);
-
- // If the IR does not match the update,
- // 1. In batch updates, this update is unnecessary.
- // 2. When called by insertEdge*()/deleteEdge*(), this update is invalid.
- // Edge does not exist in IR.
- if (Kind == DomTreeT::Insert && !HasEdge)
- return false;
-
- // Edge exists in IR.
- if (Kind == DomTreeT::Delete && HasEdge)
- return false;
-
- return true;
- }
+ bool isUpdateValid(typename DomTreeT::UpdateType Update) const;
/// Erase Basic Block node that has been unlinked from Function
/// in the DomTree and PostDomTree.
- void eraseDelBBNode(BasicBlockT *DelBB) {
- if (DT && !IsRecalculatingDomTree)
- if (DT->getNode(DelBB))
- DT->eraseNode(DelBB);
-
- if (PDT && !IsRecalculatingPostDomTree)
- if (PDT->getNode(DelBB))
- PDT->eraseNode(DelBB);
- }
+ void eraseDelBBNode(BasicBlockT *DelBB);
/// Helper function to flush deleted BasicBlocks if all available
/// trees are up-to-date.
- void tryFlushDeletedBB() {
- if (!hasPendingUpdates())
- derived().forceFlushDeletedBB();
- }
+ void tryFlushDeletedBB();
/// Drop all updates applied by all available trees and delete BasicBlocks if
/// all available trees are up-to-date.
- void dropOutOfDateUpdates() {
- if (Strategy == UpdateStrategy::Eager)
- return;
-
- tryFlushDeletedBB();
-
- // Drop all updates applied by both trees.
- if (!DT)
- PendDTUpdateIndex = PendUpdates.size();
- if (!PDT)
- PendPDTUpdateIndex = PendUpdates.size();
-
- const size_t dropIndex = std::min(PendDTUpdateIndex, PendPDTUpdateIndex);
- const auto B = PendUpdates.begin();
- const auto E = PendUpdates.begin() + dropIndex;
- assert(B <= E && "Iterator out of range.");
- PendUpdates.erase(B, E);
- // Calculate current index.
- PendDTUpdateIndex -= dropIndex;
- PendPDTUpdateIndex -= dropIndex;
- }
+ void dropOutOfDateUpdates();
};
} // namespace llvm
diff --git a/llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h b/llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h
new file mode 100644
index 0000000000000..ec6c345b994ac
--- /dev/null
+++ b/llvm/include/llvm/Analysis/GenericDomTreeUpdaterImpl.h
@@ -0,0 +1,477 @@
+//===- GenericDomTreeUpdaterImpl.h ------------------------------*- 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
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the GenericDomTreeUpdater class. This file should only
+// be i...
[truncated]
|
nikic
reviewed
Jun 28, 2024
| bool GenericDomTreeUpdater<DerivedT, DomTreeT, PostDomTreeT>::hasPostDomTree() | ||
| const { | ||
| return DT != nullptr; | ||
| } |
There was a problem hiding this comment.
It's fine (and preferable) to keep the definitions for all these tiny single-line methods in the header, assuming they work fine with forward-declarations, which I think they do. Basically the ones that were in the header prior to #96851 can also stay in the header now.
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/123/builds/815 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/7/builds/672 Here is the relevant piece of the build log for the reference: |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/139/builds/565 Here is the relevant piece of the build log for the reference: |
lravenclaw
pushed a commit
to lravenclaw/llvm-project
that referenced
this pull request
Jul 3, 2024
Move implementations of `GenericDomTreeUpdater` to GenericDomTreeUpdaterImpl.h
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Move implementations of
GenericDomTreeUpdaterto GenericDomTreeUpdaterImpl.h