[llvm] r280122 - ADT: Guarantee transferNodesFromList is only called on transfers

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 30 11:00:45 PDT 2016


Author: dexonsmith
Date: Tue Aug 30 13:00:45 2016
New Revision: 280122

URL: http://llvm.org/viewvc/llvm-project?rev=280122&view=rev
Log:
ADT: Guarantee transferNodesFromList is only called on transfers

Guarantee that ilist_traits<T>::transferNodesFromList is only called
when nodes are actually changing lists.

I also moved all the callbacks to occur *first*, before the operation.
This is the only choice for iplist<T>::merge, so we might as well be
consistent.  I expect this to have no effect in practice, although it
simplifies the logic in both iplist<T>::transfer and iplist<T>::insert.

Modified:
    llvm/trunk/include/llvm/ADT/ilist.h
    llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp
    llvm/trunk/lib/IR/SymbolTableListTraitsImpl.h

Modified: llvm/trunk/include/llvm/ADT/ilist.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/ilist.h?rev=280122&r1=280121&r2=280122&view=diff
==============================================================================
--- llvm/trunk/include/llvm/ADT/ilist.h (original)
+++ llvm/trunk/include/llvm/ADT/ilist.h Tue Aug 30 13:00:45 2016
@@ -43,9 +43,15 @@ template <typename NodeTy> struct ilist_
 
   void addNodeToList(NodeTy *) {}
   void removeNodeFromList(NodeTy *) {}
-  void transferNodesFromList(ilist_node_traits & /*SrcTraits*/,
+
+  /// Callback before transferring nodes to this list.
+  ///
+  /// \pre \c this!=&OldList
+  void transferNodesFromList(ilist_node_traits &OldList,
                              ilist_iterator<NodeTy> /*first*/,
-                             ilist_iterator<NodeTy> /*last*/) {}
+                             ilist_iterator<NodeTy> /*last*/) {
+    (void)OldList;
+  }
 };
 
 /// Default template traits for intrusive list.
@@ -165,9 +171,8 @@ public:
   }
 
   iterator insert(iterator where, NodeTy *New) {
-    auto I = base_list_type::insert(where, *New);
-    this->addNodeToList(New);  // Notify traits that we added a node...
-    return I;
+    this->addNodeToList(New); // Notify traits that we added a node...
+    return base_list_type::insert(where, *New);
   }
 
   iterator insert(iterator where, const NodeTy &New) {
@@ -182,9 +187,9 @@ public:
   }
 
   NodeTy *remove(iterator &IT) {
-    NodeTy *Node = &*IT;
-    base_list_type::erase(IT++);
-    this->removeNodeFromList(Node);  // Notify traits that we removed a node...
+    NodeTy *Node = &*IT++;
+    this->removeNodeFromList(Node); // Notify traits that we removed a node...
+    base_list_type::remove(*Node);
     return Node;
   }
 
@@ -220,11 +225,10 @@ private:
     if (position == last)
       return;
 
-    base_list_type::splice(position, L2, first, last);
+    if (this != &L2) // Notify traits we moved the nodes...
+      this->transferNodesFromList(L2, first, last);
 
-    // Callback.  Note that the nodes have moved from before-last to
-    // before-position.
-    this->transferNodesFromList(L2, first, position);
+    base_list_type::splice(position, L2, first, last);
   }
 
 public:

Modified: llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp?rev=280122&r1=280121&r2=280122&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp Tue Aug 30 13:00:45 2016
@@ -122,9 +122,8 @@ transferNodesFromList(ilist_traits<Machi
                       ilist_iterator<MachineInstr> Last) {
   assert(Parent->getParent() == FromList.Parent->getParent() &&
         "MachineInstr parent mismatch!");
-
-  // Splice within the same MBB -> no change.
-  if (Parent == FromList.Parent) return;
+  assert(this != &FromList && "Called without a real transfer...");
+  assert(Parent != FromList.Parent && "Two lists have the same parent?");
 
   // If splicing between two blocks within the same function, just update the
   // parent pointers.

Modified: llvm/trunk/lib/IR/SymbolTableListTraitsImpl.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/SymbolTableListTraitsImpl.h?rev=280122&r1=280121&r2=280122&view=diff
==============================================================================
--- llvm/trunk/lib/IR/SymbolTableListTraitsImpl.h (original)
+++ llvm/trunk/lib/IR/SymbolTableListTraitsImpl.h Tue Aug 30 13:00:45 2016
@@ -85,7 +85,7 @@ void SymbolTableListTraits<ValueSubClass
     ilist_iterator<ValueSubClass> last) {
   // We only have to do work here if transferring instructions between BBs
   ItemParentClass *NewIP = getListOwner(), *OldIP = L2.getListOwner();
-  if (NewIP == OldIP) return;  // No work to do at all...
+  assert(NewIP != OldIP && "Expected different list owners");
 
   // We only have to update symbol table entries if we are transferring the
   // instructions to a different symtab object...




More information about the llvm-commits mailing list