zero9178 created this revision.
zero9178 added reviewers: Szelethus, jmorse, kuhar.
Herald added subscribers: Chia-hungDuan, dexonsmith, rriddle, hiraditya.
zero9178 requested review of this revision.
Herald added subscribers: llvm-commits, cfe-commits, stephenneuendorffer.
Herald added projects: clang, LLVM.
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.
As a side node, my own motivation was when using MLIR and using `mlir::Block`s
as nodes. As MLIR correctly only provides `GraphTraits<mlir::Block*>` I was not
able to use `IDFCalculatorBase`.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D118386
Files:
clang/include/clang/Analysis/Analyses/Dominators.h
clang/include/clang/Analysis/CFG.h
llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h
llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
Index: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
===================================================================
--- llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
+++ llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp
@@ -2212,40 +2212,6 @@
// 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,
@@ -2253,8 +2219,7 @@
// 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);
Index: llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h
===================================================================
--- llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h
+++ llvm/include/llvm/Support/GenericIteratedDominanceFrontier.h
@@ -37,7 +37,7 @@
/// 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);
Index: clang/include/clang/Analysis/CFG.h
===================================================================
--- clang/include/clang/Analysis/CFG.h
+++ clang/include/clang/Analysis/CFG.h
@@ -1494,9 +1494,6 @@
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;
@@ -1506,9 +1503,6 @@
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;
@@ -1521,9 +1515,6 @@
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;
@@ -1536,9 +1527,6 @@
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* >
Index: clang/include/clang/Analysis/Analyses/Dominators.h
===================================================================
--- clang/include/clang/Analysis/Analyses/Dominators.h
+++ clang/include/clang/Analysis/Analyses/Dominators.h
@@ -193,7 +193,7 @@
/// 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) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
