[llvm] [Support][ADT] Minor cleanups after #101706 (PR #102180)
via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 6 09:54:42 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-adt
Author: Alexis Engelke (aengelke)
<details>
<summary>Changes</summary>
Fix GraphHasNodeNumbers indicator for graphs where NodeRef is not the graph type (e.g., Inverse<...>).
Add static_assert tests to MachineBasicBlock GraphTraits.
Use GraphHasNodeNumbers in DomTreeBase.
Don't include ad-hoc numbering map for numbered graphs in DomTreeBase.
---
Full diff: https://github.com/llvm/llvm-project/pull/102180.diff
3 Files Affected:
- (modified) llvm/include/llvm/ADT/GraphTraits.h (+2-1)
- (modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (+12)
- (modified) llvm/include/llvm/Support/GenericDomTree.h (+14-14)
``````````diff
diff --git a/llvm/include/llvm/ADT/GraphTraits.h b/llvm/include/llvm/ADT/GraphTraits.h
index 0764ecb4bb568..20bb27f50e17f 100644
--- a/llvm/include/llvm/ADT/GraphTraits.h
+++ b/llvm/include/llvm/ADT/GraphTraits.h
@@ -97,7 +97,8 @@ struct GraphTraits {
namespace detail {
template <typename T>
-using has_number_t = decltype(GraphTraits<T>::getNumber(std::declval<T>()));
+using has_number_t = decltype(GraphTraits<T>::getNumber(
+ std::declval<typename GraphTraits<T>::NodeRef>()));
} // namespace detail
/// Indicate whether a GraphTraits<NodeT>::getNumber() is supported.
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index 797e29d071dd9..2d238326ee1a3 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -1304,6 +1304,9 @@ template <> struct GraphTraits<MachineBasicBlock *> {
}
};
+static_assert(GraphHasNodeNumbers<MachineBasicBlock *>,
+ "GraphTraits getNumber() not detected");
+
template <> struct GraphTraits<const MachineBasicBlock *> {
using NodeRef = const MachineBasicBlock *;
using ChildIteratorType = MachineBasicBlock::const_succ_iterator;
@@ -1318,6 +1321,9 @@ template <> struct GraphTraits<const MachineBasicBlock *> {
}
};
+static_assert(GraphHasNodeNumbers<const MachineBasicBlock *>,
+ "GraphTraits getNumber() not detected");
+
// Provide specializations of GraphTraits to be able to treat a
// MachineFunction as a graph of MachineBasicBlocks and to walk it
// in inverse order. Inverse order for a function is considered
@@ -1341,6 +1347,9 @@ template <> struct GraphTraits<Inverse<MachineBasicBlock*>> {
}
};
+static_assert(GraphHasNodeNumbers<Inverse<MachineBasicBlock *>>,
+ "GraphTraits getNumber() not detected");
+
template <> struct GraphTraits<Inverse<const MachineBasicBlock*>> {
using NodeRef = const MachineBasicBlock *;
using ChildIteratorType = MachineBasicBlock::const_pred_iterator;
@@ -1358,6 +1367,9 @@ template <> struct GraphTraits<Inverse<const MachineBasicBlock*>> {
}
};
+static_assert(GraphHasNodeNumbers<Inverse<const MachineBasicBlock *>>,
+ "GraphTraits getNumber() not detected");
+
// These accessors are handy for sharing templated code between IR and MIR.
inline auto successors(const MachineBasicBlock *BB) { return BB->successors(); }
inline auto predecessors(const MachineBasicBlock *BB) {
diff --git a/llvm/include/llvm/Support/GenericDomTree.h b/llvm/include/llvm/Support/GenericDomTree.h
index d7b94d50e6311..38f62e86ae652 100644
--- a/llvm/include/llvm/Support/GenericDomTree.h
+++ b/llvm/include/llvm/Support/GenericDomTree.h
@@ -262,7 +262,9 @@ class DominatorTreeBase {
SmallVector<std::unique_ptr<DomTreeNodeBase<NodeT>>>;
DomTreeNodeStorageTy DomTreeNodes;
// For graphs where blocks don't have numbers, create a numbering here.
- DenseMap<const NodeT *, unsigned> NodeNumberMap;
+ std::conditional_t<!GraphHasNodeNumbers<NodeT *>,
+ DenseMap<const NodeT *, unsigned>, std::tuple<>>
+ NodeNumberMap;
DomTreeNodeBase<NodeT> *RootNode = nullptr;
ParentPtr Parent = nullptr;
@@ -355,12 +357,8 @@ class DominatorTreeBase {
}
private:
- template <typename T>
- using has_number_t =
- decltype(GraphTraits<T *>::getNumber(std::declval<T *>()));
-
std::optional<unsigned> getNodeIndex(const NodeT *BB) const {
- if constexpr (is_detected<has_number_t, NodeT>::value) {
+ if constexpr (GraphHasNodeNumbers<NodeT *>) {
// BB can be nullptr, map nullptr to index 0.
assert(BlockNumberEpoch ==
GraphTraits<ParentPtr>::getNumberEpoch(Parent) &&
@@ -374,7 +372,7 @@ class DominatorTreeBase {
}
unsigned getNodeIndexForInsert(const NodeT *BB) {
- if constexpr (is_detected<has_number_t, NodeT>::value) {
+ if constexpr (GraphHasNodeNumbers<NodeT *>) {
// getNodeIndex will never fail if nodes have getNumber().
unsigned Idx = *getNodeIndex(BB);
if (Idx >= DomTreeNodes.size()) {
@@ -736,7 +734,8 @@ class DominatorTreeBase {
}
DomTreeNodes[*IdxOpt] = nullptr;
- NodeNumberMap.erase(BB);
+ if constexpr (!GraphHasNodeNumbers<NodeT *>)
+ NodeNumberMap.erase(BB);
if (!IsPostDom) return;
@@ -830,7 +829,7 @@ class DominatorTreeBase {
private:
void updateBlockNumberEpoch() {
// Nothing to do for graphs that don't number their blocks.
- if constexpr (is_detected<has_number_t, NodeT>::value)
+ if constexpr (GraphHasNodeNumbers<NodeT *>)
BlockNumberEpoch = GraphTraits<ParentPtr>::getNumberEpoch(Parent);
}
@@ -849,9 +848,8 @@ class DominatorTreeBase {
}
/// Update dominator tree after renumbering blocks.
- template <class T_ = NodeT>
- std::enable_if_t<is_detected<has_number_t, T_>::value, void>
- updateBlockNumbers() {
+ template <typename T = NodeT>
+ std::enable_if_t<GraphHasNodeNumbers<T *>, void> updateBlockNumbers() {
updateBlockNumberEpoch();
unsigned MaxNumber = GraphTraits<ParentPtr>::getMaxNumber(Parent);
@@ -889,7 +887,8 @@ class DominatorTreeBase {
void reset() {
DomTreeNodes.clear();
- NodeNumberMap.clear();
+ if constexpr (!GraphHasNodeNumbers<NodeT *>)
+ NodeNumberMap.clear();
Roots.clear();
RootNode = nullptr;
Parent = nullptr;
@@ -989,7 +988,8 @@ class DominatorTreeBase {
/// assignable and destroyable state, but otherwise invalid.
void wipe() {
DomTreeNodes.clear();
- NodeNumberMap.clear();
+ if constexpr (!GraphHasNodeNumbers<NodeT *>)
+ NodeNumberMap.clear();
RootNode = nullptr;
Parent = nullptr;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/102180
More information about the llvm-commits
mailing list