[llvm] r281184 - ADT: Never allocate nodes in iplist<> and ilist<>
Duncan P. N. Exon Smith via llvm-commits
llvm-commits at lists.llvm.org
Sun Sep 11 16:43:43 PDT 2016
Author: dexonsmith
Date: Sun Sep 11 18:43:43 2016
New Revision: 281184
URL: http://llvm.org/viewvc/llvm-project?rev=281184&view=rev
Log:
ADT: Never allocate nodes in iplist<> and ilist<>
Remove createNode() and any API that depending on it, and add
HasCreateNode to the list of checks for HasObsoleteCustomizations. Now
an ilist *never* allocates (this was already true for iplist).
This factors out all the differences between iplist and ilist. I'll aim
to rename both to "owning_ilist" eventually, to call out the interesting
(not exactly intrusive) ownership semantics. In the meantime, I've left
both names around to reduce code churn.
One of the deleted APIs is the ilist copy constructor. I've lifted up
and tested iplist::cloneFrom (ala simple_ilist::cloneFrom) as a
replacement.
Users of ilist<> and iplist<> that want the list to allocate nodes have
a few options:
- use std::list;
- use AllocatorList or BumpPtrList (or build a similarly trivial list);
- use cloneFrom (which is explicit at the call site); or
- allocate at the call site.
See r280573, r281177, r281181, and r281182 for examples of what to do if
you're updating out-of-tree code.
Modified:
llvm/trunk/include/llvm/ADT/ilist.h
llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h
llvm/trunk/include/llvm/CodeGen/MachineFunction.h
llvm/trunk/include/llvm/CodeGen/SelectionDAG.h
llvm/trunk/include/llvm/MC/MCSection.h
llvm/trunk/unittests/ADT/IListTest.cpp
Modified: llvm/trunk/include/llvm/ADT/ilist.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ilist.h?rev=281184&r1=281183&r2=281184&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/ilist.h (original)
+++ llvm/trunk/include/llvm/ADT/ilist.h Sun Sep 11 18:43:43 2016
@@ -33,25 +33,24 @@
namespace llvm {
-/// Use new/delete by default for iplist and ilist.
+/// Use delete by default for iplist and ilist.
///
-/// Specialize this to get different behaviour for allocation-related API. (If
-/// you really want new/delete, consider just using std::list.)
+/// Specialize this to get different behaviour for ownership-related API. (If
+/// you really want ownership semantics, consider using std::list or building
+/// something like \a BumpPtrList.)
///
/// \see ilist_noalloc_traits
template <typename NodeTy> struct ilist_alloc_traits {
- /// Clone a node.
- ///
- /// TODO: Remove this and API that relies on it (it's dead code).
- static NodeTy *createNode(const NodeTy &V) { return new NodeTy(V); }
static void deleteNode(NodeTy *V) { delete V; }
};
-/// Custom traits to disable node creation and do nothing on deletion.
+/// Custom traits to do nothing on deletion.
///
/// Specialize ilist_alloc_traits to inherit from this to disable the
-/// non-intrusive parts of iplist and/or ilist. It has no createNode function,
-/// and deleteNode does nothing.
+/// non-intrusive deletion in iplist (which implies ownership).
+///
+/// If you want purely intrusive semantics with no callbacks, consider using \a
+/// simple_ilist instead.
///
/// \code
/// template <>
@@ -139,9 +138,26 @@ public:
static const bool value = sizeof(test<TraitsT>(nullptr)) == sizeof(Yes);
};
+/// Type trait to check for a traits class that has a createNode member.
+/// Allocation should be managed in a wrapper class, instead of in
+/// ilist_traits.
+template <class TraitsT, class NodeT> struct HasCreateNode {
+ typedef char Yes[1];
+ typedef char No[2];
+ template <size_t N> struct SFINAE {};
+
+ template <class U>
+ static Yes &test(U *I, decltype(I->createNode(make<NodeT>())) * = 0);
+ template <class> static No &test(...);
+
+public:
+ static const bool value = sizeof(test<TraitsT>(nullptr)) == sizeof(Yes);
+};
+
template <class TraitsT, class NodeT> struct HasObsoleteCustomization {
- static const bool value =
- HasGetNext<TraitsT, NodeT>::value || HasCreateSentinel<TraitsT>::value;
+ static const bool value = HasGetNext<TraitsT, NodeT>::value ||
+ HasCreateSentinel<TraitsT>::value ||
+ HasCreateNode<TraitsT, NodeT>::value;
};
} // end namespace ilist_detail
@@ -239,6 +255,13 @@ public:
return insert(++where, New);
}
+ /// Clone another list.
+ template <class Cloner> void cloneFrom(const iplist_impl &L2, Cloner clone) {
+ clear();
+ for (const_reference V : L2)
+ push_back(clone(V));
+ }
+
pointer remove(iterator &IT) {
pointer Node = &*IT++;
this->removeNodeFromList(Node); // Notify traits that we removed a node...
@@ -394,91 +417,7 @@ public:
iplist &operator=(const iplist &X) = delete;
};
-/// An intrusive list with ownership and callbacks specified/controlled by
-/// ilist_traits, with API that is unsafe for polymorphic types.
-template <class T, class... Options>
-class ilist : public iplist<T, Options...> {
- typedef iplist<T, Options...> base_list_type;
-
-public:
- typedef typename base_list_type::size_type size_type;
- typedef typename base_list_type::iterator iterator;
- typedef typename base_list_type::value_type value_type;
- typedef typename base_list_type::const_reference const_reference;
-
- ilist() {}
- ilist(const ilist &right) : base_list_type() {
- insert(this->begin(), right.begin(), right.end());
- }
- explicit ilist(size_type count) {
- insert(this->begin(), count, value_type());
- }
- ilist(size_type count, const_reference val) {
- insert(this->begin(), count, val);
- }
- template<class InIt> ilist(InIt first, InIt last) {
- insert(this->begin(), first, last);
- }
-
- ilist(ilist &&X) : base_list_type(std::move(X)) {}
- ilist &operator=(ilist &&X) {
- *static_cast<base_list_type *>(this) = std::move(X);
- return *this;
- }
-
- // bring hidden functions into scope
- using base_list_type::insert;
- using base_list_type::push_front;
- using base_list_type::push_back;
-
- // Main implementation here - Insert for a node passed by value...
- iterator insert(iterator where, const_reference val) {
- return insert(where, this->createNode(val));
- }
-
-
- // Front and back inserters...
- void push_front(const_reference val) { insert(this->begin(), val); }
- void push_back(const_reference val) { insert(this->end(), val); }
-
- void insert(iterator where, size_type count, const_reference val) {
- for (; count != 0; --count) insert(where, val);
- }
-
- // Assign special forms...
- void assign(size_type count, const_reference val) {
- iterator I = this->begin();
- for (; I != this->end() && count != 0; ++I, --count)
- *I = val;
- if (count != 0)
- insert(this->end(), val, val);
- else
- erase(I, this->end());
- }
- template<class InIt> void assign(InIt first1, InIt last1) {
- iterator first2 = this->begin(), last2 = this->end();
- for ( ; first1 != last1 && first2 != last2; ++first1, ++first2)
- *first1 = *first2;
- if (first2 == last2)
- erase(first1, last1);
- else
- insert(last1, first2, last2);
- }
-
-
- // Resize members...
- void resize(size_type newsize, value_type val) {
- iterator i = this->begin();
- size_type len = 0;
- for ( ; i != this->end() && len < newsize; ++i, ++len) /* empty*/ ;
-
- if (len == newsize)
- erase(i, this->end());
- else // i == end()
- insert(this->end(), newsize - len, val);
- }
- void resize(size_type newsize) { resize(newsize, value_type()); }
-};
+template <class T, class... Options> using ilist = iplist<T, Options...>;
} // End llvm namespace
Modified: llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h?rev=281184&r1=281183&r2=281184&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h (original)
+++ llvm/trunk/include/llvm/CodeGen/MachineBasicBlock.h Sun Sep 11 18:43:43 2016
@@ -53,7 +53,6 @@ public:
instr_iterator Last);
void deleteNode(MachineInstr *MI);
- // Leave out createNode...
};
class MachineBasicBlock
Modified: llvm/trunk/include/llvm/CodeGen/MachineFunction.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineFunction.h?rev=281184&r1=281183&r2=281184&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/MachineFunction.h (original)
+++ llvm/trunk/include/llvm/CodeGen/MachineFunction.h Sun Sep 11 18:43:43 2016
@@ -50,7 +50,6 @@ struct WinEHFuncInfo;
template <> struct ilist_alloc_traits<MachineBasicBlock> {
void deleteNode(MachineBasicBlock *MBB);
- // Disallow createNode...
};
template <> struct ilist_callback_traits<MachineBasicBlock> {
Modified: llvm/trunk/include/llvm/CodeGen/SelectionDAG.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/SelectionDAG.h?rev=281184&r1=281183&r2=281184&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/SelectionDAG.h (original)
+++ llvm/trunk/include/llvm/CodeGen/SelectionDAG.h Sun Sep 11 18:43:43 2016
@@ -85,7 +85,6 @@ template <> struct ilist_alloc_traits<SD
static void deleteNode(SDNode *) {
llvm_unreachable("ilist_traits<SDNode> shouldn't see a deleteNode call!");
}
- // Don't implement createNode...
};
/// Keeps track of dbg_value information through SDISel. We do
Modified: llvm/trunk/include/llvm/MC/MCSection.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCSection.h?rev=281184&r1=281183&r2=281184&view=diff
==============================================================================
--- llvm/trunk/include/llvm/MC/MCSection.h (original)
+++ llvm/trunk/include/llvm/MC/MCSection.h Sun Sep 11 18:43:43 2016
@@ -33,7 +33,6 @@ class raw_ostream;
template <> struct ilist_alloc_traits<MCFragment> {
static void deleteNode(MCFragment *V);
- // Leave out createNode...
};
/// Instances of this class represent a uniqued identifier for a section in the
Modified: llvm/trunk/unittests/ADT/IListTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/IListTest.cpp?rev=281184&r1=281183&r2=281184&view=diff
==============================================================================
--- llvm/trunk/unittests/ADT/IListTest.cpp (original)
+++ llvm/trunk/unittests/ADT/IListTest.cpp Sun Sep 11 18:43:43 2016
@@ -28,12 +28,12 @@ struct Node : ilist_node<Node> {
TEST(IListTest, Basic) {
ilist<Node> List;
- List.push_back(Node(1));
+ List.push_back(new Node(1));
EXPECT_EQ(1, List.back().Value);
EXPECT_EQ(nullptr, List.getPrevNode(List.back()));
EXPECT_EQ(nullptr, List.getNextNode(List.back()));
- List.push_back(Node(2));
+ List.push_back(new Node(2));
EXPECT_EQ(2, List.back().Value);
EXPECT_EQ(2, List.getNextNode(List.front())->Value);
EXPECT_EQ(1, List.getPrevNode(List.back())->Value);
@@ -44,9 +44,40 @@ TEST(IListTest, Basic) {
EXPECT_EQ(1, ConstList.getPrevNode(ConstList.back())->Value);
}
+TEST(IListTest, cloneFrom) {
+ Node L1Nodes[] = {Node(0), Node(1)};
+ Node L2Nodes[] = {Node(0), Node(1)};
+ ilist<Node> L1, L2, L3;
+
+ // Build L1 from L1Nodes.
+ L1.push_back(&L1Nodes[0]);
+ L1.push_back(&L1Nodes[1]);
+
+ // Build L2 from L2Nodes, based on L1 nodes.
+ L2.cloneFrom(L1, [&](const Node &N) { return &L2Nodes[N.Value]; });
+
+ // Add a node to L3 to be deleted, and then rebuild L3 by copying L1.
+ L3.push_back(new Node(7));
+ L3.cloneFrom(L1, [](const Node &N) { return new Node(N); });
+
+ EXPECT_EQ(2u, L1.size());
+ EXPECT_EQ(&L1Nodes[0], &L1.front());
+ EXPECT_EQ(&L1Nodes[1], &L1.back());
+ EXPECT_EQ(2u, L2.size());
+ EXPECT_EQ(&L2Nodes[0], &L2.front());
+ EXPECT_EQ(&L2Nodes[1], &L2.back());
+ EXPECT_EQ(2u, L3.size());
+ EXPECT_EQ(0, L3.front().Value);
+ EXPECT_EQ(1, L3.back().Value);
+
+ // Don't free nodes on the stack.
+ L1.clearAndLeakNodesUnsafely();
+ L2.clearAndLeakNodesUnsafely();
+}
+
TEST(IListTest, SpliceOne) {
ilist<Node> List;
- List.push_back(1);
+ List.push_back(new Node(1));
// The single-element splice operation supports noops.
List.splice(List.begin(), List, List.begin());
@@ -55,8 +86,8 @@ TEST(IListTest, SpliceOne) {
EXPECT_TRUE(std::next(List.begin()) == List.end());
// Altenative noop. Move the first element behind itself.
- List.push_back(2);
- List.push_back(3);
+ List.push_back(new Node(2));
+ List.push_back(new Node(3));
List.splice(std::next(List.begin()), List, List.begin());
EXPECT_EQ(3u, List.size());
EXPECT_EQ(1, List.front().Value);
@@ -111,7 +142,7 @@ TEST(IListTest, UnsafeClear) {
EXPECT_TRUE(E == List.end());
// List with contents.
- List.push_back(1);
+ List.push_back(new Node(1));
ASSERT_EQ(1u, List.size());
Node *N = &*List.begin();
EXPECT_EQ(1, N->Value);
@@ -121,8 +152,8 @@ TEST(IListTest, UnsafeClear) {
delete N;
// List is still functional.
- List.push_back(5);
- List.push_back(6);
+ List.push_back(new Node(5));
+ List.push_back(new Node(6));
ASSERT_EQ(2u, List.size());
EXPECT_EQ(5, List.front().Value);
EXPECT_EQ(6, List.back().Value);
More information about the llvm-commits
mailing list