Merge pull request #63686 from eeckstein/effects-refactoring

some refactoring in Effects and ComputeEscapeEffects

Author: eeckstein

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeEscapeEffects.swift

@@ -85,61 +85,7 @@ func addArgEffects(_ arg: FunctionArgument, argPath ap: SmallProjectionPath,
   // containing one or more references.
   let argPath = arg.type.isClass ? ap : ap.push(.anyValueFields)
 
-  struct ArgEffectsVisitor : EscapeVisitorWithResult {
-    enum EscapeDestination {
-      case notSet
-      case toReturn(SmallProjectionPath)
-      case toArgument(Int, SmallProjectionPath) // argument index, path
-    }
-    var result = EscapeDestination.notSet
-    
-    mutating func visitUse(operand: Operand, path: EscapePath) -> UseResult {
-      if operand.instruction is ReturnInst {
-        // The argument escapes to the function return
-        if path.followStores {
-          // The escaping path must not introduce a followStores.
-          return .abort
-        }
-        switch result {
-          case .notSet:
-            result = .toReturn(path.projectionPath)
-          case .toReturn(let oldPath):
-            result = .toReturn(oldPath.merge(with: path.projectionPath))
-          case .toArgument:
-            return .abort
-        }
-        return .ignore
-      }
-      if isOperandOfRecursiveCall(operand) {
-        return .ignore
-      }
-      return .continueWalk
-    }
-    
-    mutating func visitDef(def: Value, path: EscapePath) -> DefResult {
-      guard let destArg = def as? FunctionArgument else {
-        return .continueWalkUp
-      }
-      // The argument escapes to another argument (e.g. an out or inout argument)
-      if path.followStores {
-        // The escaping path must not introduce a followStores.
-        return .abort
-      }
-      let argIdx = destArg.index
-      switch result {
-        case .notSet:
-          result = .toArgument(argIdx, path.projectionPath)
-        case .toArgument(let oldArgIdx, let oldPath) where oldArgIdx == argIdx:
-          result = .toArgument(argIdx, oldPath.merge(with: path.projectionPath))
-        default:
-          return .abort
-      }
-      return .walkDown
-    }
-  }
-  
-  guard let result = arg.at(argPath).visitByWalkingDown(using: ArgEffectsVisitor(),
-                                                        context) else {
+  guard let result = arg.at(argPath).visitByWalkingDown(using: ArgEffectsVisitor(), context) else {
     return false
   }
 
@@ -152,15 +98,17 @@ func addArgEffects(_ arg: FunctionArgument, argPath ap: SmallProjectionPath,
   switch result {
   case .notSet:
     effect = EscapeEffects.ArgumentEffect(.notEscaping, argumentIndex: arg.index, pathPattern: argPath)
+
   case .toReturn(let toPath):
-    let exclusive = isExclusiveEscapeToReturn(fromArgument: arg, fromPath: argPath,
-                                              toPath: toPath, returnInst: returnInst, context)
-    effect = EscapeEffects.ArgumentEffect(.escapingToReturn(toPath, exclusive),
+    let visitor = IsExclusiveReturnEscapeVisitor(argument: arg, argumentPath: argPath, returnPath: toPath)
+    let exclusive = visitor.isExclusiveEscape(returnInst: returnInst, context)
+    effect = EscapeEffects.ArgumentEffect(.escapingToReturn(toPath: toPath, isExclusive: exclusive),
                                           argumentIndex: arg.index, pathPattern: argPath)
+
   case .toArgument(let toArgIdx, let toPath):
     // 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),
+    effect = EscapeEffects.ArgumentEffect(.escapingToArgument(toArgumentIndex: toArgIdx, toPath: toPath),
                                           argumentIndex: arg.index, pathPattern: argPath)
   }
   newEffects.append(effect)
@@ -176,8 +124,10 @@ private func getArgIndicesWithDefinedEscapingEffects(of function: Function) -> S
 
     argsWithDefinedEffects.insert(effect.argumentIndex)
     switch effect.kind {
-      case .notEscaping, .escapingToReturn:         break
-      case .escapingToArgument(let toArgIdx, _, _): argsWithDefinedEffects.insert(toArgIdx)
+    case .notEscaping, .escapingToReturn:
+      break
+    case .escapingToArgument(let toArgIdx, _):
+      argsWithDefinedEffects.insert(toArgIdx)
     }
   }
   return argsWithDefinedEffects
@@ -197,55 +147,105 @@ private func isOperandOfRecursiveCall(_ op: Operand) -> Bool {
   return false
 }
 
-/// 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 : EscapeVisitorWithResult {
-    let fromArgument: Argument
-    let fromPath: SmallProjectionPath
-    let toPath: SmallProjectionPath
-    var result = false
-    
-    mutating func visitUse(operand: Operand, path: EscapePath) -> UseResult {
-      switch operand.instruction {
-      case is ReturnInst:
-        if path.followStores { return .abort }
-        if path.projectionPath.matches(pattern: toPath) {
-          return .ignore
-        }
+private struct ArgEffectsVisitor : EscapeVisitorWithResult {
+  enum EscapeDestination {
+    case notSet
+    case toReturn(SmallProjectionPath)
+    case toArgument(Int, SmallProjectionPath) // argument index, path
+  }
+  var result = EscapeDestination.notSet
+
+  mutating func visitUse(operand: Operand, path: EscapePath) -> UseResult {
+    if operand.instruction is ReturnInst {
+      // The argument escapes to the function return
+      if path.followStores {
+        // The escaping path must not introduce a followStores.
         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 {
+      }
+      switch result {
+        case .notSet:
+          result = .toReturn(path.projectionPath)
+        case .toReturn(let oldPath):
+          result = .toReturn(oldPath.merge(with: path.projectionPath))
+        case .toArgument:
           return .abort
-        }
-      default:
-        break
       }
-      return .continueWalk
+      return .ignore
     }
-    
-    mutating func visitDef(def: Value, path: EscapePath) -> DefResult {
-      guard let arg = def as? FunctionArgument else {
-        return .continueWalkUp
-      }
+    if isOperandOfRecursiveCall(operand) {
+      return .ignore
+    }
+    return .continueWalk
+  }
+
+  mutating func visitDef(def: Value, path: EscapePath) -> DefResult {
+    guard let destArg = def as? FunctionArgument else {
+      return .continueWalkUp
+    }
+    // The argument escapes to another argument (e.g. an out or inout argument)
+    if path.followStores {
+      // The escaping path must not introduce a followStores.
+      return .abort
+    }
+    let argIdx = destArg.index
+    switch result {
+      case .notSet:
+        result = .toArgument(argIdx, path.projectionPath)
+      case .toArgument(let oldArgIdx, let oldPath) where oldArgIdx == argIdx:
+        result = .toArgument(argIdx, oldPath.merge(with: path.projectionPath))
+      default:
+        return .abort
+    }
+    return .walkDown
+  }
+}
+
+/// Returns true if when walking up from the return instruction, the `fromArgument`
+/// is the one and only argument which is reached - with a matching `fromPath`.
+private struct IsExclusiveReturnEscapeVisitor : EscapeVisitorWithResult {
+  let argument: Argument
+  let argumentPath: SmallProjectionPath
+  let returnPath: SmallProjectionPath
+  var result = false
+
+  func isExclusiveEscape(returnInst: ReturnInst, _ context: FunctionPassContext) -> Bool {
+    return returnInst.operand.at(returnPath).visit(using: self, context) ?? false
+  }
+
+  mutating func visitUse(operand: Operand, path: EscapePath) -> UseResult {
+    switch operand.instruction {
+    case is ReturnInst:
       if path.followStores { return .abort }
-      if arg == fromArgument && path.projectionPath.matches(pattern: fromPath) {
-        result = true
-        return .walkDown
+      if path.projectionPath.matches(pattern: returnPath) {
+        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
+  }
+
+  mutating func visitDef(def: Value, path: EscapePath) -> DefResult {
+    guard let arg = def as? FunctionArgument else {
+      return .continueWalkUp
+    }
+    if path.followStores { return .abort }
+    if arg == argument && path.projectionPath.matches(pattern: argumentPath) {
+      result = true
+      return .walkDown
     }
+    return .abort
   }
-  let visitor = IsExclusiveReturnEscapeVisitor(fromArgument: fromArgument, fromPath: fromPath, toPath: toPath)
-  return returnInst.operand.at(toPath).visit(using: visitor, context) ?? false
 }

SwiftCompilerSources/Sources/Optimizer/Utilities/EscapeUtils.swift

@@ -594,7 +594,7 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
     var matched = false
     for effect in effects.escapeEffects.arguments {
       switch effect.kind {
-      case .escapingToArgument(let toArgIdx, let toPath, _):
+      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 {

SwiftCompilerSources/Sources/SIL/Effects.swift

@@ -240,9 +240,9 @@ public struct EscapeEffects : CustomStringConvertible, NoReflectionChildren {
       ///    [%0: escape s1 => %r]   // field 2 of argument 0 exclusively escapes via return.
       ///    [%0: escape s1 -> %r]   // field 2 of argument 0 - and other values - escape via return
       ///
-      /// The "exclusive" flag (= second payload) is true if only the argument escapes,
-      /// but nothing else escapes to the return value.
-      /// For example, "exclusive" is true for the following function:
+      /// The `isExclusive` flag is true if only the argument escapes, but nothing else escapes to
+      /// the return value.
+      /// For example, `isExclusive` is true for the following function:
       ///
       ///   @_effect(escaping c => return)
       ///   func exclusiveEscape(_ c: Class) -> Class { return c }
@@ -254,17 +254,40 @@ public struct EscapeEffects : CustomStringConvertible, NoReflectionChildren {
       ///   @_effect(escaping c -> return)
       ///   func notExclusiveEscape(_ c: Class) -> Class { return cond ? c : global }
       ///
-      case escapingToReturn(SmallProjectionPath, Bool)         // toPath, exclusive
+      /// Also, if `isExclusive` is true, there must not be a store in the chain from the argument to
+      /// the return, e.g.
+      ///
+      ///   @_effect(escaping x -> return)
+      ///   func notExclusiveEscape(_ s: String) -> Class {
+      ///     c = new Class()
+      ///     c.s = s             // Store, which prevents the effect to be `isExclusive`
+      ///     return s
+      ///   }
+      case escapingToReturn(toPath: SmallProjectionPath, isExclusive: Bool)
 
       /// Like `escapingToReturn`, but the argument escapes to another argument.
       ///
       /// Example: The argument effects of
+      ///
       ///   func argToArgEscape(_ r: inout Class, _ c: Class) { r = c }
       ///
       /// would be
       ///    [%1: escape => %0]   // Argument 1 escapes to argument 0
       ///
-      case escapingToArgument(Int, SmallProjectionPath, Bool)  // toArgumentIndex, toPath, exclusive
+      /// It's not allowed that the argument (or a derived value) is _stored_ to an object which is loaded from somewhere.
+      /// In the following example `c` is loaded from the indirect inout argument and `s` is stored to a field of `c`:
+      ///
+      ///   func noEscapeEffect(_ c: inout Class, s: String) {
+      ///     c.s = s
+      ///   }
+      ///
+      /// In this case there is no escaping effect from `s` to `c`.
+      /// Note that theoretically this rule also applies to the `escapingToReturn` effect, but it's impossible
+      /// to construct such a situation for an argument which is only escaping to the function return.
+      ///
+      /// The `escapingToArgument` doesn't have an `isExclusive` flag, because an argument-to-argument escape
+      /// always involves a store, which makes an exclusive escape impossible.
+      case escapingToArgument(toArgumentIndex: Int, toPath: SmallProjectionPath)
     }
 
     /// To which argument does this effect apply to?
@@ -309,19 +332,19 @@ public struct EscapeEffects : CustomStringConvertible, NoReflectionChildren {
           if resultArgDelta != 1 {
             return nil
           }
-          self.kind = .escapingToArgument(0, toPath, exclusive)
+          self.kind = .escapingToArgument(toArgumentIndex: 0, toPath: toPath)
         } else {
-          self.kind = .escapingToReturn(toPath, exclusive)
+          self.kind = .escapingToReturn(toPath: toPath, isExclusive: exclusive)
         }
-      case .escapingToArgument(let toArgIdx, let toPath, let exclusive):
+      case .escapingToArgument(let toArgIdx, let toPath):
         let resultingToArgIdx = toArgIdx + resultArgDelta
         if resultingToArgIdx < 0 {
           if resultingToArgIdx != -1 {
             return nil
           }
-          self.kind = .escapingToReturn(toPath, exclusive)
+          self.kind = .escapingToReturn(toPath: toPath, isExclusive: false)
         } else {
-          self.kind = .escapingToArgument(resultingToArgIdx, toPath, exclusive)
+          self.kind = .escapingToArgument(toArgumentIndex: resultingToArgIdx, toPath: toPath)
         }
       }
     }
@@ -341,9 +364,9 @@ public struct EscapeEffects : CustomStringConvertible, NoReflectionChildren {
         case .escapingToReturn(let toPath, let exclusive):
           let toPathStr = (toPath.isEmpty ? "" : ".\(toPath)")
           return "escape\(patternStr) \(exclusive ? "=>" : "->") %r\(toPathStr)"
-        case .escapingToArgument(let toArgIdx, let toPath, let exclusive):
+        case .escapingToArgument(let toArgIdx, let toPath):
           let toPathStr = (toPath.isEmpty ? "" : ".\(toPath)")
-          return "escape\(patternStr) \(exclusive ? "=>" : "->") %\(toArgIdx)\(toPathStr)"
+          return "escape\(patternStr) -> %\(toArgIdx)\(toPathStr)"
       }
     }
 
@@ -642,16 +665,21 @@ extension StringParser {
             try throwError("multi-value returns not supported yet")
           }
           let toPath = try parsePathPatternFromSource(for: function, type: function.argumentTypes[0])
-          return EscapeEffects.ArgumentEffect(.escapingToArgument(0, toPath, exclusive),
+
+          // Exclusive escapes are ignored for indirect return values.
+          return EscapeEffects.ArgumentEffect(.escapingToArgument(toArgumentIndex: 0, toPath: toPath),
                                       argumentIndex: fromArgIdx, pathPattern: fromPath, isDerived: false)
         }
         let toPath = try parsePathPatternFromSource(for: function, type: function.resultType)
-        return EscapeEffects.ArgumentEffect(.escapingToReturn(toPath, exclusive),
+        return EscapeEffects.ArgumentEffect(.escapingToReturn(toPath: toPath, isExclusive: exclusive),
                                     argumentIndex: fromArgIdx, pathPattern: fromPath, isDerived: false)
       }
+      if exclusive {
+        try throwError("exclusive escapes to arguments are not supported")
+      }
       let toArgIdx = try parseArgumentIndexFromSource(for: function, params: params)
       let toPath = try parsePathPatternFromSource(for: function, type: function.argumentTypes[toArgIdx])
-      return EscapeEffects.ArgumentEffect(.escapingToArgument(toArgIdx, toPath, exclusive),
+      return EscapeEffects.ArgumentEffect(.escapingToArgument(toArgumentIndex: toArgIdx, toPath: toPath),
                                   argumentIndex: fromArgIdx, pathPattern: fromPath, isDerived: false)
     }
     try throwError("unknown effect")
@@ -715,12 +743,15 @@ extension StringParser {
         let effect: EscapeEffects.ArgumentEffect
         if consume("%r") {
           let toPath = consume(".") ? try parseProjectionPathFromSIL() : SmallProjectionPath()
-          effect = EscapeEffects.ArgumentEffect(.escapingToReturn(toPath, exclusive),
+          effect = EscapeEffects.ArgumentEffect(.escapingToReturn(toPath: toPath, isExclusive: exclusive),
                                                 argumentIndex: argumentIndex, pathPattern: fromPath, isDerived: isDerived)
         } else {
+          if exclusive {
+            try throwError("exclusive escapes to arguments are not supported")
+          }
           let toArgIdx = try parseArgumentIndexFromSIL()
           let toPath = consume(".") ? try parseProjectionPathFromSIL() : SmallProjectionPath()
-          effect = EscapeEffects.ArgumentEffect(.escapingToArgument(toArgIdx, toPath, exclusive),
+          effect = EscapeEffects.ArgumentEffect(.escapingToArgument(toArgumentIndex: toArgIdx, toPath: toPath),
                                                 argumentIndex: argumentIndex, pathPattern: fromPath, isDerived: isDerived)
         }
         effects.escapeEffects.arguments.append(effect)

test/SILGen/effectsattr.swift

@@ -20,7 +20,7 @@
 @_effects(notEscaping t.**) @_silgen_name("func5") func func5<T>(_ t: T) { }
 
 //CHECK-LABEL: sil hidden [ossa] @func6
-//CHECK-NEXT:  [%1: escape! v**.c* => %0.v**]
+//CHECK-NEXT:  [%1: escape! v**.c* -> %0.v**]
 //CHECK-NEXT:  {{^[^[]}}
 @_effects(escaping t.value**.class* => return.value**) @_silgen_name("func6") func func6<T>(_ t: T) -> T { }