[llvm] Revert "[CodeGen] Introduce `MachineDomTreeUpdater`" (PR #96846)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 26 21:31:42 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-analysis
Author: None (paperchalice)
<details>
<summary>Changes</summary>
Reverts llvm/llvm-project#<!-- -->95369
Many build bots failed
---
Patch is 55.98 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/96846.diff
11 Files Affected:
- (modified) llvm/include/llvm/Analysis/DomTreeUpdater.h (+164-32)
- (removed) llvm/include/llvm/Analysis/GenericDomTreeUpdater.h (-525)
- (modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (-3)
- (removed) llvm/include/llvm/CodeGen/MachineDomTreeUpdater.h (-72)
- (modified) llvm/include/llvm/CodeGen/MachinePostDominators.h (-1)
- (modified) llvm/lib/Analysis/DomTreeUpdater.cpp (+346-2)
- (modified) llvm/lib/CodeGen/CMakeLists.txt (-1)
- (modified) llvm/lib/CodeGen/MachineBasicBlock.cpp (-6)
- (removed) llvm/lib/CodeGen/MachineDomTreeUpdater.cpp (-62)
- (modified) llvm/unittests/CodeGen/CMakeLists.txt (-1)
- (removed) llvm/unittests/CodeGen/MachineDomTreeUpdaterTest.cpp (-276)
``````````diff
diff --git a/llvm/include/llvm/Analysis/DomTreeUpdater.h b/llvm/include/llvm/Analysis/DomTreeUpdater.h
index 2b838a311440e..ddb958455ccd7 100644
--- a/llvm/include/llvm/Analysis/DomTreeUpdater.h
+++ b/llvm/include/llvm/Analysis/DomTreeUpdater.h
@@ -15,8 +15,6 @@
#define LLVM_ANALYSIS_DOMTREEUPDATER_H
#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"
@@ -25,17 +23,66 @@
#include <vector>
namespace llvm {
+class PostDominatorTree;
-class DomTreeUpdater
- : public GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
- PostDominatorTree> {
- friend GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
- PostDominatorTree>;
-
+class DomTreeUpdater {
public:
- using Base =
- GenericDomTreeUpdater<DomTreeUpdater, DominatorTree, PostDominatorTree>;
- using Base::Base;
+ enum class UpdateStrategy : unsigned char { Eager = 0, Lazy = 1 };
+
+ explicit DomTreeUpdater(UpdateStrategy Strategy_) : Strategy(Strategy_) {}
+ DomTreeUpdater(DominatorTree &DT_, UpdateStrategy Strategy_)
+ : DT(&DT_), Strategy(Strategy_) {}
+ DomTreeUpdater(DominatorTree *DT_, UpdateStrategy Strategy_)
+ : DT(DT_), Strategy(Strategy_) {}
+ DomTreeUpdater(PostDominatorTree &PDT_, UpdateStrategy Strategy_)
+ : PDT(&PDT_), Strategy(Strategy_) {}
+ DomTreeUpdater(PostDominatorTree *PDT_, UpdateStrategy Strategy_)
+ : PDT(PDT_), Strategy(Strategy_) {}
+ DomTreeUpdater(DominatorTree &DT_, PostDominatorTree &PDT_,
+ UpdateStrategy Strategy_)
+ : DT(&DT_), PDT(&PDT_), Strategy(Strategy_) {}
+ DomTreeUpdater(DominatorTree *DT_, PostDominatorTree *PDT_,
+ UpdateStrategy Strategy_)
+ : DT(DT_), PDT(PDT_), Strategy(Strategy_) {}
+
+ ~DomTreeUpdater() { flush(); }
+
+ /// Returns true if the current strategy is Lazy.
+ bool isLazy() const { return Strategy == UpdateStrategy::Lazy; };
+
+ /// Returns true if the current strategy is Eager.
+ bool isEager() const { return Strategy == UpdateStrategy::Eager; };
+
+ /// Returns true if it holds a DominatorTree.
+ bool hasDomTree() const { return DT != nullptr; }
+
+ /// Returns true if it holds a PostDominatorTree.
+ bool hasPostDomTree() const { return PDT != nullptr; }
+
+ /// Returns true if there is BasicBlock 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(); }
+
+ /// Returns true if DelBB is awaiting deletion.
+ /// Returns false under Eager UpdateStrategy.
+ bool isBBPendingDeletion(BasicBlock *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 BasicBlock awaiting deletion.
+ /// Returns false under Eager UpdateStrategy.
+ bool hasPendingUpdates() const;
+
+ /// Returns true if there are DominatorTree updates queued.
+ /// Returns false under Eager UpdateStrategy or DT is nullptr.
+ bool hasPendingDomTreeUpdates() const;
+
+ /// Returns true if there are PostDominatorTree updates queued.
+ /// Returns false under Eager UpdateStrategy or PDT is nullptr.
+ bool hasPendingPostDomTreeUpdates() const;
///@{
/// \name Mutation APIs
@@ -58,6 +105,51 @@ class DomTreeUpdater
/// Although GenericDomTree provides several update primitives,
/// it is not encouraged to use these APIs directly.
+ /// Submit updates to all available trees.
+ /// The Eager Strategy flushes updates immediately while the Lazy Strategy
+ /// queues the updates.
+ ///
+ /// Note: The "existence" of an edge in a CFG refers to the CFG which DTU is
+ /// in sync with + all updates before that single update.
+ ///
+ /// CAUTION!
+ /// 1. It is required for the state of the LLVM IR to be updated
+ /// *before* submitting the updates because the internal update routine will
+ /// analyze the current state of the CFG to determine whether an update
+ /// is valid.
+ /// 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<DominatorTree::UpdateType> Updates);
+
+ /// Submit updates to all available trees. It will also
+ /// 1. discard duplicated updates,
+ /// 2. remove invalid updates. (Invalid updates means deletion of an edge that
+ /// still exists or insertion of an edge that does not exist.)
+ /// The Eager Strategy flushes updates immediately while the Lazy Strategy
+ /// queues the updates.
+ ///
+ /// Note: The "existence" of an edge in a CFG refers to the CFG which DTU is
+ /// in sync with + all updates before that single update.
+ ///
+ /// CAUTION!
+ /// 1. It is required for the state of the LLVM IR to be updated
+ /// *before* submitting the updates because the internal update routine will
+ /// analyze the current state of the CFG to determine whether an update
+ /// is valid.
+ /// 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.
+ /// 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<DominatorTree::UpdateType> Updates);
+
+ /// Notify DTU that the entry block was replaced.
+ /// Recalculate all available trees and flush all BasicBlocks
+ /// awaiting deletion immediately.
+ void recalculate(Function &F);
+
/// Delete DelBB. DelBB will be removed from its Parent and
/// erased from available trees if it exists and finally get deleted.
/// Under Eager UpdateStrategy, DelBB will be processed immediately.
@@ -80,6 +172,33 @@ class DomTreeUpdater
///@}
+ ///@{
+ /// \name Flush APIs
+ ///
+ /// CAUTION! By the moment these flush APIs are called, the current CFG needs
+ /// to be the same as the CFG which DTU is in sync with + all updates
+ /// submitted.
+
+ /// 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.
+ DominatorTree &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.
+ PostDominatorTree &getPostDomTree();
+
+ /// Apply all pending updates to available trees and flush all BasicBlocks
+ /// awaiting deletion.
+
+ void flush();
+
+ ///@}
+
+ /// Debug method to help view the internal state of this class.
+ LLVM_DUMP_METHOD void dump() const;
+
private:
class CallBackOnDeletion final : public CallbackVH {
public:
@@ -97,7 +216,16 @@ class DomTreeUpdater
}
};
+ SmallVector<DominatorTree::UpdateType, 16> PendUpdates;
+ size_t PendDTUpdateIndex = 0;
+ size_t PendPDTUpdateIndex = 0;
+ DominatorTree *DT = nullptr;
+ PostDominatorTree *PDT = nullptr;
+ const UpdateStrategy Strategy;
+ SmallPtrSet<BasicBlock *, 8> DeletedBBs;
std::vector<CallBackOnDeletion> Callbacks;
+ bool IsRecalculatingDomTree = false;
+ bool IsRecalculatingPostDomTree = false;
/// First remove all the instructions of DelBB and then make sure DelBB has a
/// valid terminator instruction which is necessary to have when DelBB still
@@ -109,28 +237,32 @@ class DomTreeUpdater
/// Returns true if at least one BasicBlock is deleted.
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
- }
-};
+ /// Helper function to apply all pending DomTree updates.
+ void applyDomTreeUpdates();
+
+ /// Helper function to apply all pending PostDomTree updates.
+ void applyPostDomTreeUpdates();
+
+ /// Helper function to flush deleted BasicBlocks if all available
+ /// trees are up-to-date.
+ void tryFlushDeletedBB();
-extern template class GenericDomTreeUpdater<DomTreeUpdater, DominatorTree,
- PostDominatorTree>;
+ /// Drop all updates applied by all available trees and delete BasicBlocks if
+ /// all available trees are up-to-date.
+ void dropOutOfDateUpdates();
+
+ /// Erase Basic Block node that has been unlinked from Function
+ /// in the DomTree and PostDomTree.
+ void eraseDelBBNode(BasicBlock *DelBB);
+
+ /// 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(DominatorTree::UpdateType Update) const;
+
+ /// Returns true if the update is self dominance.
+ bool isSelfDominance(DominatorTree::UpdateType Update) const;
+};
} // namespace llvm
#endif // LLVM_ANALYSIS_DOMTREEUPDATER_H
diff --git a/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h b/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h
deleted file mode 100644
index 7092c67083a67..0000000000000
--- a/llvm/include/llvm/Analysis/GenericDomTreeUpdater.h
+++ /dev/null
@@ -1,525 +0,0 @@
-//===- GenericDomTreeUpdater.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 defines the GenericDomTreeUpdater class, which provides a uniform
-// way to update dominator tree related data structures.
-//
-//===----------------------------------------------------------------------===//
-
-#ifndef LLVM_ANALYSIS_GENERICDOMTREEUPDATER_H
-#define LLVM_ANALYSIS_GENERICDOMTREEUPDATER_H
-
-#include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/SmallSet.h"
-#include "llvm/Support/Debug.h"
-#include "llvm/Support/raw_ostream.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);
- }
-
-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_) {}
- GenericDomTreeUpdater(DomTreeT &DT_, PostDomTreeT &PDT_,
- UpdateStrategy Strategy_)
- : DT(&DT_), PDT(&PDT_), Strategy(Strategy_) {}
- GenericDomTreeUpdater(DomTreeT *DT_, PostDomTreeT *PDT_,
- UpdateStrategy Strategy_)
- : DT(DT_), PDT(PDT_), Strategy(Strategy_) {}
-
- ~GenericDomTreeUpdater() { flush(); }
-
- /// Returns true if the current strategy is Lazy.
- bool isLazy() const { return Strategy == UpdateStrategy::Lazy; };
-
- /// Returns true if the current strategy is Eager.
- bool isEager() const { return Strategy == UpdateStrategy::Eager; };
-
- /// Returns true if it holds a DomTreeT.
- bool hasDomTree() const { return DT != nullptr; }
-
- /// Returns true if it holds a PostDomTreeT.
- bool hasPostDomTree() const { return PDT != nullptr; }
-
- /// 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(); }
-
- /// 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);
- }
-
- /// 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();
- }
-
- /// 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;
- }
-
- /// 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;
- }
-
- ///@{
- /// \name Mutation APIs
- ///
- /// These methods provide APIs for submitting updates to the DomTreeT and
- /// the PostDominatorTree.
- ///
- /// Note: There are two strategies to update the DomTreeT and the
- /// PostDominatorTree:
- /// 1. Eager UpdateStrategy: Updates are submitted and then flushed
- /// immediately.
- /// 2. Lazy UpdateStrategy: Updates are submitted but only flushed when you
- /// explicitly call Flush APIs. It is recommended to use this update strategy
- /// when you submit a bunch of updates multiple times which can then
- /// add up to a large number of updates between two queries on the
- /// DomTreeT. The incremental updater can reschedule the updates or
- /// decide to recalculate the dominator tree in order to speedup the updating
- /// process depending on the number of updates.
- ///
- /// Although GenericDomTree provides several update primitives,
- /// it is not encouraged to use these APIs directly.
-
- /// 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();
- }
-
- /// Submit updates to all available trees.
- /// The Eager Strategy flushes updates immediately while the Lazy Strategy
- /// queues the updates.
- ///
- /// Note: The "existence" of an edge in a CFG refers to the CFG which DTU is
- /// in sync with + all updates before that single update.
- ///
- /// CAUTION!
- /// 1. It is required for the state of the LLVM IR to be updated
- /// *before* submitting the updates because the internal update routine will
- /// analyze the current state of the CFG to determine whether an update
- /// is valid.
- /// 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);
- }
-
- /// Submit updates to all available trees. It will also
- /// 1. discard duplicated updates,
- /// 2. remove invalid updates. (Invalid updates means deletion of an edge that
- /// still exists or insertion of an edge that does not exist.)
- /// The Eager Strategy flushes updates immediately while the Lazy Strategy
- /// queues the updates.
- ///
- /// Note: The "existence" of an edge in a CFG refers to the CFG which DTU is
- /// in sync with + all updates before that single update.
- ///
- /// CAUTION!
- /// 1. It is required for the state of the LLVM IR to be updated
- /// *before* submitting the updates because the internal update routine will
- /// analyze the current state of the CFG to determine whether an update
- /// is valid.
- /// 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.
- /// 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
- ...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/96846
More information about the llvm-commits
mailing list