Merge pull request #65820 from eeckstein/performance-annotations

Fix several problems with performance annotations

Author: swift-ci

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/CMakeLists.txt

@@ -14,6 +14,7 @@ swift_compiler_sources(Optimizer
   InitializeStaticGlobals.swift
   ObjCBridgingOptimization.swift
   MergeCondFails.swift
+  NamedReturnValueOptimization.swift
   ReleaseDevirtualizer.swift
   SimplificationPasses.swift
   StackPromotion.swift

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/NamedReturnValueOptimization.swift

@@ -0,0 +1,131 @@
+//===--- NamedReturnValueOptimization.swift --------------------------------==//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import SIL
+
+/// Removes a `copy_addr` to an indirect out argument by replacing the source of the copy
+/// (which must be an `alloc_stack`) with the out argument itself.
+///
+/// The following SIL pattern will be optimized:
+///
+///   sil @foo : $@convention(thin) <T> () -> @out T {
+///   bb0(%0 : $*T):
+///     %2 = alloc_stack $T
+///     ...
+///     copy_addr %some_value to [init] %2       // or any other writes to %2
+///     ...
+///   bbN:
+///     copy_addr [take] %2 to [init] %0 : $*T   // the only use of %0
+///     ... // no writes
+///     return
+///
+/// to:
+///
+///   sil @foo : $@convention(thin) <T> (@out T) -> () {
+///   bb0(%0 : $*T):
+///     %2 = alloc_stack $T                      // is dead now
+///     ...
+///     copy_addr %some_value to [init] %0
+///     ...
+///   bbN:
+///     ...
+///     return
+///
+/// This optimization can be done because we know that:
+/// * The out argument dominates all uses of the copy_addr's source (because it's a function argument).
+/// * It's not aliased (by definition). We can't allow aliases to be accessed between the initialization and the return.
+///
+/// This pass shouldn't run before serialization. It might prevent predictable memory optimizations
+/// in a caller after inlining, because the memory location (the out argument = an alloc_stack in the caller)
+/// might be written multiple times after this optimization.
+///
+let namedReturnValueOptimization = FunctionPass(name: "named-return-value-optimization") {
+  (function: Function, context: FunctionPassContext) in
+
+  for outArg in function.arguments[0..<function.numIndirectResultArguments] {
+    if let copyToArg = findCopyForNRVO(for: outArg) {
+      performNRVO(with: copyToArg, context)
+    }
+  }
+}
+
+/// Returns a copy_addr which copies from an alloc_stack to the `outArg` at the end of the function.
+///
+private func findCopyForNRVO(for outArg: FunctionArgument) -> CopyAddrInst? {
+  guard let singleArgUse = outArg.uses.singleNonDebugUse,
+        let copyToArg = singleArgUse.instruction as? CopyAddrInst else {
+    return nil
+  }
+
+  assert(singleArgUse == copyToArg.destinationOperand,
+         "single use of out-argument cannot be the source of a copy")
+
+  // Don't perform NRVO unless the copy is a [take]. This is the easiest way
+  // to determine that the local variable has ownership of its value and ensures
+  // that removing a copy is a reference count neutral operation. For example,
+  // this copy can't be trivially eliminated without adding a retain.
+  //   sil @f : $@convention(thin) (@guaranteed T) -> @out T
+  //   bb0(%in : $*T, %out : $T):
+  //     %local = alloc_stack $T
+  //     store %in to %local : $*T
+  //     copy_addr %local to [init] %out : $*T
+  if !copyToArg.isTakeOfSrc {
+    return nil
+  }
+
+  guard let sourceStackAlloc = copyToArg.source as? AllocStackInst else {
+    return nil
+  }
+
+  // NRVO for alloc_stack [dynamic_lifetime] will invalidate OSSA invariants.
+  if sourceStackAlloc.hasDynamicLifetime && copyToArg.parentFunction.hasOwnership {
+    return nil
+  }
+
+  if !(copyToArg.parentBlock.terminator is ReturnInst) {
+    return nil
+  }
+
+  // This check is overly conservative, because we only need to check if the source
+  // of the copy is not written to. But the copy to the out argument is usually the last
+  // instruction of the function, so it doesn't matter.
+  if isAnyInstructionWritingToMemory(after: copyToArg) {
+    return nil
+  }
+
+  return copyToArg
+}
+
+private func performNRVO(with copy: CopyAddrInst, _ context: FunctionPassContext) {
+  copy.source.uses.replaceAllExceptDealloc(with: copy.destination, context)
+  assert(copy.source == copy.destination)
+  context.erase(instruction: copy)
+}
+
+private func isAnyInstructionWritingToMemory(after: Instruction) -> Bool {
+  var followingInst = after.next
+  while let fi = followingInst {
+    if fi.mayWriteToMemory && !(fi is DeallocStackInst) {
+      return true
+    }
+    followingInst = fi.next
+  }
+  return false
+}
+
+private extension UseList {
+  func replaceAllExceptDealloc(with replacement: Value, _ context: some MutatingContext) {
+    for use in self where !(use.instruction is Deallocation)  {
+      use.set(to: replacement, context)
+    }
+  }
+}

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/SimplificationPasses.swift

@@ -71,9 +71,9 @@ let lateOnoneSimplificationPass = FunctionPass(name: "late-onone-simplification"
 //===--------------------------------------------------------------------===//
 
 
-private func runSimplification(on function: Function, _ context: FunctionPassContext,
-                               preserveDebugInfo: Bool,
-                               _ simplify: (Instruction, SimplifyContext) -> ()) {
+func runSimplification(on function: Function, _ context: FunctionPassContext,
+                       preserveDebugInfo: Bool,
+                       _ simplify: (Instruction, SimplifyContext) -> ()) {
   var worklist = InstructionWorklist(context)
   defer { worklist.deinitialize() }
 

SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/CMakeLists.txt

@@ -17,6 +17,7 @@ swift_compiler_sources(Optimizer
   SimplifyDestructure.swift
   SimplifyGlobalValue.swift
   SimplifyLoad.swift
+  SimplifyPartialApply.swift
   SimplifyStrongRetainRelease.swift
   SimplifyStructExtract.swift
   SimplifyTupleExtract.swift

SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyApply.swift

@@ -14,48 +14,18 @@ import SIL
 
 extension ApplyInst : OnoneSimplifyable {
   func simplify(_ context: SimplifyContext) {
-    tryReplaceTrivialApplyOfPartialApply(context)
+    _ = context.tryDevirtualize(apply: self, isMandatory: false)
   }
 }
 
-private extension ApplyInst {
-  func tryReplaceTrivialApplyOfPartialApply(_ context: SimplifyContext) {
-    guard let pa = callee as? PartialApplyInst else {
-      return
-    }
-
-    if pa.referencedFunction == nil {
-      return
-    }
-
-    // Currently we don't handle generic closures. For Onone this is good enough.
-    // TODO: handle it once we replace the SILCombine simplification with this.
-    if !allArgumentsAreTrivial(arguments) {
-      return
-    }
-
-    if !allArgumentsAreTrivial(pa.arguments) {
-      return
-    }
-
-    if !substitutionMap.isEmpty {
-      return
-    }
-
-    let allArgs = Array<Value>(arguments) + Array<Value>(pa.arguments)
-    let builder = Builder(before: self, context)
-    let newApply = builder.createApply(function: pa.callee, pa.substitutionMap, arguments: allArgs,
-                                       isNonThrowing: isNonThrowing, isNonAsync: isNonAsync,
-                                       specializationInfo: specializationInfo)
-    uses.replaceAll(with: newApply, context)
-    context.erase(instruction: self)
-
-    if context.tryDeleteDeadClosure(closure: pa) {
-      context.notifyInvalidatedStackNesting()
-    }
+extension TryApplyInst : OnoneSimplifyable {
+  func simplify(_ context: SimplifyContext) {
+    _ = context.tryDevirtualize(apply: self, isMandatory: false)
   }
 }
 
-private func allArgumentsAreTrivial(_ args: LazyMapSequence<OperandArray, Value>) -> Bool {
-  return !args.contains { !$0.hasTrivialType }
+extension BeginApplyInst : OnoneSimplifyable {
+  func simplify(_ context: SimplifyContext) {
+    _ = context.tryDevirtualize(apply: self, isMandatory: false)
+  }
 }

SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyBuiltin.swift

@@ -24,6 +24,10 @@ extension BuiltinInst : OnoneSimplifyable {
         optimizeIsSameMetatype(context)
       case .Once:
         optimizeBuiltinOnce(context)
+      case .CanBeObjCClass:
+        optimizeCanBeClass(context)
+      case .AssertConf:
+        optimizeAssertConfig(context)
       default:
         if let literal = constantFold(context) {
           uses.replaceAll(with: literal, context)
@@ -90,6 +94,43 @@ private extension BuiltinInst {
     }
     return nil
   }
+
+  func optimizeCanBeClass(_ context: SimplifyContext) {
+    guard let ty = substitutionMap.replacementTypes[0] else {
+      return
+    }
+    let literal: IntegerLiteralInst
+    switch ty.canBeClass {
+    case .IsNot:
+      let builder = Builder(before: self, context)
+      literal = builder.createIntegerLiteral(0,  type: type)
+    case .Is:
+      let builder = Builder(before: self, context)
+      literal = builder.createIntegerLiteral(1,  type: type)
+    case .CanBe:
+      return
+    default:
+      fatalError()
+    }
+    uses.replaceAll(with: literal, context)
+    context.erase(instruction: self)
+  }
+
+  func optimizeAssertConfig(_ context: SimplifyContext) {
+    let literal: IntegerLiteralInst
+    switch context.options.assertConfiguration {
+    case .enabled:
+      let builder = Builder(before: self, context)
+      literal = builder.createIntegerLiteral(1,  type: type)
+    case .disabled:
+      let builder = Builder(before: self, context)
+      literal = builder.createIntegerLiteral(0,  type: type)
+    default:
+      return
+    }
+    uses.replaceAll(with: literal, context)
+    context.erase(instruction: self)
+  }
 }
 
 private func typesOfValuesAreEqual(_ lhs: Value, _ rhs: Value, in function: Function) -> Bool? {

SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyPartialApply.swift

@@ -0,0 +1,34 @@
+//===--- SimplifyPartialApply.swift ---------------------------------------===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+
+import SIL
+
+extension PartialApplyInst : OnoneSimplifyable {
+  func simplify(_ context: SimplifyContext) {
+    let optimizedApplyOfPartialApply = context.tryOptimizeApplyOfPartialApply(closure: self)
+    if optimizedApplyOfPartialApply {
+      context.notifyInvalidatedStackNesting()
+    }
+
+    if context.preserveDebugInfo && uses.contains(where: { $0.instruction is DebugValueInst }) {
+      return
+    }
+
+    // Try to delete the partial_apply.
+    // In case it became dead because of tryOptimizeApplyOfPartialApply, we don't
+    // need to copy all arguments again (to extend their lifetimes), because it
+    // was already done in tryOptimizeApplyOfPartialApply.
+    if context.tryDeleteDeadClosure(closure: self, needKeepArgsAlive: !optimizedApplyOfPartialApply) {
+      context.notifyInvalidatedStackNesting()
+    }
+  }
+}

SwiftCompilerSources/Sources/Optimizer/ModulePasses/CMakeLists.txt

@@ -7,6 +7,7 @@
 # See http://swift.org/CONTRIBUTORS.txt for Swift project authors
 
 swift_compiler_sources(Optimizer
+  MandatoryPerformanceOptimizations.swift
   ReadOnlyGlobalVariables.swift
   StackProtection.swift
 )