Merge pull request #63614 from eeckstein/fix-escape-utils

EscapeUtils: fix some bugs related to `followLoads`

Author: eeckstein

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeEscapeEffects.swift

@@ -158,9 +158,9 @@ func addArgEffects(_ arg: FunctionArgument, argPath ap: SmallProjectionPath,
     effect = EscapeEffects.ArgumentEffect(.escapingToReturn(toPath, exclusive),
                                           argumentIndex: arg.index, pathPattern: argPath)
   case .toArgument(let toArgIdx, let toPath):
-    let exclusive = isExclusiveEscapeToArgument(fromArgument: arg, fromPath: argPath,
-                                                toArgumentIndex: toArgIdx, toPath: toPath, context)
-    effect = EscapeEffects.ArgumentEffect(.escapingToArgument(toArgIdx, toPath, exclusive),
+    // Exclusive argument -> argument effects cannot appear because such an effect would
+    // involve a store which is not permitted for exclusive escapes.
+    effect = EscapeEffects.ArgumentEffect(.escapingToArgument(toArgIdx, toPath, /*exclusive*/ false),
                                           argumentIndex: arg.index, pathPattern: argPath)
   }
   newEffects.append(effect)
@@ -197,25 +197,39 @@ private func isOperandOfRecursiveCall(_ op: Operand) -> Bool {
   return false
 }
 
-/// Returns true if when walking from the `toSelection` to the `fromArgument`,
-/// there are no other arguments or escape points than `fromArgument`. Also, the
-/// path at the `fromArgument` must match with `fromPath`.
+/// Returns true if when walking up from the `returnInst`, the `fromArgument` is the one
+/// and only argument which is reached - with a matching `fromPath`.
 private
 func isExclusiveEscapeToReturn(fromArgument: Argument, fromPath: SmallProjectionPath,
                                toPath: SmallProjectionPath,
                                returnInst: ReturnInst, _ context: FunctionPassContext) -> Bool {
-  struct IsExclusiveReturnEscapeVisitor : EscapeVisitor {
+  struct IsExclusiveReturnEscapeVisitor : EscapeVisitorWithResult {
     let fromArgument: Argument
     let fromPath: SmallProjectionPath
     let toPath: SmallProjectionPath
+    var result = false
 
     mutating func visitUse(operand: Operand, path: EscapePath) -> UseResult {
-      if operand.instruction is ReturnInst {
+      switch operand.instruction {
+      case is ReturnInst:
         if path.followStores { return .abort }
         if path.projectionPath.matches(pattern: toPath) {
           return .ignore
         }
         return .abort
+      case let si as StoringInstruction:
+        // Don't allow store instructions because this allows the EscapeUtils to walk up
+        // an apply result with `followStores`.
+        if operand == si.destinationOperand {
+          return .abort
+        }
+      case let ca as CopyAddrInst:
+        // `copy_addr` is like a store.
+        if operand == ca.destinationOperand {
+          return .abort
+        }
+      default:
+        break
       }
       return .continueWalk
     }
@@ -226,37 +240,12 @@ func isExclusiveEscapeToReturn(fromArgument: Argument, fromPath: SmallProjection
       }
       if path.followStores { return .abort }
       if arg == fromArgument && path.projectionPath.matches(pattern: fromPath) {
+        result = true
         return .walkDown
       }
       return .abort
     }
   }
   let visitor = IsExclusiveReturnEscapeVisitor(fromArgument: fromArgument, fromPath: fromPath, toPath: toPath)
-  return !returnInst.operand.at(toPath).isEscaping(using: visitor, context)
+  return returnInst.operand.at(toPath).visit(using: visitor, context) ?? false
 }
-
-private
-func isExclusiveEscapeToArgument(fromArgument: Argument, fromPath: SmallProjectionPath,
-                                 toArgumentIndex: Int, toPath: SmallProjectionPath, _ context: FunctionPassContext) -> Bool {
-  struct IsExclusiveArgumentEscapeVisitor : EscapeVisitor {
-    let fromArgument: Argument
-    let fromPath: SmallProjectionPath
-    let toArgumentIndex: Int
-    let toPath: SmallProjectionPath
-    
-    mutating func visitDef(def: Value, path: EscapePath) -> DefResult {
-      guard let arg = def as? FunctionArgument else {
-        return .continueWalkUp
-      }
-      if path.followStores { return .abort }
-      if arg == fromArgument && path.projectionPath.matches(pattern: fromPath) { return .walkDown }
-      if arg.index == toArgumentIndex && path.projectionPath.matches(pattern: toPath) { return .walkDown }
-      return .abort
-    }
-  }
-  let visitor = IsExclusiveArgumentEscapeVisitor(fromArgument: fromArgument, fromPath: fromPath,
-                                                 toArgumentIndex: toArgumentIndex, toPath: toPath)
-  let toArg = fromArgument.parentFunction.arguments[toArgumentIndex]
-  return !toArg.at(toPath).isEscapingWhenWalkingDown(using: visitor, context)
-}
-

SwiftCompilerSources/Sources/Optimizer/Utilities/EscapeUtils.swift

@@ -547,7 +547,9 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
     }
 
     // Indirect arguments cannot escape the function, but loaded values from such can.
-    if !followLoads(at: path.projectionPath) {
+    if !followLoads(at: path.projectionPath) &&
+       // Except for begin_apply: it can yield an address value.
+       !apply.isBeginApplyWithIndirectResults {
       return .continueWalk
     }
 
@@ -592,7 +594,8 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
     var matched = false
     for effect in effects.escapeEffects.arguments {
       switch effect.kind {
-      case .escapingToArgument(let toArgIdx, let toPath, let exclusive):
+      case .escapingToArgument(let toArgIdx, let toPath, _):
+        // Note: exclusive argument -> argument effects cannot appear, so we don't need to handle them here.
         if effect.matches(calleeArgIdx, argPath.projectionPath) {
           guard let callerToIdx = apply.callerArgIndex(calleeArgIndex: toArgIdx) else {
             return isEscaping
@@ -602,20 +605,6 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
           let arg = apply.arguments[callerToIdx]
 
           let p = Path(projectionPath: toPath, followStores: false, knownType: nil)
-          if walkUp(addressOrValue: arg, path: p) == .abortWalk {
-            return .abortWalk
-          }
-          matched = true
-        } else if toArgIdx == calleeArgIdx && argPath.projectionPath.matches(pattern: toPath) {
-          // Handle the reverse direction of an arg-to-arg escape.
-          guard let callerArgIdx = apply.callerArgIndex(calleeArgIndex: effect.argumentIndex) else {
-            return isEscaping
-          }
-          if !exclusive { return isEscaping }
-
-          let arg = apply.arguments[callerArgIdx]
-          let p = Path(projectionPath: effect.pathPattern, followStores: false, knownType: nil)
-
           if walkUp(addressOrValue: arg, path: p) == .abortWalk {
             return .abortWalk
           }
@@ -690,6 +679,14 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
     case let ap as ApplyInst:
       return walkUpApplyResult(apply: ap, path: path.with(knownType: nil))
     case is LoadInst, is LoadWeakInst, is LoadUnownedInst:
+      if !followLoads(at: path.projectionPath) {
+        // When walking up we shouldn't end up at a load where followLoads is false,
+        // because going from a (non-followLoads) address to a load always involves a class indirection.
+        // There is one exception: loading a raw pointer, e.g.
+        //   %l = load %a : $Builtin.RawPointer
+        //   %a = pointer_to_address %l           // the up-walk starts at %a
+        return isEscaping
+      }
       return walkUp(address: (def as! UnaryInstruction).operand,
                     path: path.with(followStores: true).with(knownType: nil))
     case let atp as AddressToPointerInst:
@@ -837,3 +834,13 @@ private extension SmallProjectionPath {
     EscapeUtilityTypes.EscapePath(projectionPath: self, followStores: false, knownType: nil)
   }
 }
+
+private extension ApplySite {
+  var isBeginApplyWithIndirectResults: Bool {
+    guard let ba = self as? BeginApplyInst else {
+      return false
+    }
+    // Note that the token result is always a non-address type.
+    return ba.results.contains { $0.type.isAddress }
+  }
+}

test/SILOptimizer/addr_escape_info.sil

@@ -51,6 +51,7 @@ sil @initX : $@convention(method) (@owned X) -> @owned X {
 [%0: escape => %r, escape c*.v** => %r.c*.v**]
 }
 sil @modifyStr : $@convention(method) (@inout Str) -> ()
+sil @guaranteed_yield_coroutine : $@yield_once @convention(thin) (@inout X) -> @yields @inout X
 
 // CHECK-LABEL: Address escape information for test_simple:
 // CHECK:       value:  %1 = struct_element_addr %0 : $*Str, #Str.a
@@ -268,6 +269,28 @@ bb0:
   return %12 : $()
 }
 
+// CHECK-LABEL: Address escape information for indirect_argument_escape_to_yield:
+// CHECK:       value:  %1 = alloc_stack $X
+// CHECK-NEXT:    ==>   (%5, %6) = begin_apply %4(%1) : $@yield_once @convention(thin) (@inout X) -> @yields @inout X
+// CHECK-NEXT:    ==>   %8 = apply %7(%5) : $@convention(thin) (@inout X) -> ()
+// CHECK:       End function indirect_argument_escape_to_yield
+sil @indirect_argument_escape_to_yield : $@convention(thin) (@guaranteed X) -> () {
+bb0(%0 : $X):
+  %1 = alloc_stack $X
+  store %0 to %1 : $*X
+  fix_lifetime %1 : $*X
+  %3 = function_ref @guaranteed_yield_coroutine : $@yield_once @convention(thin) (@inout X) -> @yields @inout X
+  (%4, %5) = begin_apply %3(%1) : $@yield_once @convention(thin) (@inout X) -> @yields @inout X
+
+  %6 = function_ref @inout_class_argument : $@convention(thin) (@inout X) -> ()
+  %7 = apply %6(%4) : $@convention(thin) (@inout X) -> ()
+
+  end_apply %5
+  dealloc_stack %1 : $*X
+  %r = tuple ()
+  return %r : $()
+}
+
 // CHECK-LABEL: Address escape information for ignore_copy_addr_and_release:
 // CHECK:       value:  %1 = ref_element_addr %0 : $X, #X.s
 // CHECK-NEXT:    -     %6 = apply %5(%3) : $@convention(thin) (@in Str) -> ()
@@ -477,6 +500,28 @@ bb0:
   return %7 : $()
 }
 
+// CHECK-LABEL: Address escape information for load_rawpointer:
+// CHECK:       value:  %5 = pointer_to_address %4 : $Builtin.RawPointer to [strict] $*Int
+// CHECK-NEXT:    ==>   %8 = apply %7(%5) : $@convention(thin) (@in Int) -> ()
+// CHECK:       End function load_rawpointer
+sil @load_rawpointer : $@convention(thin) () -> () {
+bb0:
+  %0 = alloc_stack $Int
+  %1 = address_to_pointer %0 : $*Int to $Builtin.RawPointer
+  %2 = alloc_stack $Builtin.RawPointer
+  store %1 to %2 : $*Builtin.RawPointer
+  %4 = load %2 : $*Builtin.RawPointer
+  %5 = pointer_to_address %4 : $Builtin.RawPointer to [strict] $*Int
+  fix_lifetime %5 : $*Int
+  %7 = function_ref @indirect_argument : $@convention(thin) (@in Int) -> ()
+  %8 = apply %7(%5) : $@convention(thin) (@in Int) -> ()
+  dealloc_stack %2 : $*Builtin.RawPointer
+  dealloc_stack %0 : $*Int
+  %r = tuple ()
+  return %r : $()
+}
+
+
 // CHECK-LABEL: Address escape information for test_addr_alias1:
 // CHECK:      pair 0 - 1
 // CHECK-NEXT:   %2 = ref_element_addr %0 : $XandIntClass, #XandIntClass.x

test/SILOptimizer/escape_effects.sil

@@ -57,7 +57,7 @@ sil [ossa] @unknown_function : $@convention(thin) (@owned X) -> ()
 sil [ossa] @unknown_function_y : $@convention(thin) (@owned Y) -> ()
 
 // CHECK-LABEL: sil [ossa] @test_basic
-// CHECK-NEXT:  [%1: escape v** => %3.v**]
+// CHECK-NEXT:  [%1: escape v** -> %3.v**]
 // CHECK-NEXT:  [%2: noescape, escape c*.v** -> %r.v**]
 // CHECK-NEXT:  [%3: noescape]
 // CHECK-NEXT:  {{^[^[]}}
@@ -201,7 +201,7 @@ sil [ossa] @self_arg_callee : $@convention(thin) (@inout Str) -> () {
 }
 
 // CHECK-LABEL: sil [ossa] @test_self_arg_escape
-// CHECK-NEXT:  [%0: escape v** => %0.v**]
+// CHECK-NEXT:  [%0: escape v** -> %0.v**]
 // CHECK-NEXT:  {{^[^[]}}
 sil [ossa] @test_self_arg_escape : $@convention(thin) (@inout Str) -> () {
 bb0(%0 : $*Str):
@@ -212,7 +212,7 @@ bb0(%0 : $*Str):
 }
 
 // CHECK-LABEL: sil [ossa] @test_self_arg_escape_with_copy
-// CHECK-NEXT:  [%0: escape v** => %0.s1.0.v**]
+// CHECK-NEXT:  [%0: escape v** -> %0.s1.0.v**]
 // CHECK-NEXT:  {{^[^[]}}
 sil [ossa] @test_self_arg_escape_with_copy : $@convention(thin) (@inout Str) -> () {
 bb0(%0 : $*Str):
@@ -225,7 +225,7 @@ bb0(%0 : $*Str):
 }
 
 // CHECK-LABEL: sil [ossa] @test_self_arg_escape_with_bidirectional_copy
-// CHECK-NEXT:  [%0: escape v** => %0.v**]
+// CHECK-NEXT:  [%0: escape v** -> %0.v**]
 // CHECK-NEXT:  {{^[^[]}}
 sil [ossa] @test_self_arg_escape_with_bidirectional_copy : $@convention(thin) (@inout Str) -> () {
 bb0(%0 : $*Str):
@@ -240,7 +240,7 @@ bb0(%0 : $*Str):
 
 // CHECK-LABEL: sil [ossa] @test_content_to_arg_addr
 // CHECK-NEXT:  [%0: noescape **]
-// CHECK-NEXT:  [%1: noescape, escape c*.v** => %0.v**]
+// CHECK-NEXT:  [%1: noescape, escape c*.v** -> %0.v**]
 // CHECK-NEXT:  {{^[^[]}}
 sil [ossa] @test_content_to_arg_addr : $@convention(thin) (@guaranteed Y) -> @out Str {
 bb0(%0 : $*Str, %1 : @guaranteed $Y):
@@ -331,7 +331,7 @@ bb0(%0 : $*Z, %1 : $Y):
 }
 
 // CHECK-LABEL: sil @store_to_content
-// CHECK-NEXT:  [%0: escape => %r, escape c*.v** => %r.c*.v**]
+// CHECK-NEXT:  [%0: escape => %r, escape c*.v** -> %r.c*.v**]
 // CHECK-NEXT:  {{^[^[]}}
 sil @store_to_content : $@convention(thin) (@owned Z, @guaranteed Y) -> @owned Z {
 bb0(%0 : $Z, %1 : $Y):
@@ -372,3 +372,15 @@ bb0(%0 : $*Z, %1 : $Y):
   %5 = tuple ()
   return %5 : $()
 }
+
+// CHECK-LABEL: sil @store_to_class_field
+// CHECK-NEXT:  [%0: escape v** -> %r.c0.v**, escape v**.c*.v** -> %r.c0.v**.c*.v**]
+// CHECK-NEXT:  {{^[^[]}}
+sil @store_to_class_field : $@convention(thin) (@in_guaranteed Str) -> @owned Y {
+bb0(%0 : $*Str):
+  %1 = alloc_ref $Y
+  %2 = ref_element_addr %1 : $Y, #Y.s
+  copy_addr %0 to [init] %2 : $*Str
+  return %1 : $Y
+}
+

test/SILOptimizer/escape_info.sil

@@ -93,8 +93,8 @@ sil [ossa] @arg_to_return : $@convention(thin) (@owned X) -> @owned Str {
 [%0: escape => %r.s0]
 }
 sil [ossa] @arg_to_arg : $@convention(thin) (@owned X) -> @out Str {
-[%0: noescape]
-[%1: escape => %0.s0]
+[%0: noescape v**]
+[%1: escape -> %0.s0]
 }
 sil [ossa] @content_of_arg_to_return : $@convention(thin) (@owned Z) -> @owned Y {
 [%0: noescape, escape c* => %r]

test/SILOptimizer/release-hoisting.swift

@@ -0,0 +1,35 @@
+// RUN: %target-swift-frontend -O -module-name=test -emit-sil %s | %FileCheck %s
+
+protocol P {}
+
+public struct S {
+  let p: P // force S to be an address-only type
+  let i: Int
+}
+
+public final class X {
+  let s: S
+
+  init(s: S) {
+    self.s = s
+  }
+}
+
+@inline(never)
+func createX(s: S) -> X {
+  return X(s: s)
+}
+
+// Test that the load is a barrier for release hoisting and that the strong_release is not hoisted above the load.
+
+// CHECK-LABEL: sil @$s4test6testit1sSiAA1SV_tF
+// CHECK:         [[X:%[0-9]+]] = apply
+// CHECK:         [[R:%[0-9]+]] = load
+// CHECK:         strong_release [[X]]
+// CHECK:         return [[R]]
+// CHECK:       } // end sil function '$s4test6testit1sSiAA1SV_tF'
+public func testit(s: S) -> Int {
+  let x = createX(s: s)
+  return x.s.i
+}
+