Redesign gatherBorrowIntroducers to return BeginBorrowValues.

This avoids a lot of confusion because the callers expect this type. Fixing it just required some redundancy and
bridging in the EnclosigValues implementation.

Author: atrick

SwiftCompilerSources/Sources/Optimizer/Utilities/BorrowUtils.swift

@@ -393,8 +393,9 @@ enum BeginBorrowValue {
 ///     %field = ref_element_addr %first           // (none)
 ///     %load = load_borrow %field : $*C           // %load
 func gatherBorrowIntroducers(for value: Value,
-                             in borrowIntroducers: inout Stack<Value>,
+                             in borrowIntroducers: inout Stack<BeginBorrowValue>,
                              _ context: Context) {
+  assert(value.ownership == .guaranteed)
 
   // Cache introducers across multiple instances of BorrowIntroducers.
   var cache = BorrowIntroducers.Cache(context)
@@ -404,7 +405,7 @@ func gatherBorrowIntroducers(for value: Value,
 }
 
 private struct BorrowIntroducers {
-  typealias CachedIntroducers = SingleInlineArray<Value>
+  typealias CachedIntroducers = SingleInlineArray<BeginBorrowValue>
   struct Cache {
     // Cache the introducers already found for each SILValue.
     var valueIntroducers: Dictionary<HashableValue, CachedIntroducers>
@@ -430,21 +431,21 @@ private struct BorrowIntroducers {
   // introducer set to avoid adding duplicates.
   var visitedIntroducers: Set<HashableValue> = Set()
 
-  static func gather(for value: Value, in introducers: inout Stack<Value>,
+  static func gather(for value: Value, in introducers: inout Stack<BeginBorrowValue>,
                      _ cache: inout Cache, _ context: Context) {
     var borrowIntroducers = BorrowIntroducers(context: context)
     borrowIntroducers.gather(for: value, in: &introducers, &cache)
   }
 
-  private mutating func push(_ introducer: Value,
-    in introducers: inout Stack<Value>) {
-    if visitedIntroducers.insert(introducer.hashable).inserted {
-      introducers.push(introducer)
+  private mutating func push(_ beginBorrow: BeginBorrowValue,
+    in introducers: inout Stack<BeginBorrowValue>) {
+    if visitedIntroducers.insert(beginBorrow.value.hashable).inserted {
+      introducers.push(beginBorrow)
     }
   }
 
   private mutating func push<S: Sequence>(contentsOf other: S,
-    in introducers: inout Stack<Value>) where S.Element == Value {
+    in introducers: inout Stack<BeginBorrowValue>) where S.Element == BeginBorrowValue {
     for elem in other {
       push(elem, in: &introducers)
     }
@@ -457,8 +458,9 @@ private struct BorrowIntroducers {
   //
   // Otherwise recurse up the use-def chain to find all introducers.
   private mutating func gather(for value: Value,
-                               in introducers: inout Stack<Value>,
+                               in introducers: inout Stack<BeginBorrowValue>,
                                _ cache: inout Cache) {
+    assert(value.ownership == .guaranteed)
     // Check if this value's introducers have already been added to
     // 'introducers' to avoid duplicates and avoid exponential
     // recursion on aggregates.
@@ -478,23 +480,12 @@ private struct BorrowIntroducers {
   }
 
   private mutating func gatherUncached(for value: Value,
-                                       in introducers: inout Stack<Value>,
+                                       in introducers: inout Stack<BeginBorrowValue>,
                                        _ cache: inout Cache) {
-    switch value.ownership {
-    case .none, .unowned:
-      return
-
-    case .owned:
-      push(value, in: &introducers);
-      return
-
-    case .guaranteed:
-      break
-    }
     // BeginBorrowedValue handles the initial scope introducers: begin_borrow,
     // load_borrow, & reborrow.
-    if BeginBorrowValue(value) != nil {
-      push(value, in: &introducers)
+    if let beginBorrow = BeginBorrowValue(value) {
+      push(beginBorrow, in: &introducers)
       return
     }
     // Handle guaranteed forwarding phis
@@ -543,27 +534,35 @@ private struct BorrowIntroducers {
   //    %reborrow_2 is returned.
   //
   private mutating func gather(forPhi phi: Phi,
-                               in introducers: inout Stack<Value>,
+                               in introducers: inout Stack<BeginBorrowValue>,
                                _ cache: inout Cache) {
     // Phi cycles are skipped. They cannot contribute any new introducer.
     if !cache.pendingPhis.insert(phi.value) {
       return
     }
     for (pred, value) in zip(phi.predecessors, phi.incomingValues) {
+      switch value.ownership {
+      case .none:
+        continue
+      case .owned, .unowned:
+        fatalError("unexpected ownership for a guaranteed phi operand")
+      case .guaranteed:
+        break
+      }
       // Each phi operand requires a new introducer list and visited
       // values set. These values will be remapped to successor phis
       // before adding them to the caller's introducer list. It may be
       // necessary to revisit a value that was already visited by the
       // caller before remapping to phis.
-      var incomingIntroducers = Stack<Value>(context)
+      var incomingIntroducers = Stack<BeginBorrowValue>(context)
       defer {
         incomingIntroducers.deinitialize()
       }
       BorrowIntroducers.gather(for: value, in: &incomingIntroducers,
                                &cache, context)
       // Map the incoming introducers to an outer-adjacent phi if one exists.
-      push(contentsOf: mapToPhi(predecessor: pred,
-                                incomingValues: incomingIntroducers),
+      push(contentsOf: mapToGuaranteedPhi(predecessor: pred,
+                                          incomingBorrows: incomingIntroducers),
            in: &introducers)
     }
     // Remove this phi from the pending set. This phi may be visited
@@ -574,13 +573,35 @@ private struct BorrowIntroducers {
   }
 }
 
-// Given incoming values on a predecessor path, return the
-// corresponding values on the successor block. Each incoming value is
+// Given incoming borrows on a predecessor path, return the
+// corresponding borrows on the successor block. Each incoming borrow is
 // either used by a phi in the successor block, or it must dominate
 // the successor block.
+private func mapToGuaranteedPhi<PredecessorSequence: Sequence<BeginBorrowValue>> (
+  predecessor: BasicBlock, incomingBorrows: PredecessorSequence)
+-> LazyMapSequence<PredecessorSequence, BeginBorrowValue> {
+
+  let branch = predecessor.terminator as! BranchInst
+  // Gather the new introducers for the successor block.
+  return incomingBorrows.lazy.map { incomingBorrow in
+    // Find an outer adjacent phi in the successor block.
+    let incomingValue = incomingBorrow.value
+    if let incomingOp = branch.operands.first(where: { $0.value == incomingValue }) {
+      return BeginBorrowValue(branch.getArgument(for: incomingOp))!
+    }
+    // No candidates phi are outer-adjacent phis. The incoming
+    // `predDef` must dominate the current guaranteed phi.
+    return incomingBorrow
+  }
+}
+
+// Given incoming values on a predecessor path, return the corresponding values on the successor block. Each incoming
+// value is either used by a phi in the successor block, or it must dominate the successor block.
+//
+// This is Logically the same as mapToGuaranteedPhi but more efficient to simply duplicate the code.
 private func mapToPhi<PredecessorSequence: Sequence<Value>> (
   predecessor: BasicBlock, incomingValues: PredecessorSequence)
--> LazyMapSequence<PredecessorSequence, Value> {
+  -> LazyMapSequence<PredecessorSequence, Value> {
 
   let branch = predecessor.terminator as! BranchInst
   // Gather the new introducers for the successor block.
@@ -612,7 +633,7 @@ private func mapToPhi<PredecessorSequence: Sequence<Value>> (
 /// introducers of the outer enclosing borrow scope that contains this
 /// inner scope.
 ///
-/// If `value` is a `begin_borrow`, then this returns its operand.
+/// If `value` is a `begin_borrow`, then this returns its owned operand, or the introducers of its guaranteed operand.
 ///
 /// If `value` is an owned value, a function argument, or a
 /// load_borrow, then this is an empty set.
@@ -725,9 +746,18 @@ private struct EnclosingValues {
     if let beginBorrow = BeginBorrowValue(value) {
       switch beginBorrow {
       case let .beginBorrow(bbi):
+        let outerValue = bbi.operand.value
+        switch outerValue.ownership {
+        case .none, .unowned:
+          return
+        case .owned:
+          push(outerValue, in: &enclosingValues);
+          return
+        case .guaranteed:
+          break
+        }
         // Gather the outer enclosing borrow scope.
-        BorrowIntroducers.gather(for: bbi.operand.value, in: &enclosingValues,
-                                 &cache.borrowIntroducerCache, context)
+        gatherBorrows(for: outerValue, in: &enclosingValues, &cache)
       case .loadBorrow, .beginApply, .functionArgument:
         // There is no enclosing value on this path.
         break
@@ -736,11 +766,19 @@ private struct EnclosingValues {
       }
     } else {
       // Handle forwarded guaranteed values.
-      BorrowIntroducers.gather(for: value, in: &enclosingValues,
-                               &cache.borrowIntroducerCache, context)
+      gatherBorrows(for: value, in: &enclosingValues, &cache)
     }
   }
-  
+
+  mutating func gatherBorrows(for value: Value, in enclosingValues: inout Stack<Value>, _ cache: inout Cache) {
+    var introducers = Stack<BeginBorrowValue>(context)
+    defer { introducers.deinitialize() }
+    BorrowIntroducers.gather(for: value, in: &introducers, &cache.borrowIntroducerCache, context)
+    for beginBorrow in introducers {
+      enclosingValues.push(beginBorrow.value)
+    }
+  }
+
   // Given a reborrow, find the enclosing values. Each enclosing value
   // is represented by one of the following cases, which refer to the
   // example below:
@@ -841,12 +879,12 @@ let borrowIntroducersTest = FunctionTest("borrow_introducers") {
   let value = arguments.takeValue()
   print(function)
   print("Borrow introducers for: \(value)")
-  var introducers = Stack<Value>(context)
+  var introducers = Stack<BeginBorrowValue>(context)
   defer {
     introducers.deinitialize()
   }
   gatherBorrowIntroducers(for: value, in: &introducers, context)
-  introducers.forEach { print($0) }
+  introducers.forEach { print($0.value) }
 }
 
 let enclosingValuesTest = FunctionTest("enclosing_values") {

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

@@ -364,19 +364,16 @@ extension LifetimeDependence.Scope {
   }
 
   private init?(guaranteed base: Value, _ context: some Context) {
-    var introducers = Stack<Value>(context)
+    var introducers = Stack<BeginBorrowValue>(context)
     gatherBorrowIntroducers(for: base, in: &introducers, context)
     // If introducers is empty, then the dependence is on a trivial value, so
     // there is no dependence scope.
     //
     // TODO: Add a SIL verifier check that a mark_dependence [nonescaping]
     // base is never a guaranteed phi.
-    guard let introducer = introducers.pop() else { return nil }
+    guard let beginBorrow = introducers.pop() else { return nil }
     assert(introducers.isEmpty,
            "guaranteed phis not allowed when diagnosing lifetime dependence")
-    guard let beginBorrow = BeginBorrowValue(introducer) else {
-      fatalError("unknown borrow introducer")
-    }
     switch beginBorrow {
     case .beginBorrow, .loadBorrow:
       let borrowOperand = beginBorrow.baseOperand!

test/SILOptimizer/borrow_introducer_unit.sil

@@ -340,11 +340,6 @@ bb2:
   unreachable
 }
 
-// CHECK-LABEL: introducer_example: borrow_introducers with: %0
-// CHECK: Borrow introducers for: %0 = argument of bb0 : $C
-// CHECK-NEXT: %0 = argument of bb0 : $C
-// CHECK-NEXT: introducer_example: borrow_introducers with: %0
-
 // CHECK-LABEL: introducer_example: borrow_introducers with: %borrow0
 // CHECK: Borrow introducers for:   %{{.*}} = begin_borrow %0 : $C
 // CHECK-NEXT:  %{{.*}} = begin_borrow %0 : $C
@@ -356,25 +351,19 @@ bb2:
 // CHECK-NEXT:  %{{.*}} = argument of bb0 : $C
 // CHECK-NEXT: introducer_example: borrow_introducers with: %first
 
-// CHECK-LABEL: introducer_example: borrow_introducers with: %field
-// CHECK: Borrow introducers for:   %{{.*}} = ref_element_addr %{{.*}} : $C, #C.field
-// CHECK-NEXT: introducer_example: borrow_introducers with: %field
-
 // CHECK-LABEL: introducer_example: borrow_introducers with: %load
 // CHECK-LABEL: Borrow introducers for:   %{{.*}} = load_borrow %{{.*}} : $*C
 // CHECK-NEXT: %{{.*}} = load_borrow %{{.*}} : $*C
 // CHECK-NEXT: introducer_example: borrow_introducers with: %
 sil [ossa] @introducer_example : $@convention(thin) (@owned C, @guaranteed C) -> () {
 bb0(%0 : @owned $C,
     %1 : @guaranteed $C):
-  specify_test "borrow_introducers %0"
   %borrow0 = begin_borrow %0 : $C
   specify_test "borrow_introducers %borrow0"
   %pair = struct $PairC(%borrow0 : $C, %1 : $C)
   %first = struct_extract %pair : $PairC, #PairC.first
   specify_test "borrow_introducers %first"
   %field = ref_element_addr %first : $C, #C.field
-  specify_test "borrow_introducers %field"
   %load = load_borrow %field : $*C
   specify_test "borrow_introducers %load"
   end_borrow %load : $C
@@ -417,7 +406,7 @@ bb3:
 // visited once on each path, and each time the recursive algorithm
 // must find the enclosing def %0, which maps to a %outer0 on one path
 // and %outer1 on another path. If the cache fails, then we only find
-// one of the outer adjacent phis as and enclosing value for %inner.
+// one of the outer adjacent phis as an enclosing value for %inner.
 //
 // CHECK-LABEL: testEnclosingRevisitReborrow: enclosing_values with: %inner
 // CHECK: Enclosing values for: %10 = argument of bb4 : $C                        // user: %11