Skip to content

Commit fbb0619

Browse files
authored
[Support][ADT] Minor cleanups after #101706 (#102180)
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.
1 parent 135fecd commit fbb0619

File tree

3 files changed

+29
-15
lines changed

3 files changed

+29
-15
lines changed

llvm/include/llvm/ADT/GraphTraits.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ struct GraphTraits {
9797

9898
namespace detail {
9999
template <typename T>
100-
using has_number_t = decltype(GraphTraits<T>::getNumber(std::declval<T>()));
100+
using has_number_t = decltype(GraphTraits<T>::getNumber(
101+
std::declval<typename GraphTraits<T>::NodeRef>()));
101102
} // namespace detail
102103

103104
/// Indicate whether a GraphTraits<NodeT>::getNumber() is supported.

llvm/include/llvm/CodeGen/MachineBasicBlock.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1304,6 +1304,9 @@ template <> struct GraphTraits<MachineBasicBlock *> {
13041304
}
13051305
};
13061306

1307+
static_assert(GraphHasNodeNumbers<MachineBasicBlock *>,
1308+
"GraphTraits getNumber() not detected");
1309+
13071310
template <> struct GraphTraits<const MachineBasicBlock *> {
13081311
using NodeRef = const MachineBasicBlock *;
13091312
using ChildIteratorType = MachineBasicBlock::const_succ_iterator;
@@ -1318,6 +1321,9 @@ template <> struct GraphTraits<const MachineBasicBlock *> {
13181321
}
13191322
};
13201323

1324+
static_assert(GraphHasNodeNumbers<const MachineBasicBlock *>,
1325+
"GraphTraits getNumber() not detected");
1326+
13211327
// Provide specializations of GraphTraits to be able to treat a
13221328
// MachineFunction as a graph of MachineBasicBlocks and to walk it
13231329
// in inverse order. Inverse order for a function is considered
@@ -1341,6 +1347,9 @@ template <> struct GraphTraits<Inverse<MachineBasicBlock*>> {
13411347
}
13421348
};
13431349

1350+
static_assert(GraphHasNodeNumbers<Inverse<MachineBasicBlock *>>,
1351+
"GraphTraits getNumber() not detected");
1352+
13441353
template <> struct GraphTraits<Inverse<const MachineBasicBlock*>> {
13451354
using NodeRef = const MachineBasicBlock *;
13461355
using ChildIteratorType = MachineBasicBlock::const_pred_iterator;
@@ -1358,6 +1367,9 @@ template <> struct GraphTraits<Inverse<const MachineBasicBlock*>> {
13581367
}
13591368
};
13601369

1370+
static_assert(GraphHasNodeNumbers<Inverse<const MachineBasicBlock *>>,
1371+
"GraphTraits getNumber() not detected");
1372+
13611373
// These accessors are handy for sharing templated code between IR and MIR.
13621374
inline auto successors(const MachineBasicBlock *BB) { return BB->successors(); }
13631375
inline auto predecessors(const MachineBasicBlock *BB) {

llvm/include/llvm/Support/GenericDomTree.h

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,10 @@ class DominatorTreeBase {
262262
SmallVector<std::unique_ptr<DomTreeNodeBase<NodeT>>>;
263263
DomTreeNodeStorageTy DomTreeNodes;
264264
// For graphs where blocks don't have numbers, create a numbering here.
265-
DenseMap<const NodeT *, unsigned> NodeNumberMap;
265+
// TODO: use an empty struct with [[no_unique_address]] in C++20.
266+
std::conditional_t<!GraphHasNodeNumbers<NodeT *>,
267+
DenseMap<const NodeT *, unsigned>, std::tuple<>>
268+
NodeNumberMap;
266269
DomTreeNodeBase<NodeT> *RootNode = nullptr;
267270
ParentPtr Parent = nullptr;
268271

@@ -355,12 +358,8 @@ class DominatorTreeBase {
355358
}
356359

357360
private:
358-
template <typename T>
359-
using has_number_t =
360-
decltype(GraphTraits<T *>::getNumber(std::declval<T *>()));
361-
362361
std::optional<unsigned> getNodeIndex(const NodeT *BB) const {
363-
if constexpr (is_detected<has_number_t, NodeT>::value) {
362+
if constexpr (GraphHasNodeNumbers<NodeT *>) {
364363
// BB can be nullptr, map nullptr to index 0.
365364
assert(BlockNumberEpoch ==
366365
GraphTraits<ParentPtr>::getNumberEpoch(Parent) &&
@@ -374,7 +373,7 @@ class DominatorTreeBase {
374373
}
375374

376375
unsigned getNodeIndexForInsert(const NodeT *BB) {
377-
if constexpr (is_detected<has_number_t, NodeT>::value) {
376+
if constexpr (GraphHasNodeNumbers<NodeT *>) {
378377
// getNodeIndex will never fail if nodes have getNumber().
379378
unsigned Idx = *getNodeIndex(BB);
380379
if (Idx >= DomTreeNodes.size()) {
@@ -736,7 +735,8 @@ class DominatorTreeBase {
736735
}
737736

738737
DomTreeNodes[*IdxOpt] = nullptr;
739-
NodeNumberMap.erase(BB);
738+
if constexpr (!GraphHasNodeNumbers<NodeT *>)
739+
NodeNumberMap.erase(BB);
740740

741741
if (!IsPostDom) return;
742742

@@ -830,7 +830,7 @@ class DominatorTreeBase {
830830
private:
831831
void updateBlockNumberEpoch() {
832832
// Nothing to do for graphs that don't number their blocks.
833-
if constexpr (is_detected<has_number_t, NodeT>::value)
833+
if constexpr (GraphHasNodeNumbers<NodeT *>)
834834
BlockNumberEpoch = GraphTraits<ParentPtr>::getNumberEpoch(Parent);
835835
}
836836

@@ -849,9 +849,8 @@ class DominatorTreeBase {
849849
}
850850

851851
/// Update dominator tree after renumbering blocks.
852-
template <class T_ = NodeT>
853-
std::enable_if_t<is_detected<has_number_t, T_>::value, void>
854-
updateBlockNumbers() {
852+
template <typename T = NodeT>
853+
std::enable_if_t<GraphHasNodeNumbers<T *>, void> updateBlockNumbers() {
855854
updateBlockNumberEpoch();
856855

857856
unsigned MaxNumber = GraphTraits<ParentPtr>::getMaxNumber(Parent);
@@ -889,7 +888,8 @@ class DominatorTreeBase {
889888

890889
void reset() {
891890
DomTreeNodes.clear();
892-
NodeNumberMap.clear();
891+
if constexpr (!GraphHasNodeNumbers<NodeT *>)
892+
NodeNumberMap.clear();
893893
Roots.clear();
894894
RootNode = nullptr;
895895
Parent = nullptr;
@@ -989,7 +989,8 @@ class DominatorTreeBase {
989989
/// assignable and destroyable state, but otherwise invalid.
990990
void wipe() {
991991
DomTreeNodes.clear();
992-
NodeNumberMap.clear();
992+
if constexpr (!GraphHasNodeNumbers<NodeT *>)
993+
NodeNumberMap.clear();
993994
RootNode = nullptr;
994995
Parent = nullptr;
995996
}

0 commit comments

Comments
 (0)