Skip to content

[Support][ADT] Minor cleanups after #101706 #102180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion llvm/include/llvm/ADT/GraphTraits.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 12 additions & 0 deletions llvm/include/llvm/CodeGen/MachineBasicBlock.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand All @@ -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;
Expand All @@ -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) {
Expand Down
29 changes: 15 additions & 14 deletions llvm/include/llvm/Support/GenericDomTree.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,10 @@ 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;
// TODO: use an empty struct with [[no_unique_address]] in C++20.
std::conditional_t<!GraphHasNodeNumbers<NodeT *>,
DenseMap<const NodeT *, unsigned>, std::tuple<>>
Comment on lines +266 to +267
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: consider adding todo to switch to an empty struct with no_unique_address once we update to C++20.

NodeNumberMap;
DomTreeNodeBase<NodeT> *RootNode = nullptr;
ParentPtr Parent = nullptr;

Expand Down Expand Up @@ -355,12 +358,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) &&
Expand All @@ -374,7 +373,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()) {
Expand Down Expand Up @@ -736,7 +735,8 @@ class DominatorTreeBase {
}

DomTreeNodes[*IdxOpt] = nullptr;
NodeNumberMap.erase(BB);
if constexpr (!GraphHasNodeNumbers<NodeT *>)
NodeNumberMap.erase(BB);

if (!IsPostDom) return;

Expand Down Expand Up @@ -830,7 +830,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);
}

Expand All @@ -849,9 +849,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);
Expand Down Expand Up @@ -889,7 +888,8 @@ class DominatorTreeBase {

void reset() {
DomTreeNodes.clear();
NodeNumberMap.clear();
if constexpr (!GraphHasNodeNumbers<NodeT *>)
NodeNumberMap.clear();
Roots.clear();
RootNode = nullptr;
Parent = nullptr;
Expand Down Expand Up @@ -989,7 +989,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;
}
Expand Down
Loading