SIL: simplify deleting instruction while iterating over instructions.

Add `deletableInstructions()` and `reverseDeletableInstructions()` in SILBasicBlock.
It allows deleting instructions while iterating over all instructions of the block.
This is a replacement for `InstructionDeleter::updatingRange()`.
It's a simpler implementation than the existing `UpdatingListIterator` and `UpdatingInstructionIteratorRegistry`, because it just needs to keep the prev/next pointers for "deleted" instructions instead of the iterator-registration machinery.
It's also safer, because it doesn't require to delete instructions via a specific instance of an InstructionDeleter (which can be missed easily).

Author: eeckstein

include/swift/SIL/SILBasicBlock.h

@@ -31,6 +31,51 @@ class SILFunction;
 class SILArgument;
 class SILPrintContext;
 
+/// Instruction iterator which allows to "delete" instructions while iterating
+/// over the instruction list.
+///
+/// Iteration with this iterator allows to delete the current, the next or any
+/// instruction in the list while iterating.
+/// This works because instruction deletion is deferred (for details see
+/// `SILModule::scheduledForDeletion`) and removing an instruction from the list
+/// keeps the prev/next pointers (see `SILInstructionListBase`).
+template <typename IteratorBase>
+class DeletableInstructionsIterator {
+  using Self = DeletableInstructionsIterator<IteratorBase>;
+
+  IteratorBase base;
+  IteratorBase end;
+
+public:
+  using value_type = typename IteratorBase::value_type;
+  using difference_type = ptrdiff_t;
+  using pointer = value_type *;
+  using iterator_category = std::forward_iterator_tag;
+
+  DeletableInstructionsIterator(IteratorBase base, IteratorBase end)
+  : base(base), end(end) {}
+
+  value_type &operator*() const { return *base; }
+
+  SILInstruction *operator->() const { return base.operator->(); }
+
+  Self &operator++() {
+    // If the current instruction is "deleted" (which means: removed from the
+    // list), it's prev/next pointers still point to the next instruction which
+    // is still in the list - or "deleted", too.
+    ++base;
+    // Skip over all deleted instructions. Eventually we reach an instruction
+    // is still in the list (= not "deleted") or the end iterator.
+    while (base != end && base->isDeleted()) {
+      ++base;
+    }
+    return *this;
+  }
+
+  bool operator==(const Self &rhs) const { return base == rhs.base; }
+  bool operator!=(const Self &rhs) const { return !(*this == rhs); }
+};
+
 class SILBasicBlock :
 public llvm::ilist_node<SILBasicBlock>, public SILAllocated<SILBasicBlock>,
 public SwiftObjectHeader {
@@ -184,6 +229,20 @@ public SwiftObjectHeader {
   const_reverse_iterator rbegin() const { return InstList.rbegin(); }
   const_reverse_iterator rend() const { return InstList.rend(); }
 
+  /// Allows deleting instructions while iterating over all instructions of the
+  /// block.
+  ///
+  /// For details see `DeletableInstructionsIterator`.
+  llvm::iterator_range<DeletableInstructionsIterator<iterator>>
+  deletableInstructions() { return {{begin(), end()}, {end(), end()}}; }
+
+  /// Allows deleting instructions while iterating over all instructions of the
+  /// block in reverse order.
+  ///
+  /// For details see `DeletableInstructionsIterator`.
+  llvm::iterator_range<DeletableInstructionsIterator<reverse_iterator>>
+  reverseDeletableInstructions() { return {{rbegin(), rend()}, {rend(), rend()}}; }
+
   TermInst *getTerminator() {
     assert(!InstList.empty() && "Can't get successors for malformed block");
     return cast<TermInst>(&InstList.back());

include/swift/SIL/SILInstruction.h

@@ -49,6 +49,58 @@
 #include "llvm/Support/TrailingObjects.h"
 #include <array>
 
+namespace llvm {
+namespace ilist_detail {
+
+/// The base class of the instruction list in SILBasicBlock.
+///
+/// We need a custom base class to not clear the prev/next pointers when
+/// removing an instruction from the list.
+class SILInstructionListBase : public ilist_base<false> {
+public:
+  /// Remove an instruction from the list.
+  ///
+  /// In contrast to the default implementation, it does not clear the prev/
+  /// next pointers in the node. This is needed to being able to remove
+  /// instructions from the list while iterating over the list.
+  /// For details see `DeletableInstructionsIterator`.
+  template <class T> static void remove(T &N) {
+    node_base_type *Prev = N.getPrev();
+    node_base_type *Next = N.getNext();
+    Next->setPrev(Prev);
+    Prev->setNext(Next);
+  }
+
+  template <class T> static void insertBefore(T &Next, T &N) {
+    insertBeforeImpl(Next, N);
+  }
+
+  template <class T> static void transferBefore(T &Next, T &First, T &Last) {
+    transferBeforeImpl(Next, First, Last);
+  }
+};
+
+// This template specialization is needed to replace the default instruction
+// list base class with `SILInstructionListBase`.
+template <> struct compute_node_options<::swift::SILInstruction> {
+  struct type {
+    typedef ::swift::SILInstruction value_type;
+    typedef value_type *pointer;
+    typedef value_type &reference;
+    typedef const value_type *const_pointer;
+    typedef const value_type &const_reference;
+
+    static const bool enable_sentinel_tracking = false;
+    static const bool is_sentinel_tracking_explicit = false;
+    typedef void tag;
+    typedef ilist_node_base<enable_sentinel_tracking> node_base_type;
+    typedef SILInstructionListBase list_base_type;
+  };
+};
+
+} // end namespace ilist_detail
+} // end llvm namespace
+
 namespace swift {
 
 class AllocationInst;

include/swift/SILOptimizer/Utils/InstructionDeleter.h

@@ -97,7 +97,6 @@
 
 #include "swift/SIL/SILInstruction.h"
 #include "swift/SILOptimizer/Utils/InstModCallbacks.h"
-#include "swift/SILOptimizer/Utils/UpdatingInstructionIterator.h"
 
 namespace swift {
 
@@ -116,29 +115,16 @@ class InstructionDeleter {
   /// instruction's operands. This has to be deterministic.
   SmallSetVector<SILInstruction *, 8> deadInstructions;
 
-  UpdatingInstructionIteratorRegistry iteratorRegistry;
+  /// Callbacks used when adding/deleting instructions.
+  InstModCallbacks callbacks;
 
 public:
-  InstructionDeleter() : deadInstructions(), iteratorRegistry() {}
+  InstructionDeleter() : deadInstructions() {}
 
-  InstructionDeleter(InstModCallbacks &&chainedCallbacks)
-    : deadInstructions(), iteratorRegistry(std::move(chainedCallbacks)) {}
+  InstructionDeleter(InstModCallbacks &&callbacks)
+    : deadInstructions(), callbacks(std::move(callbacks)) {}
 
-  UpdatingInstructionIteratorRegistry &getIteratorRegistry() {
-    return iteratorRegistry;
-  }
-
-  InstModCallbacks &getCallbacks() { return iteratorRegistry.getCallbacks(); }
-
-  llvm::iterator_range<UpdatingInstructionIterator>
-  updatingRange(SILBasicBlock *bb) {
-    return iteratorRegistry.makeIteratorRange(bb);
-  }
-
-  llvm::iterator_range<UpdatingReverseInstructionIterator>
-  updatingReverseRange(SILBasicBlock *bb) {
-    return iteratorRegistry.makeReverseIteratorRange(bb);
-  }
+  InstModCallbacks &getCallbacks() { return callbacks; }
 
   bool hadCallbackInvocation() const {
     return const_cast<InstructionDeleter *>(this)