Merge pull request #61641 from eeckstein/side-effect-improvements

Follow-up changes for function effects

Author: eeckstein

SwiftCompilerSources/Sources/Basic/Utils.swift

@@ -76,6 +76,8 @@ public struct StringRef : CustomStringConvertible, NoReflectionChildren {
   }
 
   public static func !=(lhs: StringRef, rhs: StaticString) -> Bool { !(lhs == rhs) }
+
+  public static func ~=(pattern: StaticString, value: StringRef) -> Bool { value == pattern }
 }
 
 //===----------------------------------------------------------------------===//

SwiftCompilerSources/Sources/Optimizer/Analysis/CalleeAnalysis.swift

@@ -21,6 +21,13 @@ public struct CalleeAnalysis {
       // isDeinitBarrierFn:
       { (inst : BridgedInstruction, bca: BridgedCalleeAnalysis) -> Bool in
         return inst.instruction.isDeinitBarrier(bca.analysis)
+      },
+      // getMemBehaviorFn
+      { (bridgedCtxt: BridgedPassContext, bridgedApply: BridgedInstruction, observeRetains: Bool) -> BridgedMemoryBehavior in
+        let context = PassContext(_bridged: bridgedCtxt)
+        let apply = bridgedApply.instruction as! ApplySite
+        let e = context.calleeAnalysis.getSideEffects(of: apply)
+        return e.getMemBehavior(observeRetains: observeRetains)
       }
     )
   }
@@ -52,6 +59,40 @@ public struct CalleeAnalysis {
     }
     return FunctionArray(bridged: bridgedDtors)
   }
+
+  /// Returns the global (i.e. not argument specific) side effects of an apply.
+  public func getSideEffects(of apply: ApplySite) -> SideEffects.GlobalEffects {
+    guard let callees = getCallees(callee: apply.callee) else {
+      return .worstEffects
+    }
+
+    var result = SideEffects.GlobalEffects()
+    for callee in callees {
+      let calleeEffects = callee.getSideEffects()
+      result.merge(with: calleeEffects)
+    }
+    return result
+  }
+
+  /// Returns the argument specific side effects of an apply.
+  public func getSideEffects(of apply: ApplySite, forArgument argumentIdx: Int, path: SmallProjectionPath) -> SideEffects.GlobalEffects {
+    let calleeArgIdx = apply.calleeArgIndex(callerArgIndex: argumentIdx)
+    let convention = apply.getArgumentConvention(calleeArgIndex: calleeArgIdx)
+    let argument = apply.arguments[argumentIdx].at(path)
+
+    guard let callees = getCallees(callee: apply.callee) else {
+      return .worstEffects.restrictedTo(argument: argument, withConvention: convention)
+    }
+  
+    var result = SideEffects.GlobalEffects()
+    for callee in callees {
+      let calleeEffects = callee.getSideEffects(forArgument: argument,
+                                                atIndex: calleeArgIdx,
+                                                withConvention: convention)
+      result.merge(with: calleeEffects)
+    }
+    return result.restrictedTo(argument: argument, withConvention: convention)
+  }
 }
 
 extension FullApplySite {

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeEscapeEffects.swift

@@ -28,15 +28,14 @@ import SIL
 let computeEscapeEffects = FunctionPass(name: "compute-escape-effects", {
   (function: Function, context: PassContext) in
 
-  var newEffects = Stack<EscapeEffects.ArgumentEffect>(context)
-  defer { newEffects.deinitialize() }
+  var newEffects = function.effects.escapeEffects.arguments.filter {!$0.isDerived }
 
   let returnInst = function.returnInstruction
   let argsWithDefinedEffects = getArgIndicesWithDefinedEscapingEffects(of: function)
 
   for arg in function.arguments {
     // We are not interested in arguments with trivial types.
-    if !arg.type.isNonTrivialOrContainsRawPointer(in: function) { continue }
+    if arg.hasTrivialNonPointerType { continue }
 
     // Also, we don't want to override defined effects.
     if argsWithDefinedEffects.contains(arg.index) { continue }
@@ -52,7 +51,7 @@ let computeEscapeEffects = FunctionPass(name: "compute-escape-effects", {
                                                     context) {
       let effect = EscapeEffects.ArgumentEffect(.notEscaping, argumentIndex: arg.index,
                                                 pathPattern: SmallProjectionPath(.anything))
-      newEffects.push(effect)
+      newEffects.append(effect)
       continue
     }
 
@@ -65,17 +64,22 @@ let computeEscapeEffects = FunctionPass(name: "compute-escape-effects", {
     }
   }
 
+  // Don't modify the effects if they didn't change. This avoids sending a change notification
+  // which can trigger unnecessary other invalidations.
+  if newEffects == function.effects.escapeEffects.arguments {
+    return
+  }
+
   context.modifyEffects(in: function) { (effects: inout FunctionEffects) in
-    effects.escapeEffects.arguments = effects.escapeEffects.arguments.filter { !$0.isDerived }
-    effects.escapeEffects.arguments.append(contentsOf: newEffects)
+    effects.escapeEffects.arguments = newEffects
   }
 })
 
 
 /// Returns true if an argument effect was added.
 private
 func addArgEffects(_ arg: FunctionArgument, argPath ap: SmallProjectionPath,
-                   to newEffects: inout Stack<EscapeEffects.ArgumentEffect>,
+                   to newEffects: inout [EscapeEffects.ArgumentEffect],
                    _ returnInst: ReturnInst?, _ context: PassContext) -> Bool {
   // Correct the path if the argument is not a class reference itself, but a value type
   // containing one or more references.
@@ -159,7 +163,7 @@ func addArgEffects(_ arg: FunctionArgument, argPath ap: SmallProjectionPath,
     effect = EscapeEffects.ArgumentEffect(.escapingToArgument(toArgIdx, toPath, exclusive),
                                           argumentIndex: arg.index, pathPattern: argPath)
   }
-  newEffects.push(effect)
+  newEffects.append(effect)
   return true
 }
 

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeSideEffects.swift

@@ -66,6 +66,14 @@ let computeSideEffects = FunctionPass(name: "compute-side-effects", {
     collectedEffects.addEffectsForEcapingArgument(argument: argument)
   }
 
+  // Don't modify the effects if they didn't change. This avoids sending a change notification
+  // which can trigger unnecessary other invalidations.
+  if let existingEffects = function.effects.sideEffects,
+     existingEffects.arguments == collectedEffects.argumentEffects,
+     existingEffects.global == collectedEffects.globalEffects {
+    return
+  }
+
   // Finally replace the function's side effects.
   context.modifyEffects(in: function) { (effects: inout FunctionEffects) in
     effects.sideEffects = SideEffects(arguments: collectedEffects.argumentEffects, global: collectedEffects.globalEffects)
@@ -160,6 +168,11 @@ private struct CollectedEffects {
       is CondFailInst:
       break
 
+    case is BeginCOWMutationInst, is IsUniqueInst, is IsEscapingClosureInst:
+      // Model reference count reading as "destroy" for now. Although we could intoduce a "read-refcount"
+      // effect, it would not give any significant benefit in any of our current optimizations.
+      addEffects(.destroy, to: inst.operands[0].value, fromInitialPath: SmallProjectionPath(.anyValueFields))
+
     default:
       if inst.mayRelease {
         globalEffects = .worstEffects
@@ -195,15 +208,17 @@ private struct CollectedEffects {
 
     if escapeWalker.hasUnknownUses(argument: argument) {
       // Worst case: we don't know anything about how the argument escapes.
-      addEffects(globalEffects.restrictedTo(argument: argument, withConvention: argument.convention), to: argument)
+      addEffects(globalEffects.restrictedTo(argument: argument.at(SmallProjectionPath(.anything)),
+                                            withConvention: argument.convention), to: argument)
 
     } else if escapeWalker.foundTakingLoad {
       // In most cases we can just ignore loads. But if the load is actually "taking" the
       // underlying memory allocation, we must consider this as a "destroy", because we don't
       // know what's happening with the loaded value. If there is any destroying instruction in the
       // function, it might be the destroy of the loaded value.
       let effects = SideEffects.GlobalEffects(ownership: globalEffects.ownership)
-      addEffects(effects.restrictedTo(argument: argument, withConvention: argument.convention), to: argument)
+      addEffects(effects.restrictedTo(argument: argument.at(SmallProjectionPath(.anything)),
+                                      withConvention: argument.convention), to: argument)
 
     } else if escapeWalker.foundConsumingPartialApply && globalEffects.ownership.destroy {
       // Similar situation with apply instructions which consume the callee closure.
@@ -228,7 +243,7 @@ private struct CollectedEffects {
       } else {
         // The callee doesn't have any computed effects. At least we can do better
         // if it has any defined effect attribute (like e.g. `[readnone]`).
-        globalEffects.merge(with: callee.definedSideEffects)
+        globalEffects.merge(with: callee.definedGlobalEffects)
       }
     }
 
@@ -244,15 +259,12 @@ private struct CollectedEffects {
           if let calleePath = calleeEffect.copy    { addEffects(.copy,    to: argument, fromInitialPath: calleePath) }
           if let calleePath = calleeEffect.destroy { addEffects(.destroy, to: argument, fromInitialPath: calleePath) }
         } else {
-          var calleeEffects = callee.definedSideEffects
-
           let convention = callee.getArgumentConvention(for: calleeArgIdx)
-          if convention.isIndirectIn {
-            calleeEffects.memory.read = true
-          } else if convention == .indirectOut {
-            calleeEffects.memory.write = true
-          }
-          addEffects(calleeEffects.restrictedTo(argument: argument, withConvention: convention), to: argument)
+          let wholeArgument = argument.at(SmallProjectionPath(.anything))
+          let calleeEffects = callee.getSideEffects(forArgument: wholeArgument,
+                                                    atIndex: calleeArgIdx,
+                                                    withConvention: convention)
+          addEffects(calleeEffects.restrictedTo(argument: wholeArgument, withConvention: convention), to: argument)
         }
       }
     }
@@ -266,7 +278,7 @@ private struct CollectedEffects {
     // deallocates an object.
     if let destructors = calleeAnalysis.getDestructors(of: addressOrValue.type) {
       for destructor in destructors {
-        globalEffects.merge(with: destructor.effects.accumulatedSideEffects)
+        globalEffects.merge(with: destructor.getSideEffects())
       }
     } else {
       globalEffects = .worstEffects
@@ -368,7 +380,7 @@ private struct ArgumentEscapingWalker : ValueDefUseWalker, AddressDefUseWalker {
   mutating func hasUnknownUses(argument: FunctionArgument) -> Bool {
     if argument.type.isAddress {
       return walkDownUses(ofAddress: argument, path: UnusedWalkingPath()) == .abortWalk
-    } else if argument.type.isTrivial(in: argument.function) {
+    } else if argument.hasTrivialType {
       return false
     } else {
       return walkDownUses(ofValue: argument, path: UnusedWalkingPath()) == .abortWalk
@@ -407,22 +419,22 @@ private struct ArgumentEscapingWalker : ValueDefUseWalker, AddressDefUseWalker {
     switch inst {
     case let copy as CopyAddrInst:
       if address == copy.sourceOperand &&
-          !address.value.type.isTrivial(in: inst.function) &&
+          !address.value.hasTrivialType &&
           (!function.hasOwnership || copy.isTakeOfSrc) {
         foundTakingLoad = true
       }
       return .continueWalk
 
     case let load as LoadInst:
-      if !address.value.type.isTrivial(in: function) &&
+      if !address.value.hasTrivialType &&
           // In non-ossa SIL we don't know if a load is taking.
           (!function.hasOwnership || load.ownership == .take) {
         foundTakingLoad = true
       }
       return .continueWalk
 
     case is LoadWeakInst, is LoadUnownedInst, is LoadBorrowInst:
-      if !function.hasOwnership && !address.value.type.isTrivial(in: function) {
+      if !function.hasOwnership && !address.value.hasTrivialType {
         foundTakingLoad = true
       }
       return .continueWalk

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ReleaseDevirtualizer.swift

@@ -149,7 +149,7 @@ private struct FindAllocationOfRelease : ValueUseDefWalker {
     //
     var nonTrivialOperandFound = false
     for operand in def.operands {
-      if !operand.value.type.isTrivial(in: def.function) {
+      if !operand.value.hasTrivialType {
         if nonTrivialOperandFound {
           return .abortWalk
         }

SwiftCompilerSources/Sources/Optimizer/PassManager/PassContext.swift

@@ -119,7 +119,7 @@ struct PassContext {
   }
 
   func modifyEffects(in function: Function, _ body: (inout FunctionEffects) -> ()) {
-    notifyFunctionDataChanged()
+    notifyEffectsChanged()
     function._modifyEffects(body)
   }
 
@@ -139,8 +139,8 @@ struct PassContext {
     PassContext_notifyChanges(_bridged, branchesChanged)
   }
 
-  fileprivate func notifyFunctionDataChanged() {
-    PassContext_notifyChanges(_bridged, functionDataChanged)
+  fileprivate func notifyEffectsChanged() {
+    PassContext_notifyChanges(_bridged, effectsChanged)
   }
 }
 
@@ -243,7 +243,7 @@ extension RefCountingInst {
 
 extension Function {
   func set(needStackProtection: Bool, _ context: PassContext) {
-    context.notifyFunctionDataChanged()
+    context.notifyEffectsChanged()
     SILFunction_setNeedStackProtection(bridged, needStackProtection ? 1 : 0)
   }
 }

SwiftCompilerSources/Sources/Optimizer/Utilities/EscapeUtils.swift

@@ -197,12 +197,11 @@ extension EscapeVisitor {
   }
 
   func hasRelevantType(_ value: Value, at path: SmallProjectionPath, analyzeAddresses: Bool) -> Bool {
-    let type = value.type
-    if type.isNonTrivialOrContainsRawPointer(in: value.function) { return true }
+    if !value.hasTrivialNonPointerType { return true }
 
     // For selected addresses we also need to consider trivial types (`value`
     // is a selected address if the path does not contain any class projections).
-    if analyzeAddresses && type.isAddress && !path.hasClassProjection { return true }
+    if analyzeAddresses && value.type.isAddress && !path.hasClassProjection { return true }
     return false
   }
 }
@@ -542,14 +541,15 @@ fileprivate struct EscapeWalker<V: EscapeVisitor> : ValueDefUseWalker,
       let svi = instruction as! SingleValueInstruction
 
       // Even when analyzing addresses, a loaded trivial value can be ignored.
-      if !svi.type.isNonTrivialOrContainsRawPointer(in: svi.function) { return .continueWalk }
+      if svi.hasTrivialNonPointerType { return .continueWalk }
       return walkDownUses(ofValue: svi, path: path.with(knownType: nil))
     case let atp as AddressToPointerInst:
       return walkDownUses(ofValue: atp, path: path.with(knownType: nil))
     case let ia as IndexAddrInst:
       assert(operand.index == 0)
       return walkDownUses(ofAddress: ia, path: path.with(knownType: nil))
-    case is DeallocStackInst, is InjectEnumAddrInst, is FixLifetimeInst, is EndBorrowInst, is EndAccessInst:
+    case is DeallocStackInst, is InjectEnumAddrInst, is FixLifetimeInst, is EndBorrowInst, is EndAccessInst,
+         is DebugValueInst:
       return .continueWalk
     default:
       return isEscaping

SwiftCompilerSources/Sources/SIL/ApplySite.swift

@@ -133,4 +133,11 @@ extension FullApplySite {
     }
     return nil
   }
+
+  /// The number of indirect out arguments.
+  ///
+  /// 0 if the callee has a direct or no return value and 1, if it has an indirect return value.
+  public var numIndirectResultArguments: Int {
+    return FullApplySite_numIndirectResultArguments(bridged)
+  }
 }