Merge pull request #62480 from eeckstein/instruction-iteration

SIL: simplify deleting instructions while iterating over instructions.

SwiftCompilerSources/Sources/Optimizer/DataStructures/InstructionRange.swift

@@ -94,7 +94,7 @@ struct InstructionRange : CustomStringConvertible, NoReflectionChildren {
   var isValid: Bool {
     blockRange.isValid &&
     // Check if there are any inserted instructions before the begin instruction in its block.
-    !ReverseList(first: begin).dropFirst().contains { insertedInsts.contains($0) }
+    !ReverseInstructionList(first: begin).dropFirst().contains { insertedInsts.contains($0) }
   }
 
   /// Returns the end instructions.
@@ -115,7 +115,7 @@ struct InstructionRange : CustomStringConvertible, NoReflectionChildren {
   /// Returns the interior instructions.
   var interiors: LazySequence<FlattenSequence<
                    LazyMapSequence<Stack<BasicBlock>,
-                                   LazyFilterSequence<ReverseList<Instruction>>>>> {
+                                   LazyFilterSequence<ReverseInstructionList>>>> {
     blockRange.inserted.lazy.flatMap {
       var include = blockRange.contains($0)
       return $0.instructions.reversed().lazy.filter {

SwiftCompilerSources/Sources/Optimizer/PassManager/PassRegistration.swift

@@ -75,6 +75,7 @@ private func registerSwiftPasses() {
   registerPass(deadEndBlockDumper, { deadEndBlockDumper.run($0) })
   registerPass(rangeDumper, { rangeDumper.run($0) })
   registerPass(runUnitTests, { runUnitTests.run($0) })
+  registerPass(testInstructionIteration, { testInstructionIteration.run($0) })
 }
 
 private func registerSwiftAnalyses() {

SwiftCompilerSources/Sources/Optimizer/TestPasses/CMakeLists.txt

@@ -14,4 +14,5 @@ swift_compiler_sources(Optimizer
   SILPrinter.swift
   RangeDumper.swift
   RunUnitTests.swift
+  TestInstructionIteration.swift
 )

SwiftCompilerSources/Sources/Optimizer/TestPasses/TestInstructionIteration.swift

@@ -0,0 +1,77 @@
+//===--- TestInstructionIteration.swift -----------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2022 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import SIL
+
+/// Tests instruction iteration while modifying the instruction list.
+///
+/// This pass iterates over the instruction list of the function's block and performs
+/// modifications of the instruction list - mostly deleting instructions.
+/// Modifications are triggered by `string_literal` instructions with known "commands".
+/// E.g. if a
+/// ```
+///   %1 = string_literal utf8 "delete_strings"
+/// ```
+/// is encountered during the iteration, it triggers the deletion of all `string_literal`
+/// instructions of the basic block (including the current one).
+let testInstructionIteration = FunctionPass(name: "test-instruction-iteration") {
+  (function: Function, context: PassContext) in
+
+  print("Test instruction iteration in \(function.name):")
+
+  let reverse = function.name.string.hasSuffix("backward")
+
+  for block in function.blocks {
+    print("\(block.name):")
+    let termLoc = block.terminator.location
+    if reverse {
+      for inst in block.instructions.reversed() {
+        handle(instruction: inst, context)
+      }
+    } else {
+      for inst in block.instructions {
+        handle(instruction: inst, context)
+      }
+    }
+    if block.instructions.isEmpty || !(block.instructions.reversed().first is TermInst) {
+      let builder = Builder(atEndOf: block, location: termLoc, context)
+      builder.createUnreachable()
+    }
+  }
+  print("End function \(function.name):")
+}
+
+private func handle(instruction: Instruction, _ context: PassContext) {
+  print(instruction)
+  if let sl = instruction as? StringLiteralInst {
+    switch sl.string {
+      case "delete_strings":
+        deleteAllInstructions(ofType: StringLiteralInst.self, in: instruction.block, context)
+      case "delete_ints":
+        deleteAllInstructions(ofType: IntegerLiteralInst.self, in: instruction.block, context)
+      case "delete_branches":
+        deleteAllInstructions(ofType: BranchInst.self, in: instruction.block, context)
+      case "split_block":
+        _ = context.splitBlock(at: instruction)
+      default:
+        break
+    }
+  }
+}
+
+private func deleteAllInstructions<InstType: Instruction>(ofType: InstType.Type, in block: BasicBlock, _ context: PassContext) {
+  for inst in block.instructions {
+    if inst is InstType {
+      context.erase(instruction: inst)
+    }
+  }
+}

SwiftCompilerSources/Sources/SIL/BasicBlock.swift

@@ -13,15 +13,10 @@
 import Basic
 import SILBridging
 
-final public class BasicBlock : ListNode, CustomStringConvertible, HasShortDescription {
+final public class BasicBlock : CustomStringConvertible, HasShortDescription {
   public var next: BasicBlock? { SILBasicBlock_next(bridged).block }
   public var previous: BasicBlock? { SILBasicBlock_previous(bridged).block }
 
-  // Needed for ReverseList<BasicBlock>.reversed(). Never use directly.
-  public var _firstInList: BasicBlock { SILFunction_firstBlock(function.bridged).block! }
-  // Needed for List<BasicBlock>.reversed(). Never use directly.
-  public var _lastInList: BasicBlock { SILFunction_lastBlock(function.bridged).block! }
-
   public var function: Function { SILBasicBlock_getFunction(bridged).function }
 
   public var description: String {
@@ -32,8 +27,8 @@ final public class BasicBlock : ListNode, CustomStringConvertible, HasShortDescr
 
   public var arguments: ArgumentArray { ArgumentArray(block: self) }
 
-  public var instructions: List<Instruction> {
-    List(first: SILBasicBlock_firstInst(bridged).instruction)
+  public var instructions: InstructionList {
+    InstructionList(first: SILBasicBlock_firstInst(bridged).instruction)
   }
 
   public var terminator: TermInst {
@@ -75,6 +70,66 @@ final public class BasicBlock : ListNode, CustomStringConvertible, HasShortDescr
 public func == (lhs: BasicBlock, rhs: BasicBlock) -> Bool { lhs === rhs }
 public func != (lhs: BasicBlock, rhs: BasicBlock) -> Bool { lhs !== rhs }
 
+/// The list of instructions in a BasicBlock.
+///
+/// It's allowed to delete the current, next or any other instructions while
+/// iterating over the instruction list.
+public struct InstructionList : CollectionLikeSequence, IteratorProtocol {
+  private var currentInstruction: Instruction?
+
+  public init(first: Instruction?) { currentInstruction = first }
+
+  public mutating func next() -> Instruction? {
+    if var inst = currentInstruction {
+      while inst.isDeleted {
+        guard let nextInst = inst.next else {
+          return nil
+        }
+        inst = nextInst
+      }
+      currentInstruction = inst.next
+      return inst
+    }
+    return nil
+  }
+
+  public var first: Instruction? { currentInstruction }
+
+  public func reversed() -> ReverseInstructionList {
+    if let inst = currentInstruction {
+      let lastInst = SILBasicBlock_lastInst(inst.block.bridged).instruction
+      return ReverseInstructionList(first: lastInst)
+    }
+    return ReverseInstructionList(first: nil)
+  }
+}
+
+/// The list of instructions in a BasicBlock in reverse order.
+///
+/// It's allowed to delete the current, next or any other instructions while
+/// iterating over the instruction list.
+public struct ReverseInstructionList : CollectionLikeSequence, IteratorProtocol {
+  private var currentInstruction: Instruction?
+
+  public init(first: Instruction?) { currentInstruction = first }
+
+  public mutating func next() -> Instruction? {
+    if var inst = currentInstruction {
+      while inst.isDeleted {
+        guard let nextInst = inst.previous else {
+          return nil
+        }
+        inst = nextInst
+      }
+      currentInstruction = inst.previous
+      return inst
+    }
+    return nil
+  }
+
+  public var first: Instruction? { currentInstruction }
+}
+
 public struct ArgumentArray : RandomAccessCollection {
   fileprivate let block: BasicBlock
 

SwiftCompilerSources/Sources/SIL/Builder.swift

@@ -187,4 +187,12 @@ public struct Builder {
       return bi.getAs(BranchInst.self)
     }
   }
+
+  @discardableResult
+  public func createUnreachable() -> UnreachableInst {
+    notifyInstructionsChanged()
+    notifyBranchesChanged()
+    let ui = SILBuilder_createUnreachable(bridged)
+    return ui.getAs(UnreachableInst.self)
+  }
 }

SwiftCompilerSources/Sources/SIL/Function.swift

@@ -42,8 +42,8 @@ final public class Function : CustomStringConvertible, HasShortDescription, Hash
     SILFunction_firstBlock(bridged).block!
   }
 
-  public var blocks : List<BasicBlock> {
-    return List(first: SILFunction_firstBlock(bridged).block)
+  public var blocks : BasicBlockList {
+    BasicBlockList(first: SILFunction_firstBlock(bridged).block)
   }
 
   public var arguments: LazyMapSequence<ArgumentArray, FunctionArgument> {
@@ -51,7 +51,7 @@ final public class Function : CustomStringConvertible, HasShortDescription, Hash
   }
 
   /// All instructions of all blocks.
-  public var instructions: LazySequence<FlattenSequence<LazyMapSequence<List<BasicBlock>, List<Instruction>>>> {
+  public var instructions: LazySequence<FlattenSequence<LazyMapSequence<BasicBlockList, InstructionList>>> {
     blocks.lazy.flatMap { $0.instructions }
   }
 
@@ -358,3 +358,43 @@ public extension SideEffects.GlobalEffects {
     }
   }
 }
+
+public struct BasicBlockList : CollectionLikeSequence, IteratorProtocol {
+  private var currentBlock: BasicBlock?
+
+  public init(first: BasicBlock?) { currentBlock = first }
+
+  public mutating func next() -> BasicBlock? {
+    if let block = currentBlock {
+      currentBlock = block.next
+      return block
+    }
+    return nil
+  }
+
+  public var first: BasicBlock? { currentBlock }
+
+  public func reversed() -> ReverseBasicBlockList {
+    if let block = currentBlock {
+      let lastBlock = SILFunction_lastBlock(block.function.bridged).block
+      return ReverseBasicBlockList(first: lastBlock)
+    }
+    return ReverseBasicBlockList(first: nil)
+  }
+}
+
+public struct ReverseBasicBlockList : CollectionLikeSequence, IteratorProtocol {
+  private var currentBlock: BasicBlock?
+
+  public init(first: BasicBlock?) { currentBlock = first }
+
+  public mutating func next() -> BasicBlock? {
+    if let block = currentBlock {
+      currentBlock = block.previous
+      return block
+    }
+    return nil
+  }
+
+  public var first: BasicBlock? { currentBlock }
+}

SwiftCompilerSources/Sources/SIL/Instruction.swift

@@ -17,7 +17,7 @@ import SILBridging
 //                       Instruction base classes
 //===----------------------------------------------------------------------===//
 
-public class Instruction : ListNode, CustomStringConvertible, Hashable {
+public class Instruction : CustomStringConvertible, Hashable {
   final public var next: Instruction? {
     SILInstruction_next(bridged).instruction
   }
@@ -26,11 +26,6 @@ public class Instruction : ListNode, CustomStringConvertible, Hashable {
     SILInstruction_previous(bridged).instruction
   }
 
-  // Needed for ReverseList<Instruction>.reversed(). Never use directly.
-  public var _firstInList: Instruction { SILBasicBlock_firstInst(block.bridged).instruction! }
-  // Needed for List<Instruction>.reversed(). Never use directly.
-  public var _lastInList: Instruction { SILBasicBlock_lastInst(block.bridged).instruction! }
-
   final public var block: BasicBlock {
     SILInstruction_getParent(bridged).block
   }
@@ -42,6 +37,10 @@ public class Instruction : ListNode, CustomStringConvertible, Hashable {
     return String(_cxxString: stdString)
   }
 
+  final public var isDeleted: Bool {
+    return SILInstruction_isDeleted(bridged)
+  }
+
   final public var operands: OperandArray {
     return OperandArray(opArray: SILInstruction_getOperands(bridged))
   }

SwiftCompilerSources/Sources/SIL/Utils.swift

@@ -17,71 +17,6 @@ import SILBridging
 // Otherwise The Optimizer would fall back to Swift's assert implementation.
 @_exported import Basic
 
-//===----------------------------------------------------------------------===//
-//                              Lists
-//===----------------------------------------------------------------------===//
-
-public protocol ListNode : AnyObject {
-  associatedtype Element
-  var next: Element? { get }
-  var previous: Element? { get }
-  
-  /// The first node in the list. Used to implement `reversed()`.
-  var _firstInList: Element { get }
-
-  /// The last node in the list. Used to implement `reversed()`.
-  var _lastInList: Element { get }
-}
-
-public struct List<NodeType: ListNode> :
-      CollectionLikeSequence, IteratorProtocol where NodeType.Element == NodeType {
-  private var currentNode: NodeType?
-  
-  public init(first: NodeType?) { currentNode = first }
-
-  public mutating func next() -> NodeType? {
-    if let node = currentNode {
-      currentNode = node.next
-      return node
-    }
-    return nil
-  }
-
-  public var first: NodeType? { currentNode }
-
-  public func reversed() -> ReverseList<NodeType> {
-    if let node = first {
-      return ReverseList(first: node._lastInList)
-    }
-    return ReverseList(first: nil)
-  }
-}
-
-public struct ReverseList<NodeType: ListNode> :
-      CollectionLikeSequence, IteratorProtocol where NodeType.Element == NodeType {
-  private var currentNode: NodeType?
-  
-  public init(first: NodeType?) { currentNode = first }
-
-  public mutating func next() -> NodeType? {
-    if let node = currentNode {
-      currentNode = node.previous
-      return node
-    }
-    return nil
-  }
-
-  public var first: NodeType? { currentNode }
-
-  public func reversed() -> ReverseList<NodeType> {
-    if let node = first {
-      return ReverseList(first: node._firstInList)
-    }
-    return ReverseList(first: nil)
-  }
-}
-
-
 //===----------------------------------------------------------------------===//
 //                            Sequence Utilities
 //===----------------------------------------------------------------------===//

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 {
@@ -85,7 +130,7 @@ private:
   ///    DD and EEE are uninitialized
   ///
   /// See also: SILBitfield::bitfieldID, SILFunction::currentBitfieldID.
-  uint64_t lastInitializedBitfieldID = 0;
+  int64_t lastInitializedBitfieldID = 0;
 
   // Used by `BasicBlockBitfield`.
   unsigned getCustomBits() const { return customBits; }
@@ -148,9 +193,10 @@ public:
 
   void push_back(SILInstruction *I);
   void push_front(SILInstruction *I);
-  void remove(SILInstruction *I);
   void erase(SILInstruction *I);
   void erase(SILInstruction *I, SILModule &module);
+  static void moveInstruction(SILInstruction *inst, SILInstruction *beforeInst);
+  void moveInstructionToFront(SILInstruction *inst);
 
   void eraseAllInstructions(SILModule &module);
 
@@ -183,6 +229,20 @@ public:
   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/SILBitfield.h

@@ -30,7 +30,7 @@ template <class Impl, class T> class SILBitfield {
   /// that the bits of that block are not initialized yet.
   /// See also: SILBasicBlock::lastInitializedBitfieldID,
   ///           SILFunction::currentBitfieldID
-  uint64_t bitfieldID;
+  int64_t bitfieldID;
 
   short startBit;
   short endBit;

include/swift/SIL/SILBridging.h

@@ -340,6 +340,7 @@ BridgedArgumentConvention SILArgument_getConvention(BridgedArgument argument);
 OptionalBridgedInstruction SILInstruction_next(BridgedInstruction inst);
 OptionalBridgedInstruction SILInstruction_previous(BridgedInstruction inst);
 BridgedBasicBlock SILInstruction_getParent(BridgedInstruction inst);
+bool SILInstruction_isDeleted(BridgedInstruction inst);
 BridgedArrayRef SILInstruction_getOperands(BridgedInstruction inst);
 void SILInstruction_setOperand(BridgedInstruction inst, SwiftInt index,
                                BridgedValue value);
@@ -456,6 +457,7 @@ BridgedInstruction SILBuilder_createUncheckedEnumData(BridgedBuilder builder,
 BridgedInstruction SILBuilder_createBranch(
           BridgedBuilder builder, BridgedBasicBlock destBlock,
           BridgedValueArray arguments);
+BridgedInstruction SILBuilder_createUnreachable(BridgedBuilder builder);
 
 SWIFT_END_NULLABILITY_ANNOTATIONS
 

include/swift/SIL/SILFunction.h

@@ -266,7 +266,7 @@ private:
   /// A monotonically increasing ID which is incremented whenever a
   /// BasicBlockBitfield or NodeBitfield is constructed.
   /// For details see SILBitfield::bitfieldID;
-  uint64_t currentBitfieldID = 1;
+  int64_t currentBitfieldID = 1;
 
   /// Unique identifier for vector indexing and deterministic sorting.
   /// May be reused when zombie functions are recovered.

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;
@@ -376,9 +428,6 @@ protected:
     NumCreatedInstructions++;
   }
 
-  /// This method unlinks 'self' from the containing basic block.
-  void removeFromParent();
-
   ~SILInstruction() {
     NumDeletedInstructions++;
   }
@@ -394,7 +443,7 @@ public:
 
   /// Returns true if this instruction is removed from its function and
   /// scheduled to be deleted.
-  bool isDeleted() const { return !ParentBB; }
+  bool isDeleted() const { return asSILNode()->isMarkedAsDeleted(); }
 
   enum class MemoryBehavior {
     None,
@@ -10175,7 +10224,6 @@ public:
   }
 
   void addNodeToList(SILInstruction *I);
-  void removeNodeFromList(SILInstruction *I);
   void transferNodesFromList(ilist_traits<SILInstruction> &L2,
                              instr_iterator first, instr_iterator last);
 

include/swift/SIL/SILModule.h

@@ -217,7 +217,7 @@ private:
   /// This avoids dangling instruction pointers within the run of a pass and in
   /// analysis caches. Note that the analysis invalidation mechanism ensures
   /// that analysis caches are invalidated before flushDeletedInsts().
-  llvm::iplist<SILInstruction> scheduledForDeletion;
+  std::vector<SILInstruction*> scheduledForDeletion;
 
   /// The swift Module associated with this SILModule.
   ModuleDecl *TheSwiftModule;

include/swift/SIL/SILNode.h

@@ -330,8 +330,13 @@ protected:
   /// -> AAA, BB and C are initialized,
   ///    DD and EEE are uninitialized
   ///
+  /// If the ID is negative, it means that the node (in case it's an instruction)
+  /// is deleted, i.e. it does not belong to the function anymore. Conceptually
+  /// this results in setting all bitfields to zero, which e.g. "removes" the
+  /// node from all NodeSets.
+  ///
   /// See also: SILBitfield::bitfieldID, SILFunction::currentBitfieldID.
-  uint64_t lastInitializedBitfieldID = 0;
+  int64_t lastInitializedBitfieldID = 0;
 
 private:
   SwiftMetatype getSILNodeMetatype(SILNodeKind kind);
@@ -389,6 +394,12 @@ public:
     lastInitializedBitfieldID = 0;
   }
 
+  void markAsDeleted() {
+    lastInitializedBitfieldID = -1;
+  }
+
+  bool isMarkedAsDeleted() const { return lastInitializedBitfieldID < 0; }
+
   static SILNode *instAsNode(SILInstruction *inst);
   static const SILNode *instAsNode(const SILInstruction *inst);
 

include/swift/SILOptimizer/PassManager/Passes.def

@@ -226,6 +226,8 @@ SWIFT_FUNCTION_PASS(ComputeEscapeEffects, "compute-escape-effects",
      "Computes function escape effects")
 SWIFT_FUNCTION_PASS(ComputeSideEffects, "compute-side-effects",
      "Computes function side effects")
+SWIFT_FUNCTION_PASS(TestInstructionIteration, "test-instruction-iteration",
+     "Tests instruction iteration")
 PASS(FlowIsolation, "flow-isolation",
      "Enforces flow-sensitive actor isolation rules")
 PASS(FunctionOrderPrinter, "function-order-printer",

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)

include/swift/SILOptimizer/Utils/UpdatingInstructionIterator.h

@@ -1,293 +0,0 @@
-//===--- UpdatingInstructionIterator.h --------------------------*- C++ -*-===//
-//
-// This source file is part of the Swift.org open source project
-//
-// Copyright (c) 2014 - 2021 Apple Inc. and the Swift project authors
-// Licensed under Apache License v2.0 with Runtime Library Exception
-//
-// See https://swift.org/LICENSE.txt for license information
-// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
-//
-//===----------------------------------------------------------------------===//
-///
-/// Classes for tracking instruction iterators to be updated when instructions
-/// are added or deleted.
-///
-/// UpdatingInstructionIteratorRegistry typically resides in the currently
-/// active InstructionDeleter object.
-///
-/// UpdatingListIterator is the iterator adapter.
-/// It is produced by: UpdatingInstructionIteratorRegistry::makeIterator()
-///
-/// Iterators are typically encapsulated in a range returned by
-/// UpdatingInstructionIteratorRegistry::iteratorRange() for use in
-/// range-based for loops:
-///
-///   for (SILInstruction *inst : registry.iteratorRange(bb)) ...
-///
-/// Or more commonly, directly from the InstructionDeleter:
-///
-///   for (SILInstruction *inst : deleter.updatingRange(bb)) ...
-///
-//===----------------------------------------------------------------------===//
-
-#ifndef SWIFT_SILOPTIMIZER_UTILS_UPDATINGINSTRUCTIONITERATOR_H
-#define SWIFT_SILOPTIMIZER_UTILS_UPDATINGINSTRUCTIONITERATOR_H
-
-#include "swift/SIL/SILBasicBlock.h"
-#include "swift/SIL/SILInstruction.h"
-
-namespace swift {
-
-/// Safely iterate over list elements while deleting elements and inserting new
-/// elements. Advancing the iterator moves to the element following most
-/// recently visited element. This holds even if the most recent element has
-/// been deleted and new instructions have been added after its original
-/// position.
-///
-/// Iterator copies are expensive because each copy registers
-/// itself in a central data structure. Postorder increment is currently
-/// unavailable to avoid excessive copies.
-///
-/// This adapter converts the base iterator's value_type into a pointer to the
-/// same type. When the element has been deleted, dereferencing the iterator
-/// returns a nullptr. This works with any bidirectional base iterator in which
-/// the storage of each element is stable. Typically an llvm::ilist. The
-/// implementation assumes that both forward and reverse iterators point to the
-/// storage of the current element.
-template <typename IteratorBase, bool IsReverse, typename Registry>
-class UpdatingListIterator {
-  using Self = UpdatingListIterator<IteratorBase, IsReverse, Registry>;
-
-  Registry *registry;
-  IteratorBase base;
-  bool isDeleted = false;
-
-public:
-  using value_type = typename IteratorBase::pointer;
-  using difference_type = ptrdiff_t;
-  using pointer = value_type *;
-  using iterator_category = std::bidirectional_iterator_tag;
-
-  UpdatingListIterator(Registry &registry, IteratorBase base)
-      : registry(&registry), base(base) {
-    registry.registerIterator(this);
-  }
-
-  ~UpdatingListIterator() {
-    if (registry)
-      registry->destroyIterator(this);
-  }
-
-  // Copying forces a new entry in the registry.
-  UpdatingListIterator(const Self &rhs)
-      : registry(rhs.registry), base(rhs.base), isDeleted(rhs.isDeleted) {
-    registry->registerIterator(this);
-  }
-
-  Self &operator=(const Self &rhs) {
-    this->registry = rhs.registry;
-    this->base = rhs.base;
-    this->advanced = rhs.advanced;
-    registry->registerIterator(this);
-    return *this;
-  }
-
-  /// Explicit conversion between forward/reverse iterators.
-  ///
-  /// Note that this does not produce the same result as getting the iterator's
-  /// instruction and producing a new iterator in the opposite
-  /// direction. Instead, the new iterator points to the previous instruction in
-  /// the original iteration order. This ensures that forward and reverse ranges
-  /// enclose the same set of instructions.
-  template <typename OtherIteratorBase>
-  explicit UpdatingListIterator(
-      const UpdatingListIterator<OtherIteratorBase, !IsReverse, Registry> &rhs)
-      : UpdatingListIterator(IteratorBase(rhs.base)) {}
-
-  /// This returns nullptr for deleted instructions.
-  value_type operator*() const { return isDeleted ? nullptr : &*base; }
-
-  SILInstruction *operator->() const { return operator*(); }
-
-  Self &operator++() {
-    advance();
-    return *this;
-  }
-
-  bool operator==(const Self &rhs) const {
-    return this->registry == rhs.registry && this->base == rhs.base
-           && this->isDeleted == rhs.isDeleted;
-  }
-  bool operator!=(const Self &rhs) const { return !(*this == rhs); }
-
-  void advance() {
-    if (isDeleted)
-      isDeleted = false;
-    else
-      ++base;
-  }
-
-  void notifyDelete(IteratorBase positionToDelete) {
-    if (base == positionToDelete) {
-      isDeleted = true;
-      ++base;
-    }
-  }
-
-  void notifyNew(IteratorBase newPosition) {
-    if (isDeleted && std::prev(base) == newPosition) {
-      // The deleted forward iterator was already advanced. Move it back to the
-      // position of the new element.
-      --base;
-    }
-  }
-};
-
-class UpdatingInstructionIteratorRegistry;
-
-using UpdatingInstructionIterator =
-    UpdatingListIterator<SILBasicBlock::iterator, false,
-                         UpdatingInstructionIteratorRegistry>;
-using UpdatingReverseInstructionIterator =
-    UpdatingListIterator<SILBasicBlock::reverse_iterator, true,
-                         UpdatingInstructionIteratorRegistry>;
-
-/// Track instruction iterators that need updating when instructions are added or
-/// deleted. Iterators can be tracked across multiple levels of the call
-/// stack. This registry object must outlive any iterators that it vends.
-///
-/// While the registry is active, all instruction modification must go through
-/// its callbacks.
-class UpdatingInstructionIteratorRegistry {
-
-  /// Track iterators that need updating. Seldom expect to have more than 4
-  /// (making a single range creates 4 but immediately discards 2). It is
-  /// possible for iterators to be copied and destroyed on each iteration of a
-  /// loop (although we should try hard to avoid that), so this does need to
-  /// immediately reuse old slots.
-  SmallVector<UpdatingInstructionIterator *, 4> forwardIterators;
-  SmallVector<UpdatingReverseInstructionIterator *, 4> reverseIterators;
-
-  std::function<void(SILInstruction *)> chainedDelete;
-  std::function<void(SILInstruction *)> chainedNew;
-
-  /// Callbacks used when adding/deleting instructions.
-  InstModCallbacks callbacks;
-
-
-public:
-  UpdatingInstructionIteratorRegistry() {
-    callbacks = std::move(InstModCallbacks()
-                    .onDelete([this](SILInstruction *toDelete) {
-                      notifyDelete(toDelete);
-                      toDelete->eraseFromParent();
-                    })
-                    .onCreateNewInst(
-                        [this](SILInstruction *newlyCreatedInst) {
-                          notifyNew(newlyCreatedInst);
-                        }));
-  }
-
-  UpdatingInstructionIteratorRegistry(InstModCallbacks &&chainedCallbacks) :
-    // Copy the two std::functions that we need. The rest of the callbacks are
-    // copied implicitly by assignment.
-    chainedDelete(std::move(chainedCallbacks.deleteInstFunc)),
-    chainedNew(std::move(chainedCallbacks.createdNewInstFunc)) {
-    callbacks = std::move(chainedCallbacks
-                    .onDelete([this](SILInstruction *toDelete) {
-                      notifyDelete(toDelete);
-                      if (chainedDelete) {
-                        chainedDelete(toDelete);
-                        return;
-                      }
-                      toDelete->eraseFromParent();
-                    })
-                    .onCreateNewInst(
-                        [this](SILInstruction *newlyCreatedInst) {
-                          notifyNew(newlyCreatedInst);
-                          if (chainedNew) {
-                            chainedNew(newlyCreatedInst);
-                          }
-                        }));
-  }
-
-  // The callbacks capture 'this'. So copying is invalid.
-  UpdatingInstructionIteratorRegistry(
-      const UpdatingInstructionIteratorRegistry &) = delete;
-
-  UpdatingInstructionIteratorRegistry &
-  operator=(const UpdatingInstructionIteratorRegistry &) = delete;
-
-  InstModCallbacks &getCallbacks() { return callbacks; }
-
-  void registerIterator(UpdatingInstructionIterator *i) {
-    forwardIterators.push_back(i);
-  }
-
-  void registerIterator(UpdatingReverseInstructionIterator *i) {
-    reverseIterators.push_back(i);
-  }
-
-  void destroyIterator(UpdatingInstructionIterator *i) {
-    auto pos = std::find(forwardIterators.begin(), forwardIterators.end(), i);
-    assert(pos != forwardIterators.end() && "unregistered iterator");
-    forwardIterators.erase(pos);
-  }
-  void destroyIterator(UpdatingReverseInstructionIterator *i) {
-    auto pos = std::find(reverseIterators.begin(), reverseIterators.end(), i);
-    assert(pos != reverseIterators.end() && "unregistered iterator");
-    reverseIterators.erase(pos);
-  }
-
-  UpdatingInstructionIterator makeIterator(SILBasicBlock::iterator i) {
-    return {*this, i};
-  }
-
-  UpdatingInstructionIterator makeIterator(SILInstruction *inst) {
-    return makeIterator(inst->getIterator());
-  }
-
-  UpdatingReverseInstructionIterator
-  makeReverseIterator(SILBasicBlock::reverse_iterator i) {
-    return {*this, i};
-  }
-
-  UpdatingReverseInstructionIterator
-  makeReverseIterator(SILInstruction *inst) {
-    return makeReverseIterator(inst->getReverseIterator());
-  }
-
-  llvm::iterator_range<UpdatingInstructionIterator>
-  makeIteratorRange(SILBasicBlock *bb) {
-    return {makeIterator(bb->begin()), makeIterator(bb->end())};
-  }
-
-  llvm::iterator_range<UpdatingReverseInstructionIterator>
-  makeReverseIteratorRange(SILBasicBlock *bb) {
-    return {makeReverseIterator(bb->rbegin()), makeReverseIterator(bb->rend())};
-  }
-
-protected:
-  void notifyDelete(SILInstruction *toDelete) {
-    for (auto *iterator : forwardIterators) {
-      iterator->notifyDelete(toDelete->getIterator());
-    }
-    for (auto *iterator : reverseIterators) {
-      iterator->notifyDelete(toDelete->getReverseIterator());
-    }
-  }
-
-  void notifyNew(SILInstruction *newlyCreatedInst) {
-    for (auto *iterator : forwardIterators) {
-      iterator->notifyNew(newlyCreatedInst->getIterator());
-    }
-    for (auto *iterator : reverseIterators) {
-      iterator->notifyNew(newlyCreatedInst->getReverseIterator());
-    }
-  }
-};
-
-} // end namespace swift
-
-#endif

lib/SIL/IR/SILBasicBlock.cpp

@@ -91,10 +91,6 @@ void SILBasicBlock::push_front(SILInstruction *I) {
   InstList.push_front(I);
 }
 
-void SILBasicBlock::remove(SILInstruction *I) {
-  InstList.remove(I);
-}
-
 void SILBasicBlock::eraseAllInstructions(SILModule &module) {
   while (!empty()) {
     erase(&*begin(), module);
@@ -107,11 +103,26 @@ void SILBasicBlock::erase(SILInstruction *I) {
 }
 
 void SILBasicBlock::erase(SILInstruction *I, SILModule &module) {
+  assert(!I->isDeleted() && "double delete of instruction");
   module.willDeleteInstruction(I);
   InstList.remove(I);
+  I->asSILNode()->markAsDeleted();
   module.scheduleForDeletion(I);
 }
 
+void SILBasicBlock::moveInstruction(SILInstruction *inst,
+                                    SILInstruction *beforeInst) {
+  if (inst == beforeInst)
+    return;
+  inst->getParent()->InstList.remove(inst);
+  beforeInst->getParent()->insert(beforeInst->getIterator(), inst);
+}
+
+void SILBasicBlock::moveInstructionToFront(SILInstruction *inst) {
+  inst->getParent()->InstList.remove(inst);
+  push_front(inst);
+}
+
 /// This method unlinks 'self' from the containing SILFunction and deletes it.
 void SILBasicBlock::eraseFromParent() {
   getParent()->eraseBlock(this);

lib/SIL/IR/SILInstruction.cpp

@@ -61,15 +61,9 @@ SILBasicBlock *llvm::ilist_traits<SILInstruction>::getContainingBlock() {
 
 
 void llvm::ilist_traits<SILInstruction>::addNodeToList(SILInstruction *I) {
-  assert(I->ParentBB == nullptr && "Already in a list!");
   I->ParentBB = getContainingBlock();
 }
 
-void llvm::ilist_traits<SILInstruction>::removeNodeFromList(SILInstruction *I) {
-  // When an instruction is removed from a BB, clear the parent pointer.
-  I->ParentBB = nullptr;
-}
-
 void llvm::ilist_traits<SILInstruction>::
 transferNodesFromList(llvm::ilist_traits<SILInstruction> &L2,
                       instr_iterator first, instr_iterator last) {
@@ -103,16 +97,6 @@ transferNodesFromList(llvm::ilist_traits<SILInstruction> &L2,
   ASSERT_IMPLEMENTS_STATIC(CLASS, PARENT, classof, bool(SILNodePointer));
 #include "swift/SIL/SILNodes.def"
 
-void SILInstruction::removeFromParent() {
-#ifndef NDEBUG
-  for (auto result : getResults()) {
-    assert(result->use_empty() && "Uses of SILInstruction remain at deletion.");
-  }
-#endif
-  getParent()->remove(this);
-  ParentBB = nullptr;
-}
-
 /// eraseFromParent - This method unlinks 'self' from the containing basic
 /// block and deletes it.
 ///
@@ -126,18 +110,13 @@ void SILInstruction::eraseFromParent() {
 }
 
 void SILInstruction::moveFront(SILBasicBlock *Block) {
-  getParent()->remove(this);
-  Block->push_front(this);
+  Block->moveInstructionToFront(this);
 }
 
 /// Unlink this instruction from its current basic block and insert it into
 /// the basic block that Later lives in, right before Later.
 void SILInstruction::moveBefore(SILInstruction *Later) {
-  if (this == Later)
-    return;
-
-  getParent()->remove(this);
-  Later->getParent()->insert(Later, this);
+  SILBasicBlock::moveInstruction(this, Later);
 }
 
 /// Unlink this instruction from its current basic block and insert it into

lib/SIL/IR/SILModule.cpp

@@ -164,8 +164,8 @@ void SILModule::checkForLeaks() const {
   if (!getOptions().checkSILModuleLeaks)
     return;
 
-  int instsInModule = std::distance(scheduledForDeletion.begin(),
-                                    scheduledForDeletion.end());
+  int instsInModule = scheduledForDeletion.size();
+
   for (const SILFunction &F : *this) {
     const SILFunction *sn = &F;
     do {
@@ -264,8 +264,10 @@ void SILModule::willDeleteInstruction(SILInstruction *I) {
     if (const CanOpenedArchetypeType archeTy =
             svi->getDefinedOpenedArchetype()) {
       OpenedArchetypeKey key = {archeTy, svi->getFunction()};
-      assert(RootOpenedArchetypeDefs.lookup(key) == svi &&
-             "archetype def was not registered");
+      // In case `willDeleteInstruction` is called twice for the same instruction,
+      // we need to check if the archetype is really still in the map for this
+      // instruction.
+      if (RootOpenedArchetypeDefs.lookup(key) == svi)
         RootOpenedArchetypeDefs.erase(key);
     }
   }
@@ -274,15 +276,14 @@ void SILModule::willDeleteInstruction(SILInstruction *I) {
 void SILModule::scheduleForDeletion(SILInstruction *I) {
   I->dropAllReferences();
   scheduledForDeletion.push_back(I);
-  I->ParentBB = nullptr;
 }
 
 void SILModule::flushDeletedInsts() {
-  while (!scheduledForDeletion.empty()) {
-    SILInstruction *inst = &*scheduledForDeletion.begin();
-    scheduledForDeletion.erase(inst);
-    AlignedFree(inst);
+  for (SILInstruction *instToDelete : scheduledForDeletion) {
+    SILInstruction::destroy(instToDelete);
+    AlignedFree(instToDelete);
   }
+  scheduledForDeletion.clear();
 }
 
 SILWitnessTable *

lib/SIL/Utils/SILBridging.cpp

@@ -743,6 +743,10 @@ BridgedBasicBlock SILInstruction_getParent(BridgedInstruction inst) {
   return {i->getParent()};
 }
 
+bool SILInstruction_isDeleted(BridgedInstruction inst) {
+  return castToInst(inst)->isDeleted();
+}
+
 BridgedArrayRef SILInstruction_getOperands(BridgedInstruction inst) {
   auto operands = castToInst(inst)->getAllOperands();
   return {(const unsigned char *)operands.data(), operands.size()};
@@ -1184,3 +1188,9 @@ BridgedInstruction SILBuilder_createBranch(
                                castToBasicBlock(destBlock),
                                getSILValues(arguments, argValues))};
 }
+
+BridgedInstruction SILBuilder_createUnreachable(BridgedBuilder b) {
+  SILBuilder builder(castToInst(b.insertBefore), castToBasicBlock(b.insertAtEnd),
+                     b.loc.getScope());
+  return {builder.createUnreachable(RegularLocation(b.loc.getLocation()))};
+}

lib/SILOptimizer/IRGenTransforms/TargetConstantFolding.cpp

@@ -72,9 +72,9 @@ private:
       for (SILBasicBlock &block : function) {
         InstructionDeleter deleter;
 
-        for (SILInstruction *inst : deleter.updatingRange(&block)) {
-          if (constFold(inst, IGM)) {
-            deleter.forceDelete(inst);
+        for (SILInstruction &inst : block.deletableInstructions()) {
+          if (constFold(&inst, IGM)) {
+            deleter.forceDelete(&inst);
             changed = true;
           }
         }

lib/SILOptimizer/Mandatory/AddressLowering.cpp

@@ -3895,9 +3895,9 @@ static void deleteRewrittenInstructions(AddressLoweringState &pass) {
       continue;
     }
     // willDeleteInstruction was already called for open_existential_value to
-    // update the registered type. Carry out the remaining deletion steps.
-    deadInst->getParent()->remove(deadInst);
-    pass.getModule()->scheduleForDeletion(deadInst);
+    // update the registered type. Now fully erase the instruction, which will
+    // harmlessly call willDeleteInstruction again.
+    deadInst->getParent()->erase(deadInst);
   }
 
   pass.valueStorageMap.clear();

lib/SILOptimizer/Mandatory/ClosureLifetimeFixup.cpp

@@ -1004,9 +1004,9 @@ static bool fixupClosureLifetimes(SILFunction &fn, bool &checkStackNesting,
   for (auto &block : fn) {
     SILSSAUpdater updater;
 
-    for (SILInstruction *inst : updater.getDeleter().updatingRange(&block)) {
+    for (SILInstruction &inst : block.deletableInstructions()) {
       // Handle, copy_block_without_escaping instructions.
-      if (auto *cb = dyn_cast<CopyBlockWithoutEscapingInst>(inst)) {
+      if (auto *cb = dyn_cast<CopyBlockWithoutEscapingInst>(&inst)) {
         if (fixupCopyBlockWithoutEscaping(cb, updater.getDeleter(), modifiedCFG)) {
           changed = true;
         }
@@ -1015,7 +1015,7 @@ static bool fixupClosureLifetimes(SILFunction &fn, bool &checkStackNesting,
 
       // Otherwise, look at convert_escape_to_noescape [not_guaranteed]
       // instructions.
-      auto *cvt = dyn_cast<ConvertEscapeToNoEscapeInst>(inst);
+      auto *cvt = dyn_cast<ConvertEscapeToNoEscapeInst>(&inst);
       if (!cvt || cvt->isLifetimeGuaranteed())
         continue;
 

lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp

@@ -2856,9 +2856,9 @@ bool swift::optimizeMemoryAccesses(SILFunction &fn) {
 
   InstructionDeleter deleter;
   for (auto &bb : fn) {
-    for (SILInstruction *inst : deleter.updatingRange(&bb)) {
+    for (SILInstruction &inst : bb.deletableInstructions()) {
       // First see if i is an allocation that we can optimize. If not, skip it.
-      AllocationInst *alloc = getOptimizableAllocation(inst);
+      AllocationInst *alloc = getOptimizableAllocation(&inst);
       if (!alloc) {
         continue;
       }
@@ -2900,9 +2900,9 @@ bool swift::eliminateDeadAllocations(SILFunction &fn) {
 
   for (auto &bb : fn) {
     InstructionDeleter deleter;
-    for (SILInstruction *inst : deleter.updatingRange(&bb)) {
+    for (SILInstruction &inst : bb.deletableInstructions()) {
       // First see if i is an allocation that we can optimize. If not, skip it.
-      AllocationInst *alloc = getOptimizableAllocation(inst);
+      AllocationInst *alloc = getOptimizableAllocation(&inst);
       if (!alloc) {
         continue;
       }

lib/SILOptimizer/Transforms/DeadObjectElimination.cpp

@@ -752,16 +752,16 @@ class DeadObjectElimination : public SILFunctionTransform {
 
     for (auto &BB : Fn) {
 
-      for (SILInstruction *inst : deleter.updatingRange(&BB)) {
-        if (auto *A = dyn_cast<AllocRefInstBase>(inst))
+      for (SILInstruction &inst : BB.deletableInstructions()) {
+        if (auto *A = dyn_cast<AllocRefInstBase>(&inst))
           Changed |= processAllocRef(A);
-        else if (auto *A = dyn_cast<AllocStackInst>(inst))
+        else if (auto *A = dyn_cast<AllocStackInst>(&inst))
           Changed |= processAllocStack(A);
-        else if (auto *KPI = dyn_cast<KeyPathInst>(inst))
+        else if (auto *KPI = dyn_cast<KeyPathInst>(&inst))
           Changed |= processKeyPath(KPI);
-        else if (auto *A = dyn_cast<AllocBoxInst>(inst))
+        else if (auto *A = dyn_cast<AllocBoxInst>(&inst))
           Changed |= processAllocBox(A);
-        else if (auto *A = dyn_cast<ApplyInst>(inst))
+        else if (auto *A = dyn_cast<ApplyInst>(&inst))
           Changed |= processAllocApply(A, DEBlocks);
       }
       deleter.cleanupDeadInstructions();

lib/SILOptimizer/Transforms/GenericSpecializer.cpp

@@ -291,8 +291,8 @@ bool MandatoryGenericSpecializer::optimize(SILFunction *func,
     if (!rrBlocks.reachesReturn(&block) || !neBlocks.isNonErrorHandling(&block))
       continue;
   
-    for (SILInstruction *inst : deleter.updatingReverseRange(&block)) {
-      changed |= optimizeInst(inst, funcBuilder, deleter, cha, invalidatedStackNesting);
+    for (SILInstruction &inst : block.reverseDeletableInstructions()) {
+      changed |= optimizeInst(&inst, funcBuilder, deleter, cha, invalidatedStackNesting);
     }
   }
   deleter.cleanupDeadInstructions();

lib/SILOptimizer/Transforms/SILMem2Reg.cpp

@@ -1994,8 +1994,8 @@ bool MemoryToRegisters::run() {
     if (!domInfo->isReachableFromEntry(&block)) {
       continue;
     }
-    for (SILInstruction *inst : deleter.updatingReverseRange(&block)) {
-      auto *asi = dyn_cast<AllocStackInst>(inst);
+    for (SILInstruction &inst : block.reverseDeletableInstructions()) {
+      auto *asi = dyn_cast<AllocStackInst>(&inst);
       if (!asi)
         continue;
 

lib/SILOptimizer/UtilityPasses/OSSALifetimeAnalysis.cpp

@@ -108,8 +108,8 @@ void OSSALifetimeAnalysis::run() {
     SmallVector<SILValue, 8> traceValues;
     InstructionDeleter deleter;
     for (auto &block : function) {
-      for (SILInstruction *inst : deleter.updatingRange(&block)) {
-        if (auto *debugValue = dyn_cast<DebugValueInst>(inst)) {
+      for (SILInstruction &inst : block.deletableInstructions()) {
+        if (auto *debugValue = dyn_cast<DebugValueInst>(&inst)) {
           if (!debugValue->hasTrace())
             continue;
           traceValues.push_back(debugValue->getOperand());

lib/SILOptimizer/UtilityPasses/ParseTestSpecification.cpp

@@ -29,8 +29,8 @@ void findAndDeleteTraceValues(SILFunction *function,
                               SmallVectorImpl<SILValue> &values) {
   InstructionDeleter deleter;
   for (auto &block : *function) {
-    for (SILInstruction *inst : deleter.updatingRange(&block)) {
-      if (auto *debugValue = dyn_cast<DebugValueInst>(inst)) {
+    for (SILInstruction &inst : block.deletableInstructions()) {
+      if (auto *debugValue = dyn_cast<DebugValueInst>(&inst)) {
         if (!debugValue->hasTrace())
           continue;
         values.push_back(debugValue->getOperand());
@@ -591,8 +591,8 @@ void swift::test::getTestSpecifications(
     SmallVectorImpl<UnparsedSpecification> &specifications) {
   InstructionDeleter deleter;
   for (auto &block : *function) {
-    for (SILInstruction *inst : deleter.updatingRange(&block)) {
-      if (auto *tsi = dyn_cast<TestSpecificationInst>(inst)) {
+    for (SILInstruction &inst : block.deletableInstructions()) {
+      if (auto *tsi = dyn_cast<TestSpecificationInst>(&inst)) {
         auto ref = tsi->getArgumentsSpecification();
         auto *anchor = findAnchorInstructionAfter(tsi);
         specifications.push_back({std::string(ref.begin(), ref.end()), anchor});

test/SIL/instruction_iteration.sil

@@ -0,0 +1,233 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-sil-opt %s -test-instruction-iteration -o %t/out.sil | %FileCheck --check-prefix=LOG %s
+// RUN: %FileCheck %s < %t/out.sil
+
+// REQUIRES: swift_in_compiler
+
+sil_stage canonical
+
+import Builtin
+
+// LOG-LABEL: Test instruction iteration in delete_current_forward:
+// LOG-NEXT:  bb0:
+// LOG-NEXT:    %0 = tuple ()
+// LOG-NEXT:    %1 = string_literal utf8 "delete_strings"
+// LOG-NEXT:    %1 = integer_literal $Builtin.Int64, 3
+// LOG-NEXT:    return %0 : $()
+// LOG-NEXT:  End function delete_current_forward:
+
+// CHECK-LABEL: sil @delete_current_forward : $@convention(thin) () -> () {
+// CHECK-NEXT:  bb0:
+// CHECK-NEXT:    %0 = tuple ()
+// CHECK-NEXT:    %1 = integer_literal $Builtin.Int64, 3
+// CHECK-NEXT:    return %0 : $()
+// CHECK-NEXT:  } // end sil function 'delete_current_forward'
+sil @delete_current_forward : $@convention(thin) () -> () {
+bb0:
+  %0 = tuple ()
+  %1 = string_literal utf8 "delete_strings"
+  %2 = integer_literal $Builtin.Int64, 3 
+  return %0 : $()
+}
+
+// LOG-LABEL: Test instruction iteration in delete_next_forward:
+// LOG-NEXT:  bb0:
+// LOG-NEXT:    %0 = tuple ()
+// LOG-NEXT:    %1 = string_literal utf8 "delete_ints"
+// LOG-NEXT:    return %0 : $()
+// LOG-NEXT:  End function delete_next_forward:
+
+// CHECK-LABEL: sil @delete_next_forward : $@convention(thin) () -> () {
+// CHECK-NEXT:  bb0:
+// CHECK-NEXT:    %0 = tuple ()
+// CHECK-NEXT:    %1 = string_literal utf8 "delete_ints"
+// CHECK-NEXT:    return %0 : $()
+// CHECK-NEXT:  } // end sil function 'delete_next_forward'
+sil @delete_next_forward : $@convention(thin) () -> () {
+bb0:
+  %0 = tuple ()
+  %1 = string_literal utf8 "delete_ints"
+  %2 = integer_literal $Builtin.Int64, 3 
+  return %0 : $()
+}
+
+// LOG-LABEL: Test instruction iteration in delete_current_and_next_forward:
+// LOG-NEXT:  bb0:
+// LOG-NEXT:    %0 = tuple ()
+// LOG-NEXT:    %1 = string_literal utf8 "delete_strings"
+// LOG-NEXT:    return %0 : $()
+// LOG-NEXT:  End function delete_current_and_next_forward:
+
+// CHECK-LABEL: sil @delete_current_and_next_forward : $@convention(thin) () -> () {
+// CHECK-NEXT:  bb0:
+// CHECK-NEXT:    %0 = tuple ()
+// CHECK-NEXT:    return %0 : $()
+// CHECK-NEXT:  } // end sil function 'delete_current_and_next_forward'
+sil @delete_current_and_next_forward : $@convention(thin) () -> () {
+bb0:
+  %0 = tuple ()
+  %1 = string_literal utf8 "delete_strings"
+  %2 = string_literal utf8 ""
+  return %0 : $()
+}
+
+// LOG-LABEL: Test instruction iteration in delete_until_end_forward:
+// LOG-NEXT:  bb0:
+// LOG-NEXT:    %0 = tuple ()
+// LOG-NEXT:    %1 = string_literal utf8 "delete_branches"
+// LOG-NEXT:    %2 = string_literal utf8 "delete_strings"
+// LOG-NEXT:  bb1:
+// LOG-NEXT:    return %0 : $()
+// LOG-NEXT:  End function delete_until_end_forward:
+
+// CHECK-LABEL: sil @delete_until_end_forward : $@convention(thin) () -> () {
+// CHECK-NEXT:  bb0:
+// CHECK-NEXT:    %0 = tuple ()
+// CHECK-NEXT:    unreachable
+// CHECK:       } // end sil function 'delete_until_end_forward'
+sil @delete_until_end_forward : $@convention(thin) () -> () {
+bb0:
+  %0 = tuple ()
+  %1 = string_literal utf8 "delete_branches"
+  %2 = string_literal utf8 "delete_strings"
+  %3 = string_literal utf8 ""
+  %4 = string_literal utf8 ""
+  br bb1
+bb1:
+  return %0 : $()
+}
+
+// LOG-LABEL: Test instruction iteration in delete_current_backward:
+// LOG-NEXT:  bb0:
+// LOG-NEXT:    return %0 : $()
+// LOG-NEXT:    %2 = string_literal utf8 "delete_strings"
+// LOG-NEXT:    %1 = integer_literal $Builtin.Int64, 3
+// LOG-NEXT:    %0 = tuple ()
+// LOG-NEXT:  End function delete_current_backward:
+
+// CHECK-LABEL: sil @delete_current_backward : $@convention(thin) () -> () {
+// CHECK-NEXT:  bb0:
+// CHECK-NEXT:    %0 = tuple ()
+// CHECK-NEXT:    %1 = integer_literal $Builtin.Int64, 3
+// CHECK-NEXT:    return %0 : $()
+// CHECK-NEXT:  } // end sil function 'delete_current_backward'
+sil @delete_current_backward : $@convention(thin) () -> () {
+bb0:
+  %0 = tuple ()
+  %1 = integer_literal $Builtin.Int64, 3 
+  %2 = string_literal utf8 "delete_strings"
+  return %0 : $()
+}
+
+// LOG-LABEL: Test instruction iteration in delete_next_backward:
+// LOG-NEXT:  bb0:
+// LOG-NEXT:    return %0 : $()
+// LOG-NEXT:    %2 = string_literal utf8 "delete_ints"
+// LOG-NEXT:    %0 = tuple ()
+// LOG-NEXT:  End function delete_next_backward:
+
+// CHECK-LABEL: sil @delete_next_backward : $@convention(thin) () -> () {
+// CHECK-NEXT:  bb0:
+// CHECK-NEXT:    %0 = tuple ()
+// CHECK-NEXT:    %1 = string_literal utf8 "delete_ints"
+// CHECK-NEXT:    return %0 : $()
+// CHECK-NEXT:  } // end sil function 'delete_next_backward'
+sil @delete_next_backward : $@convention(thin) () -> () {
+bb0:
+  %0 = tuple ()
+  %1 = integer_literal $Builtin.Int64, 3 
+  %2 = string_literal utf8 "delete_ints"
+  return %0 : $()
+}
+
+// LOG-LABEL: Test instruction iteration in delete_current_and_next_backward:
+// LOG-NEXT:  bb0:
+// LOG-NEXT:    return %0 : $()
+// LOG-NEXT:    %2 = string_literal utf8 "delete_strings"
+// LOG-NEXT:    %0 = tuple ()
+// LOG-NEXT:  End function delete_current_and_next_backward:
+
+// CHECK-LABEL: sil @delete_current_and_next_backward : $@convention(thin) () -> () {
+// CHECK-NEXT:  bb0:
+// CHECK-NEXT:    %0 = tuple ()
+// CHECK-NEXT:    return %0 : $()
+// CHECK-NEXT:  } // end sil function 'delete_current_and_next_backward'
+sil @delete_current_and_next_backward : $@convention(thin) () -> () {
+bb0:
+  %0 = tuple ()
+  %1 = string_literal utf8 ""
+  %2 = string_literal utf8 "delete_strings"
+  return %0 : $()
+}
+
+// LOG-LABEL: Test instruction iteration in delete_until_end_backward:
+// LOG-NEXT:  bb0:
+// LOG-NEXT:    return %3 : $()
+// LOG-NEXT:    %3 = tuple ()
+// LOG-NEXT:    %2 = string_literal utf8 "delete_strings"
+// LOG-NEXT:  End function delete_until_end_backward:
+
+// CHECK-LABEL: sil @delete_until_end_backward : $@convention(thin) () -> () {
+// CHECK-NEXT:  bb0:
+// CHECK-NEXT:    %0 = tuple ()
+// CHECK-NEXT:    return %0 : $()
+// CHECK-NEXT:  } // end sil function 'delete_until_end_backward'
+sil @delete_until_end_backward : $@convention(thin) () -> () {
+bb0:
+  %0 = string_literal utf8 ""
+  %1 = string_literal utf8 ""
+  %2 = string_literal utf8 "delete_strings"
+  %3 = tuple ()
+  return %3 : $()
+}
+
+// LOG-LABEL: Test instruction iteration in split_block_forward:
+// LOG-NEXT:  bb0:
+// LOG-NEXT:    %0 = tuple ()
+// LOG-NEXT:    %1 = string_literal utf8 "split_block"
+// LOG-NEXT:    %2 = integer_literal $Builtin.Int64, 3
+// LOG-NEXT:    return %0 : $()
+// LOG-NEXT:  End function split_block_forward:
+
+// CHECK-LABEL: sil @split_block_forward : $@convention(thin) () -> () {
+// CHECK-NEXT:  bb0:
+// CHECK-NEXT:    %0 = tuple ()
+// CHECK-NEXT:    unreachable
+// CHECK:       bb1:
+// CHECK-NEXT:    %2 = string_literal utf8 "split_block"
+// CHECK-NEXT:    %3 = integer_literal $Builtin.Int64, 3
+// CHECK-NEXT:    return %0 : $()
+// CHECK-NEXT:  } // end sil function 'split_block_forward'
+sil @split_block_forward : $@convention(thin) () -> () {
+bb0:
+  %0 = tuple ()
+  %1 = string_literal utf8 "split_block"
+  %2 = integer_literal $Builtin.Int64, 3 
+  return %0 : $()
+}
+
+// LOG-LABEL: Test instruction iteration in split_block_backward:
+// LOG-NEXT:  bb0:
+// LOG-NEXT:    return %0 : $()
+// LOG-NEXT:    %2 = integer_literal $Builtin.Int64, 3
+// LOG-NEXT:    %1 = string_literal utf8 "split_block"
+// LOG-NEXT:    %0 = tuple ()
+// LOG-NEXT:  End function split_block_backward:
+
+// CHECK-LABEL: sil @split_block_backward : $@convention(thin) () -> () {
+// CHECK-NEXT:  bb0:
+// CHECK-NEXT:    %0 = tuple ()
+// CHECK-NEXT:    unreachable
+// CHECK:       bb1:
+// CHECK-NEXT:    %2 = string_literal utf8 "split_block"
+// CHECK-NEXT:    %3 = integer_literal $Builtin.Int64, 3
+// CHECK-NEXT:    return %0 : $()
+// CHECK-NEXT:  } // end sil function 'split_block_backward'
+sil @split_block_backward : $@convention(thin) () -> () {
+bb0:
+  %0 = tuple ()
+  %1 = string_literal utf8 "split_block"
+  %2 = integer_literal $Builtin.Int64, 3 
+  return %0 : $()
+}
+

unittests/SIL/SILBitfieldTest.cpp

@@ -21,13 +21,13 @@ class BasicBlockBitfield;
 
 struct SILFunction {
   BasicBlockBitfield *newestAliveBlockBitfield = nullptr;
-  uint64_t currentBitfieldID = 1;
+  int64_t currentBitfieldID = 1;
 };
 
 struct SILBasicBlock {
   SILFunction *function;
   uint32_t customBits = 0;
-  uint64_t lastInitializedBitfieldID = 0;
+  int64_t lastInitializedBitfieldID = 0;
 
   enum { numCustomBits = 32 };