Skip to content

Commit

Permalink
[Support][NFC] Fix generic ChildrenGetterTy of IDFCalculatorBase
Browse files Browse the repository at this point in the history
Both IDFCalculatorBase and its accompanying DominatorTreeBase only supports pointer nodes. The template argument is the block type itself and any uses of GraphTraits is therefore done via a pointer to the node type.
However, the ChildrenGetterTy type of IDFCalculatorBase has a use on just the node type instead of a pointer to the node type. Various parts of the monorepo has worked around this issue by providing specializations of GraphTraits for the node type directly, or not been affected by using specializations instead of the generic case. These are unnecessary however and instead the generic code should be fixed instead.

An example from within Tree is eg. A use of IDFCalculatorBase in InstrRefBasedImpl.cpp. It basically instantiates a IDFCalculatorBase<MachineBasicBlock, false> but due to the bug above then goes on to specialize GraphTraits<MachineBasicBlock> although GraphTraits<MachineBasicBlock*> exists (and should be used instead).

Similar dead code exists in clang which defines redundant GraphTraits to work around this bug.

This patch fixes both the original issue and removes the dead code that was used to work around the issue.

Differential Revision: https://reviews.llvm.org/D118386
  • Loading branch information
zero9178 committed Jan 30, 2022
1 parent e107518 commit e0b11c7
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 50 deletions.
2 changes: 1 addition & 1 deletion clang/include/clang/Analysis/Analyses/Dominators.h
Expand Up @@ -193,7 +193,7 @@ namespace IDFCalculatorDetail {
/// Specialize ChildrenGetterTy to skip nullpointer successors.
template <bool IsPostDom>
struct ChildrenGetterTy<clang::CFGBlock, IsPostDom> {
using NodeRef = typename GraphTraits<clang::CFGBlock>::NodeRef;
using NodeRef = typename GraphTraits<clang::CFGBlock *>::NodeRef;
using ChildrenTy = SmallVector<NodeRef, 8>;

ChildrenTy get(const NodeRef &N) {
Expand Down
12 changes: 0 additions & 12 deletions clang/include/clang/Analysis/CFG.h
Expand Up @@ -1494,9 +1494,6 @@ template <> struct GraphTraits< ::clang::CFGBlock *> {
static ChildIteratorType child_end(NodeRef N) { return N->succ_end(); }
};

template <> struct GraphTraits<clang::CFGBlock>
: GraphTraits<clang::CFGBlock *> {};

template <> struct GraphTraits< const ::clang::CFGBlock *> {
using NodeRef = const ::clang::CFGBlock *;
using ChildIteratorType = ::clang::CFGBlock::const_succ_iterator;
Expand All @@ -1506,9 +1503,6 @@ template <> struct GraphTraits< const ::clang::CFGBlock *> {
static ChildIteratorType child_end(NodeRef N) { return N->succ_end(); }
};

template <> struct GraphTraits<const clang::CFGBlock>
: GraphTraits<clang::CFGBlock *> {};

template <> struct GraphTraits<Inverse< ::clang::CFGBlock *>> {
using NodeRef = ::clang::CFGBlock *;
using ChildIteratorType = ::clang::CFGBlock::const_pred_iterator;
Expand All @@ -1521,9 +1515,6 @@ template <> struct GraphTraits<Inverse< ::clang::CFGBlock *>> {
static ChildIteratorType child_end(NodeRef N) { return N->pred_end(); }
};

template <> struct GraphTraits<Inverse<clang::CFGBlock>>
: GraphTraits<clang::CFGBlock *> {};

template <> struct GraphTraits<Inverse<const ::clang::CFGBlock *>> {
using NodeRef = const ::clang::CFGBlock *;
using ChildIteratorType = ::clang::CFGBlock::const_pred_iterator;
Expand All @@ -1536,9 +1527,6 @@ template <> struct GraphTraits<Inverse<const ::clang::CFGBlock *>> {
static ChildIteratorType child_end(NodeRef N) { return N->pred_end(); }
};

template <> struct GraphTraits<const Inverse<clang::CFGBlock>>
: GraphTraits<clang::CFGBlock *> {};

// Traits for: CFG

template <> struct GraphTraits< ::clang::CFG* >
Expand Down
Expand Up @@ -37,7 +37,7 @@ namespace IDFCalculatorDetail {
/// May be specialized if, for example, one wouldn't like to return nullpointer
/// successors.
template <class NodeTy, bool IsPostDom> struct ChildrenGetterTy {
using NodeRef = typename GraphTraits<NodeTy>::NodeRef;
using NodeRef = typename GraphTraits<NodeTy *>::NodeRef;
using ChildrenTy = SmallVector<NodeRef, 8>;

ChildrenTy get(const NodeRef &N);
Expand Down
37 changes: 1 addition & 36 deletions llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
Expand Up @@ -2212,49 +2212,14 @@ void InstrRefBasedLDV::buildMLocValueMap(
// redundant PHIs.
}

// Boilerplate for feeding MachineBasicBlocks into IDF calculator. Provide
// template specialisations for graph traits and a successor enumerator.
namespace llvm {
template <> struct GraphTraits<MachineBasicBlock> {
using NodeRef = MachineBasicBlock *;
using ChildIteratorType = MachineBasicBlock::succ_iterator;

static NodeRef getEntryNode(MachineBasicBlock *BB) { return BB; }
static ChildIteratorType child_begin(NodeRef N) { return N->succ_begin(); }
static ChildIteratorType child_end(NodeRef N) { return N->succ_end(); }
};

template <> struct GraphTraits<const MachineBasicBlock> {
using NodeRef = const MachineBasicBlock *;
using ChildIteratorType = MachineBasicBlock::const_succ_iterator;

static NodeRef getEntryNode(const MachineBasicBlock *BB) { return BB; }
static ChildIteratorType child_begin(NodeRef N) { return N->succ_begin(); }
static ChildIteratorType child_end(NodeRef N) { return N->succ_end(); }
};

using MachineDomTreeBase = DomTreeBase<MachineBasicBlock>::NodeType;
using MachineDomTreeChildGetter =
typename IDFCalculatorDetail::ChildrenGetterTy<MachineDomTreeBase, false>;

namespace IDFCalculatorDetail {
template <>
typename MachineDomTreeChildGetter::ChildrenTy
MachineDomTreeChildGetter::get(const NodeRef &N) {
return {N->succ_begin(), N->succ_end()};
}
} // namespace IDFCalculatorDetail
} // namespace llvm

void InstrRefBasedLDV::BlockPHIPlacement(
const SmallPtrSetImpl<MachineBasicBlock *> &AllBlocks,
const SmallPtrSetImpl<MachineBasicBlock *> &DefBlocks,
SmallVectorImpl<MachineBasicBlock *> &PHIBlocks) {
// Apply IDF calculator to the designated set of location defs, storing
// required PHIs into PHIBlocks. Uses the dominator tree stored in the
// InstrRefBasedLDV object.
IDFCalculatorDetail::ChildrenGetterTy<MachineDomTreeBase, false> foo;
IDFCalculatorBase<MachineDomTreeBase, false> IDF(DomTree->getBase(), foo);
IDFCalculatorBase<MachineBasicBlock, false> IDF(DomTree->getBase());

IDF.setLiveInBlocks(AllBlocks);
IDF.setDefiningBlocks(DefBlocks);
Expand Down

0 comments on commit e0b11c7

Please sign in to comment.