Merge pull request #61495 from nate-chandler/rdar90412220

[SILOptimizer] Add deinit-barrier side-effect.

Author: nate-chandler

SwiftCompilerSources/Sources/Optimizer/Analysis/CalleeAnalysis.swift

@@ -16,6 +16,15 @@ import SIL
 public struct CalleeAnalysis {
   let bridged: BridgedCalleeAnalysis
 
+  static func register() {
+    CalleeAnalysis_register(
+      // isDeinitBarrierFn:
+      { (inst : BridgedInstruction, bca: BridgedCalleeAnalysis) -> Bool in
+        return inst.instruction.isDeinitBarrier(bca.analysis)
+      }
+    )
+  }
+
   public func getCallees(callee: Value) -> FunctionArray? {
     let bridgedFuncs = CalleeAnalysis_getCallees(bridged, callee.bridged)
     if bridgedFuncs.incomplete != 0 {
@@ -45,6 +54,33 @@ public struct CalleeAnalysis {
   }
 }
 
+extension FullApplySite {
+  fileprivate func isBarrier(_ analysis: CalleeAnalysis) -> Bool {
+    guard let callees = analysis.getCallees(callee: callee) else {
+      return true
+    }
+    return callees.contains { $0.isDeinitBarrier }
+  }
+}
+
+extension Instruction {
+  public final func maySynchronize(_ analysis: CalleeAnalysis) -> Bool {
+    if let site = self as? FullApplySite {
+      return site.isBarrier(analysis)
+    }
+    return maySynchronizeNotConsideringSideEffects
+  }
+
+  /// Whether lifetime ends of lexical values may safely be hoisted over this
+  /// instruction.
+  ///
+  /// Deinitialization barriers constrain variable lifetimes. Lexical
+  /// end_borrow, destroy_value, and destroy_addr cannot be hoisted above them.
+  public final func isDeinitBarrier(_ analysis: CalleeAnalysis) -> Bool {
+    return mayAccessPointer || mayLoadWeakOrUnowned || maySynchronize(analysis)
+  }
+}
+
 public struct FunctionArray : RandomAccessCollection, FormattedLikeArray {
   fileprivate let bridged: BridgedCalleeList
 
@@ -55,3 +91,9 @@ public struct FunctionArray : RandomAccessCollection, FormattedLikeArray {
     return BridgedFunctionArray_get(bridged, index).function
   }
 }
+// Bridging utilities
+
+extension BridgedCalleeAnalysis {
+  public var analysis: CalleeAnalysis { .init(bridged: self) }
+}
+

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ComputeSideEffects.swift

@@ -88,6 +88,7 @@ private struct CollectedEffects {
   }
 
   mutating func addInstructionEffects(_ inst: Instruction) {
+    var checkedIfDeinitBarrier = false
     switch inst {
     case is CopyValueInst, is RetainValueInst, is StrongRetainInst:
       addEffects(.copy, to: inst.operands[0].value, fromInitialPath: SmallProjectionPath(.anyValueFields))
@@ -131,12 +132,14 @@ private struct CollectedEffects {
         addDestroyEffects(of: calleeValue)
       }
       handleApply(apply)
+      checkedIfDeinitBarrier = true
 
     case let pa as PartialApplyInst:
       if pa.canBeAppliedInFunction(context) {
         // Only if the created closure can actually be called in the function
         // we have to consider side-effects within the closure.
         handleApply(pa)
+        checkedIfDeinitBarrier = true
       }
 
     case let fl as FixLifetimeInst:
@@ -178,6 +181,13 @@ private struct CollectedEffects {
         globalEffects.allocates = true
       }
     }
+    // If we didn't already, check whether the instruction could be a deinit
+    // barrier.  If it's an apply of some sort, that was already done in
+    // handleApply.
+    if !checkedIfDeinitBarrier,
+       inst.mayBeDeinitBarrierNotConsideringSideEffects {
+      globalEffects.isDeinitBarrier = true
+    }
   }
 
   mutating func addEffectsForEcapingArgument(argument: FunctionArgument) {

SwiftCompilerSources/Sources/Optimizer/PassManager/PassRegistration.swift

@@ -17,6 +17,7 @@ import Parse
 @_cdecl("initializeSwiftModules")
 public func initializeSwiftModules() {
   registerSILClasses()
+  registerSwiftAnalyses()
   registerSwiftPasses()
   initializeSwiftParseModules()
 }
@@ -75,3 +76,7 @@ private func registerSwiftPasses() {
   registerPass(rangeDumper, { rangeDumper.run($0) })
   registerPass(runUnitTests, { runUnitTests.run($0) })
 }
+
+private func registerSwiftAnalyses() {
+  CalleeAnalysis.register()
+}

SwiftCompilerSources/Sources/SIL/Effects.swift

@@ -395,19 +395,29 @@ public struct SideEffects : CustomStringConvertible, NoReflectionChildren {
     /// are not observable form the outside and are therefore not considered.
     public var allocates: Bool
 
+    /// If true, destroys of lexical values may not be hoisted over applies of
+    /// the function.
+    ///
+    /// This is true when the function (or a callee, transitively) contains a
+    /// deinit barrier instruction.
+    public var isDeinitBarrier: Bool
+
     /// When called with default arguments, it creates an "effect-free" GlobalEffects.
     public init(memory: Memory = Memory(read: false, write: false),
                 ownership: Ownership = Ownership(copy: false, destroy: false),
-                allocates: Bool = false) {
+                allocates: Bool = false,
+                isDeinitBarrier: Bool = false) {
       self.memory = memory
       self.ownership = ownership
       self.allocates = allocates
+      self.isDeinitBarrier = isDeinitBarrier
     }
 
     public mutating func merge(with other: GlobalEffects) {
       memory.merge(with: other.memory)
       ownership.merge(with: other.ownership)
       allocates = allocates || other.allocates
+      isDeinitBarrier = isDeinitBarrier || other.isDeinitBarrier
     }
 
     /// Removes effects, which cannot occur for an `argument` value with a given `convention`.
@@ -444,12 +454,13 @@ public struct SideEffects : CustomStringConvertible, NoReflectionChildren {
     }
 
     public static var worstEffects: GlobalEffects {
-      GlobalEffects(memory: .worstEffects, ownership: .worstEffects, allocates: true)
+      GlobalEffects(memory: .worstEffects, ownership: .worstEffects, allocates: true, isDeinitBarrier: true)
     }
 
     public var description: String {
       var res: [String] = [memory.description, ownership.description].filter { !$0.isEmpty }
       if allocates { res += ["allocate"] }
+      if isDeinitBarrier { res += ["deinit_barrier"] }
       return res.joined(separator: ",")
     }
   }
@@ -652,6 +663,7 @@ extension StringParser {
       else if consume("copy")     { globalEffects.ownership.copy = true }
       else if consume("destroy")  { globalEffects.ownership.destroy = true }
       else if consume("allocate") { globalEffects.allocates = true }
+      else if consume("deinit_barrier") { globalEffects.isDeinitBarrier = true }
       else {
         break
       }

SwiftCompilerSources/Sources/SIL/Function.swift

@@ -186,6 +186,10 @@ final public class Function : CustomStringConvertible, HasShortDescription, Hash
     SILFunction_needsStackProtection(bridged) != 0
   }
 
+  public var isDeinitBarrier: Bool {
+    effects.sideEffects?.global.isDeinitBarrier ?? true
+  }
+
   // Only to be called by PassContext
   public func _modifyEffects(_ body: (inout FunctionEffects) -> ()) {
     body(&effects)

SwiftCompilerSources/Sources/SIL/Instruction.swift

@@ -108,6 +108,41 @@ public class Instruction : ListNode, CustomStringConvertible, Hashable {
     return SILInstruction_hasUnspecifiedSideEffects(bridged)
   }
 
+  public final var mayAccessPointer: Bool {
+    return swift_mayAccessPointer(bridged)
+  }
+
+  /// Whether this instruction loads or copies a value whose storage does not
+  /// increment the stored value's reference count.
+  public final var mayLoadWeakOrUnowned: Bool {
+    switch self {
+    case is LoadWeakInst, is LoadUnownedInst, is StrongCopyUnownedValueInst, is StrongCopyUnmanagedValueInst:
+      return true
+    default:
+      return false
+    }
+  }
+
+  /// Conservatively, whether this instruction could involve a synchronization
+  /// point like a memory barrier, lock or syscall.
+  public final var maySynchronizeNotConsideringSideEffects: Bool {
+    switch self {
+    case is FullApplySite, is EndApplyInst, is AbortApplyInst:
+      return true
+    default:
+      return false
+    }
+  }
+
+  /// Conservatively, whether this instruction could be a barrier to hoisting
+  /// destroys.
+  ///
+  /// Does not consider function so effects, so every apply is treated as a
+  /// barrier.
+  public final var mayBeDeinitBarrierNotConsideringSideEffects: Bool {
+    return mayAccessPointer || mayLoadWeakOrUnowned || maySynchronizeNotConsideringSideEffects
+  }
+
   public func visitReferencedFunctions(_ cl: (Function) -> ()) {
   }
 
@@ -618,6 +653,10 @@ final public class ProjectBoxInst : SingleValueInstruction, UnaryInstruction {
 
 final public class CopyValueInst : SingleValueInstruction, UnaryInstruction {}
 
+final public class StrongCopyUnownedValueInst : SingleValueInstruction, UnaryInstruction {}
+
+final public class StrongCopyUnmanagedValueInst : SingleValueInstruction, UnaryInstruction  {}
+
 final public class EndCOWMutationInst : SingleValueInstruction, UnaryInstruction {}
 
 final public

SwiftCompilerSources/Sources/SIL/Registration.swift

@@ -60,6 +60,8 @@ public func registerSILClasses() {
   register(ReleaseValueInst.self)
   register(DestroyValueInst.self)
   register(DestroyAddrInst.self)
+  register(StrongCopyUnownedValueInst.self)
+  register(StrongCopyUnmanagedValueInst.self)
   register(InjectEnumAddrInst.self)
   register(LoadInst.self)
   register(LoadWeakInst.self)

docs/SIL.rst

@@ -2444,10 +2444,10 @@ required.
 Deinit Barriers
 ```````````````
 
-Deinit barriers (see swift::isDeinitBarrier) are instructions which would be
-affected by the side effects of deinitializers.  To maintain the order of
-effects that is visible to the programmer, destroys of lexical values cannot be
-reordered with respect to them.  There are three kinds:
+Deinit barriers (see Instruction.isDeinitBarrier(_:)) are instructions which
+would be affected by the side effects of deinitializers.  To maintain the order
+of effects that is visible to the programmer, destroys of lexical values cannot
+be reordered with respect to them.  There are three kinds:
 
 1. synchronization points (locks, memory barriers, syscalls, etc.)
 2. loads of weak or unowned values

include/swift/SIL/MemAccessUtils.h

@@ -232,11 +232,9 @@ inline bool accessKindMayConflict(SILAccessKind a, SILAccessKind b) {
   return !(a == SILAccessKind::Read && b == SILAccessKind::Read);
 }
 
-/// Return true if \p instruction is a deinitialization barrier.
-///
-/// Deinitialization barriers constrain variable lifetimes. Lexical end_borrow,
-/// destroy_value, and destroy_addr cannot be hoisted above them.
-bool isDeinitBarrier(SILInstruction *instruction);
+/// Whether \p instruction accesses storage whose representation is unidentified
+/// such as by reading a pointer.
+bool mayAccessPointer(SILInstruction *instruction);
 
 } // end namespace swift
 

include/swift/SIL/SILBridging.h

@@ -364,6 +364,7 @@ swift::SILDebugLocation SILInstruction_getLocation(BridgedInstruction inst);
 BridgedMemoryBehavior SILInstruction_getMemBehavior(BridgedInstruction inst);
 bool SILInstruction_mayRelease(BridgedInstruction inst);
 bool SILInstruction_hasUnspecifiedSideEffects(BridgedInstruction inst);
+bool swift_mayAccessPointer(BridgedInstruction inst);
 
 BridgedInstruction MultiValueInstResult_getParent(BridgedMultiValueResult result);
 SwiftInt MultiValueInstResult_getIndex(BridgedMultiValueResult result);

include/swift/SIL/SILInstruction.h

@@ -649,12 +649,6 @@ class SILInstruction : public llvm::ilist_node<SILInstruction> {
   /// Can this instruction abort the program in some manner?
   bool mayTrap() const;
 
-  /// Involves a synchronization point like a memory barrier, lock or syscall.
-  ///
-  /// TODO: We need side-effect analysis and library annotation for this to be
-  ///       a reasonable API.  For now, this is just a placeholder.
-  bool maySynchronize() const;
-
   /// Returns true if the given instruction is completely identical to RHS.
   bool isIdenticalTo(const SILInstruction *RHS) const {
     return isIdenticalTo(RHS,

include/swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h

@@ -230,6 +230,9 @@ class BasicCalleeAnalysis : public SILAnalysis {
   }
 };
 
+bool isDeinitBarrier(SILInstruction *const instruction,
+                     BasicCalleeAnalysis *bca);
+
 } // end namespace swift
 
 #endif

include/swift/SILOptimizer/OptimizerBridging.h

@@ -40,6 +40,10 @@ typedef struct {
   void * _Nullable bca;
 } BridgedCalleeAnalysis;
 
+typedef bool (* _Nonnull InstructionIsDeinitBarrierFn)(BridgedInstruction, BridgedCalleeAnalysis bca);
+
+void CalleeAnalysis_register(InstructionIsDeinitBarrierFn isDeinitBarrierFn);
+
 typedef struct {
   void * _Nullable dea;
 } BridgedDeadEndBlocksAnalysis;

include/swift/SILOptimizer/Utils/CanonicalizeBorrowScope.h

@@ -37,6 +37,8 @@
 
 namespace swift {
 
+class BasicCalleeAnalysis;
+
 //===----------------------------------------------------------------------===//
 //                       MARK: CanonicalizeBorrowScope
 //===----------------------------------------------------------------------===//
@@ -149,6 +151,7 @@ class CanonicalizeBorrowScope {
 
 bool shrinkBorrowScope(
     BeginBorrowInst const &bbi, InstructionDeleter &deleter,
+    BasicCalleeAnalysis *calleeAnalysis,
     SmallVectorImpl<CopyValueInst *> &modifiedCopyValueInsts);
 
 MoveValueInst *foldDestroysOfCopiedLexicalBorrow(BeginBorrowInst *bbi,
@@ -157,7 +160,8 @@ MoveValueInst *foldDestroysOfCopiedLexicalBorrow(BeginBorrowInst *bbi,
 
 bool hoistDestroysOfOwnedLexicalValue(SILValue const value,
                                       SILFunction &function,
-                                      InstructionDeleter &deleter);
+                                      InstructionDeleter &deleter,
+                                      BasicCalleeAnalysis *calleeAnalysis);
 
 } // namespace swift
 

include/swift/SILOptimizer/Utils/InstOptUtils.h

@@ -34,6 +34,7 @@ namespace swift {
 
 class DominanceInfo;
 class DeadEndBlocks;
+class BasicCalleeAnalysis;
 template <class T> class NullablePtr;
 
 /// Transform a Use Range (Operand*) into a User Range (SILInstruction *)

lib/SIL/IR/SILInstruction.cpp

@@ -1358,13 +1358,6 @@ bool SILInstruction::mayTrap() const {
   }
 }
 
-bool SILInstruction::maySynchronize() const {
-  // TODO: We need side-effect analysis and library annotation for this to be
-  //       a reasonable API.  For now, this is just a placeholder.
-  return isa<FullApplySite>(this) || isa<EndApplyInst>(this) ||
-         isa<AbortApplyInst>(this);
-}
-
 bool SILInstruction::isMetaInstruction() const {
   // Every instruction that implements getVarInfo() should be in this list.
   switch (getKind()) {