[PATCH] D57120: [ADT] Notify ilist traits about in-list transfers

[Reid Kleckner via Phabricator via llvm-commits]

rnk created this revision.
rnk added reviewers: lattner, hfinkel, chandlerc.
Herald added subscribers: dexonsmith, hiraditya.

Previously no client of ilist traits has needed to know about transfers
of nodes within the same list, so as an optimization, ilist doesn't call
transferNodesFromList in that case. However, now there are clients that
want to use ilist traits to cache instruction ordering information to
optimize dominance queries of instructions in the same basic block.
This change updates the existing ilist traits users to detect in-list
transfers and do nothing in that case.

After this change, we can start caching instruction ordering information
in LLVM IR data structures. There are two main ways to do that:

- by putting an order integer into the Instruction class
- by maintaining order integers in a hash table on BasicBlock

I plan to implement and measure both, but I wanted to commit this change
first to enable other out of tree ilist clients to implement this
optimization as well.


https://reviews.llvm.org/D57120

Files:
  llvm/include/llvm/ADT/ilist.h
  llvm/include/llvm/CodeGen/MachineFunction.h
  llvm/lib/CodeGen/MachineBasicBlock.cpp
  llvm/lib/IR/SymbolTableListTraitsImpl.h


Index: llvm/lib/IR/SymbolTableListTraitsImpl.h
===================================================================
--- llvm/lib/IR/SymbolTableListTraitsImpl.h
+++ llvm/lib/IR/SymbolTableListTraitsImpl.h
@@ -83,7 +83,8 @@
     SymbolTableListTraits &L2, iterator first, iterator last) {
   // We only have to do work here if transferring instructions between BBs
   ItemParentClass *NewIP = getListOwner(), *OldIP = L2.getListOwner();
-  assert(NewIP != OldIP && "Expected different list owners");
+  if (NewIP == OldIP)
+    return;
 
   // We only have to update symbol table entries if we are transferring the
   // instructions to a different symtab object...
Index: llvm/lib/CodeGen/MachineBasicBlock.cpp
===================================================================
--- llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -132,8 +132,12 @@
                                                        instr_iterator First,
                                                        instr_iterator Last) {
   assert(Parent->getParent() == FromList.Parent->getParent() &&
-        "MachineInstr parent mismatch!");
-  assert(this != &FromList && "Called without a real transfer...");
+         "cannot transfer MachineInstrs between MachineFunctions");
+
+  // If it's within the same BB, there's nothing to do.
+  if (this == &FromList)
+    return;
+
   assert(Parent != FromList.Parent && "Two lists have the same parent?");
 
   // If splicing between two blocks within the same function, just update the
Index: llvm/include/llvm/CodeGen/MachineFunction.h
===================================================================
--- llvm/include/llvm/CodeGen/MachineFunction.h
+++ llvm/include/llvm/CodeGen/MachineFunction.h
@@ -85,7 +85,7 @@
 
   template <class Iterator>
   void transferNodesFromList(ilist_callback_traits &OldList, Iterator, Iterator) {
-    llvm_unreachable("Never transfer between lists");
+    assert(this == &OldList && "never transfer MBBs between functions");
   }
 };
 
Index: llvm/include/llvm/ADT/ilist.h
===================================================================
--- llvm/include/llvm/ADT/ilist.h
+++ llvm/include/llvm/ADT/ilist.h
@@ -65,9 +65,8 @@
   void addNodeToList(NodeTy *) {}
   void removeNodeFromList(NodeTy *) {}
 
-  /// Callback before transferring nodes to this list.
-  ///
-  /// \pre \c this!=&OldList
+  /// Callback before transferring nodes to this list. The nodes may already be
+  /// in this same list.
   template <class Iterator>
   void transferNodesFromList(ilist_callback_traits &OldList, Iterator /*first*/,
                              Iterator /*last*/) {
@@ -286,8 +285,8 @@
     if (position == last)
       return;
 
-    if (this != &L2) // Notify traits we moved the nodes...
-      this->transferNodesFromList(L2, first, last);
+    // Notify traits we moved the nodes...
+    this->transferNodesFromList(L2, first, last);
 
     base_list_type::splice(position, L2, first, last);
   }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D57120.183182.patch
Type: text/x-patch
Size: 2997 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190123/8cb68dbc/attachment.bin>