swiftlang · eeckstein · Jul 12, 2023 · Jul 11, 2023 · Jul 11, 2023
@@ -113,7 +113,7 @@ private struct CollectedEffects {
 
     case let store as StoreInst:
       addEffects(.write, to: store.destination)
-      if store.destinationOwnership == .assign {
+      if store.storeOwnership == .assign {
         addDestroyEffects(ofAddress: store.destination)
       }
 
@@ -454,7 +454,7 @@ private struct ArgumentEscapingWalker : ValueDefUseWalker, AddressDefUseWalker {
     case let load as LoadInst:
       if !address.value.hasTrivialType &&
           // In non-ossa SIL we don't know if a load is taking.
-          (!function.hasOwnership || load.ownership == .take) {
+          (!function.hasOwnership || load.loadOwnership == .take) {
         foundTakingLoad = true
       }
       return .continueWalk

@@ -14,7 +14,7 @@ import SIL
 
 /// Eliminates dead store instructions.
 ///
-/// A store is dead if after the store has occurred:
+/// A store is dead if, after the store has occurred:
 ///
 /// 1. The value in memory is not read until the memory object is deallocated:
 ///
@@ -47,6 +47,9 @@ import SIL
 ///     ...              // no reads from %1
 ///     store %7 to %3
 ///
+/// The algorithm is a data flow analysis which starts at the original store and searches
+/// for successive stores by following the control flow in forward direction.
+///
 let deadStoreElimination = FunctionPass(name: "dead-store-elimination") {
   (function: Function, context: FunctionPassContext) in
 
@@ -85,7 +88,7 @@ private func tryEliminate(store: StoreInst, _ context: FunctionPassContext) {
         case .dead:
           // The new individual stores are inserted right after the current store and
           // will be optimized in the following loop iterations.
-          store.trySplit(context)
+          store.split(context)
       }
   }
 }
@@ -111,18 +114,24 @@ private extension StoreInst {
   }
 
   func isDead(at accessPath: AccessPath, _ context: FunctionPassContext) -> DataflowResult {
-    var worklist = InstructionWorklist(context)
-    defer { worklist.deinitialize() }
+    var scanner = InstructionScanner(storePath: accessPath, storeAddress: self.destination, context.aliasAnalysis)
+    let storageDefBlock = accessPath.base.reference?.referenceRoot.parentBlock
 
-    worklist.pushIfNotVisited(self.next!)
+    switch scanner.scan(instructions: InstructionList(first: self.next)) {
+    case .dead:
+      return .dead
 
-    let storageDefBlock = accessPath.base.reference?.referenceRoot.parentBlock
-    var scanner = InstructionScanner(storePath: accessPath, storeAddress: self.destination, context.aliasAnalysis)
+    case .alive:
+      return DataflowResult(aliveWith: scanner.potentiallyDeadSubpath)
+
+    case .transparent:
+      // Continue with iterative data flow analysis starting at the block's successors.
+      var worklist = BasicBlockWorklist(context)
+      defer { worklist.deinitialize() }
+      worklist.pushIfNotVisited(contentsOf: self.parentBlock.successors)
+
+      while let block = worklist.pop() {
 
-    while let startInstInBlock = worklist.pop() {
-      let block = startInstInBlock.parentBlock
-      switch scanner.scan(instructions: InstructionList(first: startInstInBlock)) {
-      case .transparent:
         // Abort if we find the storage definition of the access in case of a loop, e.g.
         //
         //   bb1:
@@ -133,43 +142,45 @@ private extension StoreInst {
         //
         // The storage root is different in each loop iteration. Therefore the store of a
         // successive loop iteration does not overwrite the store of the previous iteration.
-        if let storageDefBlock = storageDefBlock,
-           block.successors.contains(storageDefBlock) {
+        if let storageDefBlock = storageDefBlock, block == storageDefBlock {
+          return DataflowResult(aliveWith: scanner.potentiallyDeadSubpath)
+        }
+        switch scanner.scan(instructions: block.instructions) {
+        case .transparent:
+          worklist.pushIfNotVisited(contentsOf: block.successors)
+        case .dead:
+          break
+        case .alive:
           return DataflowResult(aliveWith: scanner.potentiallyDeadSubpath)
         }
-        worklist.pushIfNotVisited(contentsOf: block.successors.lazy.map { $0.instructions.first! })
-      case .dead:
-        break
-      case .alive:
-        return DataflowResult(aliveWith: scanner.potentiallyDeadSubpath)
       }
+      return .dead
     }
-    return .dead
   }
 
-  func trySplit(_ context: FunctionPassContext) {
+  func split(_ context: FunctionPassContext) {
+    let builder = Builder(after: self, context)
     let type = source.type
     if type.isStruct {
-      let builder = Builder(after: self, context)
       for idx in 0..<type.getNominalFields(in: parentFunction).count {
         let srcField = builder.createStructExtract(struct: source, fieldIndex: idx)
         let destFieldAddr = builder.createStructElementAddr(structAddress: destination, fieldIndex: idx)
-        builder.createStore(source: srcField, destination: destFieldAddr, ownership: destinationOwnership)
+        builder.createStore(source: srcField, destination: destFieldAddr, ownership: storeOwnership)
       }
-      context.erase(instruction: self)
     } else if type.isTuple {
-      let builder = Builder(after: self, context)
       for idx in 0..<type.tupleElements.count {
         let srcField = builder.createTupleExtract(tuple: source, elementIndex: idx)
         let destFieldAddr = builder.createTupleElementAddr(tupleAddress: destination, elementIndex: idx)
-        builder.createStore(source: srcField, destination: destFieldAddr, ownership: destinationOwnership)
+        builder.createStore(source: srcField, destination: destFieldAddr, ownership: storeOwnership)
       }
-      context.erase(instruction: self)
+    } else {
+      fatalError("a materializable projection path should only contain struct and tuple projections")
     }
+    context.erase(instruction: self)
   }
 
   var hasValidOwnershipForDeadStoreElimination: Bool {
-    switch destinationOwnership {
+    switch storeOwnership {
     case .unqualified, .trivial:
       return true
     case .initialize, .assign:
@@ -181,11 +192,11 @@ private extension StoreInst {
 }
 
 private struct InstructionScanner {
-  let storePath: AccessPath
-  let storeAddress: Value
-  let aliasAnalysis: AliasAnalysis
+  private let storePath: AccessPath
+  private let storeAddress: Value
+  private let aliasAnalysis: AliasAnalysis
 
-  var potentiallyDeadSubpath: AccessPath? = nil
+  private(set) var potentiallyDeadSubpath: AccessPath? = nil
 
   // Avoid quadratic complexity by limiting the number of visited instructions for each store.
   // The limit of 1000 instructions is not reached by far in "real-world" functions.
@@ -208,14 +219,16 @@ private struct InstructionScanner {
       switch inst {
       case let successiveStore as StoreInst:
         let successivePath = successiveStore.destination.accessPath
-        if successivePath.isEqualOrOverlaps(storePath) {
+        if successivePath.isEqualOrContains(storePath) {
           return .dead
         }
-        if storePath.isEqualOrOverlaps(successivePath),
-           potentiallyDeadSubpath == nil {
+        if potentiallyDeadSubpath == nil,
+           storePath.getMaterializableProjection(to: successivePath) != nil {
           // Storing to a sub-field of the original store doesn't make the original store dead.
           // But when we split the original store, then one of the new individual stores might be
           // overwritten by this store.
+          // Requiring that the projection to the partial store path is materializable guarantees
+          // that we can split the store.
           potentiallyDeadSubpath = successivePath
         }
       case is DeallocRefInst, is DeallocStackRefInst, is DeallocBoxInst:
@@ -241,6 +254,8 @@ private struct InstructionScanner {
         if inst.mayRead(fromAddress: storeAddress, aliasAnalysis) {
           return .alive
         }
+        // TODO: We might detect that this is a partial read of the original store which potentially
+        //       enables partial dead store elimination.
       }
     }
     return .transparent

@@ -43,7 +43,7 @@ extension InitEnumDataAddrInst : OnoneSimplifyable {
 
       let builder = Builder(before: store, context)
       let enumInst = builder.createEnum(caseIndex: self.caseIndex, payload: store.source, enumType: self.enum.type.objectType)
-      let storeOwnership = StoreInst.Ownership(for: self.enum.type, in: parentFunction, initialize: true)
+      let storeOwnership = StoreInst.StoreOwnership(for: self.enum.type, in: parentFunction, initialize: true)
       builder.createStore(source: enumInst, destination: self.enum, ownership: storeOwnership)
       context.erase(instruction: store)
       context.erase(instruction: inject)

@@ -45,7 +45,7 @@ extension LoadInst : OnoneSimplifyable, SILCombineSimplifyable {
 
       operand.set(to: uac.fromAddress, context)
       let builder = Builder(before: self, context)
-      let newLoad = builder.createLoad(fromAddress: uac.fromAddress, ownership: ownership)
+      let newLoad = builder.createLoad(fromAddress: uac.fromAddress, ownership: loadOwnership)
       let cast = builder.createUpcast(from: newLoad, to: type)
       uses.replaceAll(with: cast, context)
       context.erase(instruction: self)
@@ -184,7 +184,7 @@ extension LoadInst : OnoneSimplifyable, SILCombineSimplifyable {
 
   /// Removes the `load [copy]` if the loaded value is just destroyed.
   private func removeIfDead(_ context: SimplifyContext) {
-    if ownership == .copy,
+    if loadOwnership == .copy,
        loadedValueIsDead(context) {
       for use in uses {
         context.erase(instruction: use.instruction)

@@ -162,7 +162,11 @@ enum AccessBase : CustomStringConvertible, Hashable {
     }
   }
 
-  // Returns true if it's guaranteed that this access has the same base address as the `other` access.
+  /// Returns true if it's guaranteed that this access has the same base address as the `other` access.
+  ///
+  /// `isEqual` abstracts away the projection instructions that are included as part of the AccessBase:
+  /// multiple `project_box` and `ref_element_addr` instructions are equivalent bases as long as they
+  /// refer to the same variable or class property.
   func isEqual(to other: AccessBase) -> Bool {
     switch (self, other) {
     case (.box(let pb1), .box(let pb2)):
@@ -275,11 +279,52 @@ struct AccessPath : CustomStringConvertible {
     return false
   }
 
-  func isEqualOrOverlaps(_ other: AccessPath) -> Bool {
-    return base.isEqual(to: other.base) &&
-           // Note: an access with a smaller path overlaps an access with larger path, e.g.
-           //       `s0` overlaps `s0.s1`
-           projectionPath.isSubPath(of: other.projectionPath)
+  /// Returns true if this access addresses the same memory location as `other` or if `other`
+  /// is a sub-field of this access.
+
+  /// Note that this access _contains_ `other` if `other` has a _larger_ projection path than this acccess.
+  /// For example:
+  ///   `%value.s0` contains `%value.s0.s1`
+  func isEqualOrContains(_ other: AccessPath) -> Bool {
+    return getProjection(to: other) != nil
+  }
+
+  /// Returns the projection path to `other` if this access path is equal or contains `other`.
+  ///
+  /// For example,
+  ///   `%value.s0`.getProjection(to: `%value.s0.s1`)
+  /// yields
+  ///   `s1`
+  func getProjection(to other: AccessPath) -> SmallProjectionPath? {
+    if !base.isEqual(to: other.base) {
+      return nil
+    }
+    return projectionPath.subtract(from: other.projectionPath)
+  }
+
+  /// Like `getProjection`, but also requires that the resulting projection path is materializable.
+  func getMaterializableProjection(to other: AccessPath) -> SmallProjectionPath? {
+    if let projectionPath = getProjection(to: other),
+       projectionPath.isMaterializable {
+      return projectionPath
+    }
+    return nil
+  }
+}
+
+private extension SmallProjectionPath {
+  /// Returns true if the path only contains projections which can be materialized as
+  /// SIL struct or tuple projection instructions - for values or addresses.
+  var isMaterializable: Bool {
+    let (kind, _, subPath) = pop()
+    switch kind {
+    case .root:
+      return true
+    case .structField, .tupleField:
+      return subPath.isMaterializable
+    default:
+      return false
+    }
   }
 }
 

@@ -435,7 +435,7 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
     case is StoreInst, is StoreWeakInst, is StoreUnownedInst:
       let store = instruction as! StoringInstruction
       assert(operand == store.destinationOperand)
-      if let si = store as? StoreInst, si.destinationOwnership == .assign {
+      if let si = store as? StoreInst, si.storeOwnership == .assign {
         if handleDestroy(of: operand.value, path: path.with(knownType: nil)) == .abortWalk {
           return .abortWalk
         }

@@ -113,7 +113,7 @@ public struct Builder {
     return notifyNew(cast.getAs(UpcastInst.self))
   }
 
-  public func createLoad(fromAddress: Value, ownership: LoadInst.Ownership) -> LoadInst {
+  public func createLoad(fromAddress: Value, ownership: LoadInst.LoadOwnership) -> LoadInst {
     let load = bridged.createLoad(fromAddress.bridged, ownership.rawValue)
     return notifyNew(load.getAs(LoadInst.self))
   }
@@ -274,7 +274,7 @@ public struct Builder {
   }
 
   @discardableResult
-  public func createStore(source: Value, destination: Value, ownership: StoreInst.Ownership) -> StoreInst {
+  public func createStore(source: Value, destination: Value, ownership: StoreInst.StoreOwnership) -> StoreInst {
     let store = bridged.createStore(source.bridged, destination.bridged, ownership.rawValue)
     return notifyNew(store.getAs(StoreInst.self))
   }

@@ -228,7 +228,7 @@ extension StoringInstruction {
 
 final public class StoreInst : Instruction, StoringInstruction {
   // must match with enum class StoreOwnershipQualifier
-  public enum Ownership: Int {
+  public enum StoreOwnership: Int {
     case unqualified = 0, initialize = 1, assign = 2, trivial = 3
 
     public init(for type: Type, in function: Function, initialize: Bool) {
@@ -243,8 +243,8 @@ final public class StoreInst : Instruction, StoringInstruction {
       }
     }
   }
-  public var destinationOwnership: Ownership {
-    Ownership(rawValue: bridged.StoreInst_getStoreOwnership())!
+  public var storeOwnership: StoreOwnership {
+    StoreOwnership(rawValue: bridged.StoreInst_getStoreOwnership())!
   }
 }
 
@@ -387,11 +387,11 @@ final public class LoadInst : SingleValueInstruction, UnaryInstruction {
   public var address: Value { operand.value }
 
   // must match with enum class LoadOwnershipQualifier
-  public enum Ownership: Int {
+  public enum LoadOwnership: Int {
     case unqualified = 0, take = 1, copy = 2, trivial = 3
   }
-  public var ownership: Ownership {
-    Ownership(rawValue: bridged.LoadInst_getLoadOwnership())!
+  public var loadOwnership: LoadOwnership {
+    LoadOwnership(rawValue: bridged.LoadInst_getLoadOwnership())!
   }
 }