Optimizer: some small fixes and improvements for exclusivity checking

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/AllocBoxToStack.swift

@@ -54,7 +54,7 @@ import SIL
 let allocBoxToStack = FunctionPass(name: "allocbox-to-stack") {
   (function: Function, context: FunctionPassContext) in
 
-  _ = tryConvertBoxesToStack(in: function, context)
+  _ = tryConvertBoxesToStack(in: function, isMandatory: false, context)
 }
 
 /// The "mandatory" version of the pass, which runs in the mandatory pipeline.
@@ -72,7 +72,7 @@ let mandatoryAllocBoxToStack = ModulePass(name: "mandatory-allocbox-to-stack") {
 
   while let function = worklist.pop() {
     moduleContext.transform(function: function) { context in
-      let specFns = tryConvertBoxesToStack(in: function, context)
+      let specFns = tryConvertBoxesToStack(in: function, isMandatory: true, context)
       worklist.pushIfNotVisited(contentsOf: specFns.specializedFunctions)
       originalsOfSpecializedFunctions.pushIfNotVisited(contentsOf: specFns.originalFunctions)
     }
@@ -86,9 +86,11 @@ let mandatoryAllocBoxToStack = ModulePass(name: "mandatory-allocbox-to-stack") {
 /// Converts all non-escaping `alloc_box` to `alloc_stack` and specializes called functions if a
 /// box is passed to a function.
 /// Returns the list of original functions for which a specialization has been created.
-private func tryConvertBoxesToStack(in function: Function, _ context: FunctionPassContext) -> FunctionSpecializations {
+private func tryConvertBoxesToStack(in function: Function, isMandatory: Bool,
+                                    _ context: FunctionPassContext
+) -> FunctionSpecializations {
   var promotableBoxes = Array<(AllocBoxInst, Flags)>()
-  var functionsToSpecialize = FunctionSpecializations()
+  var functionsToSpecialize = FunctionSpecializations(isMandatory: isMandatory)
 
   findPromotableBoxes(in: function, &promotableBoxes, &functionsToSpecialize)
 
@@ -189,6 +191,9 @@ private struct FunctionSpecializations {
   private var promotableArguments = CrossFunctionValueWorklist()
   private var originals = FunctionWorklist()
   private var originalToSpecialized = Dictionary<Function, Function>()
+  private let isMandatory: Bool
+
+  init(isMandatory: Bool) { self.isMandatory = isMandatory }
 
   var originalFunctions: [Function] { originals.functions }
   var specializedFunctions: [Function] { originals.functions.lazy.map { originalToSpecialized[$0]! } }
@@ -221,6 +226,12 @@ private struct FunctionSpecializations {
         context.erase(instruction: user)
       case let projectBox as ProjectBoxInst:
         assert(projectBox.fieldIndex == 0, "only single-field boxes are handled")
+        if isMandatory {
+          // Once we have promoted the box to stack, access violations can be detected statically by the
+          // DiagnoseStaticExclusivity pass (which runs after MandatoryAllocBoxToStack).
+          // Therefore we can convert dynamic accesses to static accesses.
+          makeAccessesStatic(of: projectBox, context)
+        }
         projectBox.replace(with: stack, context)
       case is MarkUninitializedInst, is CopyValueInst, is BeginBorrowInst, is MoveValueInst:
         // First, replace the instruction with the original `box`, which adds more uses to `box`.
@@ -478,6 +489,14 @@ private func hoistMarkUnresolvedInsts(stackAddress: Value,
     .replaceAll(with: mu, context)
 }
 
+private func makeAccessesStatic(of address: Value, _ context: FunctionPassContext) {
+  for beginAccess in address.uses.users(ofType: BeginAccessInst.self) {
+    if beginAccess.enforcement == .dynamic {
+      beginAccess.set(enforcement: .static, context: context)
+    }
+  }
+}
+
 private extension ApplySite {
   func getSpecializableCallee() -> Function? {
     if let callee = referencedFunction,

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/TempLValueElimination.swift

@@ -113,6 +113,13 @@ private func tryEliminate(copy: CopyLikeInstruction, _ context: FunctionPassCont
     return
   }
 
+  // If exclusivity is checked for the alloc_stack we must not replace it with the copy-destination.
+  // If the copy-destination is also in an access-scope this would result in an exclusivity violation
+  // which was not there before.
+  guard allocStack.uses.users(ofType: BeginAccessInst.self).isEmpty else {
+    return
+  }
+
   var worklist = InstructionWorklist(context)
   defer { worklist.deinitialize() }
   worklist.pushIfNotVisited(firstUseOfAllocStack)

SwiftCompilerSources/Sources/SIL/Instruction.swift

@@ -1629,12 +1629,39 @@ final public class BeginAccessInst : SingleValueInstruction, UnaryInstruction {
   public var accessKind: AccessKind {
     AccessKind(rawValue: bridged.BeginAccessInst_getAccessKind())!
   }
-  public func set(accessKind: BeginAccessInst.AccessKind, context: some MutatingContext) {
+  public func set(accessKind: AccessKind, context: some MutatingContext) {
     context.notifyInstructionsChanged()
     bridged.BeginAccess_setAccessKind(accessKind.rawValue)
     context.notifyInstructionChanged(self)
   }
 
+  // The raw values must match SILAccessEnforcement.
+  public enum Enforcement: Int {
+    /// The access's enforcement has not yet been determined.
+    case unknown = 0
+
+    /// The access is statically known to not conflict with other accesses.
+    case `static` = 1
+
+    /// The access is not statically known to not conflict with anything and must be dynamically checked.
+    case dynamic = 2
+
+    /// The access is not statically known to not conflict with anything but dynamic checking should
+    /// be suppressed, leaving it undefined behavior.
+    case unsafe = 3
+
+    /// Access to pointers that are signed via pointer authentication.
+    case signed = 4
+  }
+  public var enforcement: Enforcement {
+    Enforcement(rawValue: bridged.BeginAccessInst_getEnforcement())!
+  }
+  public func set(enforcement: Enforcement, context: some MutatingContext) {
+    context.notifyInstructionsChanged()
+    bridged.BeginAccess_setEnforcement(enforcement.rawValue)
+    context.notifyInstructionChanged(self)
+  }
+
   public var isStatic: Bool { bridged.BeginAccessInst_isStatic() }
   public var isUnsafe: Bool { bridged.BeginAccessInst_isUnsafe() }
 

include/swift/SIL/SILBridging.h

@@ -824,6 +824,8 @@ struct BridgedInstruction {
   BRIDGED_INLINE bool BeginAccessInst_isStatic() const;
   BRIDGED_INLINE bool BeginAccessInst_isUnsafe() const;
   BRIDGED_INLINE void BeginAccess_setAccessKind(SwiftInt accessKind) const;
+  BRIDGED_INLINE SwiftInt BeginAccessInst_getEnforcement() const;
+  BRIDGED_INLINE void BeginAccess_setEnforcement(SwiftInt accessKind) const;
   BRIDGED_INLINE bool CopyAddrInst_isTakeOfSrc() const;
   BRIDGED_INLINE bool CopyAddrInst_isInitializationOfDest() const;
   BRIDGED_INLINE void CopyAddrInst_setIsTakeOfSrc(bool isTakeOfSrc) const;

include/swift/SIL/SILBridgingImpl.h

@@ -1494,6 +1494,14 @@ void BridgedInstruction::BeginAccess_setAccessKind(SwiftInt accessKind) const {
   getAs<swift::BeginAccessInst>()->setAccessKind((swift::SILAccessKind)accessKind);
 }
 
+SwiftInt BridgedInstruction::BeginAccessInst_getEnforcement() const {
+  return (SwiftInt)getAs<swift::BeginAccessInst>()->getEnforcement();
+}
+
+void BridgedInstruction::BeginAccess_setEnforcement(SwiftInt accessKind) const {
+  getAs<swift::BeginAccessInst>()->setEnforcement((swift::SILAccessEnforcement)accessKind);
+}
+
 bool BridgedInstruction::CopyAddrInst_isTakeOfSrc() const {
   return getAs<swift::CopyAddrInst>()->isTakeOfSrc();
 }

include/swift/SIL/SILInstruction.h

@@ -4885,6 +4885,7 @@ class EndBorrowInst
 };
 
 /// Different kinds of access.
+/// This enum must stay in sync with `BeginAccessInst.AccessKind` in SwiftCompilerSources.
 enum class SILAccessKind : uint8_t {
   /// An access which takes uninitialized memory and initializes it.
   Init,
@@ -4907,6 +4908,7 @@ enum { NumSILAccessKindBits = 2 };
 StringRef getSILAccessKindName(SILAccessKind kind);
 
 /// Different kinds of exclusivity enforcement for accesses.
+/// This enum must stay in sync with `BeginAccessInst.Enforcement` in SwiftCompilerSources.
 enum class SILAccessEnforcement : uint8_t {
   /// The access's enforcement has not yet been determined.
   Unknown,

lib/SILOptimizer/Mandatory/RawSILInstLowering.cpp

@@ -290,13 +290,20 @@ lowerAssignOrInitInstruction(SILBuilderWithScope &b,
       auto isRefSelf = selfValue->getType().getASTType()->mayHaveSuperclass();
 
       SILValue selfRef;
+      bool needInsertEndAccess = false;
       if (isRefSelf) {
         selfRef = b.emitBeginBorrowOperation(loc, selfValue);
+      } else if (isa<BeginAccessInst>(selfValue)) {
+        // Don't insert an access scope if there is already one. This avoids
+        // inserting a dynamic access check when the parent is static (and therefore
+        // can be statically enforced).
+        selfRef = selfValue;
       } else {
         selfRef = b.createBeginAccess(loc, selfValue, SILAccessKind::Modify,
                                       SILAccessEnforcement::Dynamic,
                                       /*noNestedConflict=*/false,
                                       /*fromBuiltin=*/false);
+        needInsertEndAccess = true;
       }
 
       auto emitFieldReference = [&](VarDecl *field,
@@ -341,7 +348,7 @@ lowerAssignOrInitInstruction(SILBuilderWithScope &b,
       if (isRefSelf) {
         if (selfRef != selfValue)
           b.emitEndBorrowOperation(loc, selfRef);
-      } else {
+      } else if (needInsertEndAccess) {
         b.createEndAccess(loc, selfRef, /*aborted=*/false);
       }
 

lib/SILOptimizer/PassManager/PassPipeline.cpp

@@ -117,7 +117,6 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
   P.startPipeline("Mandatory Diagnostic Passes + Enabling Optimization Passes");
   P.addDiagnoseInvalidEscapingCaptures();
   P.addReferenceBindingTransform();
-  P.addDiagnoseStaticExclusivity();
   P.addNestedSemanticFunctionCheck();
   P.addCapturePromotion();
 
@@ -130,6 +129,10 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
 #else
   P.addLegacyAllocBoxToStack();
 #endif
+  // Needs to run after MandatoryAllocBoxToStack, because MandatoryAllocBoxToStack
+  // can convert dynamic accesses to static accesses.
+  P.addDiagnoseStaticExclusivity();
+
   P.addNoReturnFolding();
   P.addBooleanLiteralFolding();
   addDefiniteInitialization(P);

test/SILGen/moveonly_escaping_closure.swift

@@ -143,7 +143,7 @@ func testLocalLetClosureCaptureVar() {
         consumeVal(x) // expected-note {{consumed here}}
         // expected-note @-1 {{consumed again here}}
         borrowConsumeVal(x, x)
-        // expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
+        // expected-error @-1 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
         // expected-note @-2 {{conflicting access is here}}
         // expected-note @-3 {{used here}}
         // expected-note @-4 {{used here}}
@@ -975,7 +975,7 @@ func testLocalLetClosureCaptureConsuming(_ x: consuming SingleElt) {
         consumeVal(x) // expected-note {{consumed here}}
         // expected-note @-1 {{consumed again here}}
         borrowConsumeVal(x, x) // expected-note {{used here}}
-        // expected-error @-1 {{overlapping accesses, but deinitialization requires exclusive access}}
+        // expected-error @-1 {{overlapping accesses to 'x', but deinitialization requires exclusive access}}
         // expected-note @-2 {{conflicting access is here}}
         // expected-note @-3 {{consumed here}}
         // expected-note @-4 {{used here}}

test/SILOptimizer/allocbox_to_stack_ownership.sil

@@ -1435,3 +1435,19 @@ die:
   destroy_value %copy
   unreachable
 }
+
+// CHECK-LABEL: sil [ossa] @test_access_enforcement :
+// OPT:           begin_access [modify] [dynamic]
+// MANDATORY:     begin_access [modify] [static]
+// CHECK-LABEL: } // end sil function 'test_access_enforcement'
+sil [ossa] @test_access_enforcement : $(Int) -> Int {
+bb0(%0 : $Int):
+  %1 = alloc_box ${ var Int }
+  %2 = project_box %1 : ${ var Int }, 0
+  %3 = begin_access [modify] [dynamic] %2
+  store %0 to [trivial] %3
+  end_access %3
+  %4 = load [trivial] %2
+  destroy_value %1
+  return %4 : $Int
+}

test/SILOptimizer/allocboxtostack_localapply.swift

@@ -179,6 +179,7 @@ public func testdfs2() -> Int {
 
 // CHECK-LABEL: sil @$s26allocboxtostack_localapply15call2localfuncsSiyF :
 // CHECK-NOT:     alloc_box
+// CHECK-NOT:     begin_access
 // CHECK-LABEL:} // end sil function '$s26allocboxtostack_localapply15call2localfuncsSiyF'
 public func call2localfuncs() -> Int {
     var a1 = 1
@@ -194,3 +195,6 @@ public func call2localfuncs() -> Int {
     return a1
 }
 
+// CHECK-LABEL: sil {{.*}} @$s26allocboxtostack_localapply15call2localfuncsSiyF13innerFunctionL_yyFTf0s_n :
+// CHECK-NOT:     begin_access
+// CHECK:       } // end sil function '$s26allocboxtostack_localapply15call2localfuncsSiyF13innerFunctionL_yyFTf0s_n'

test/SILOptimizer/exclusivity_static_diagnostics.swift

@@ -263,12 +263,9 @@ func callsClosureLiteralImmediately() {
 
 func callsStoredClosureLiteral() {
   var i = 7;
-  let c = { (p: inout Int) in i}
+  let c = { (p: inout Int) in i} // expected-note {{conflicting access is here}}
 
-  // Closure literals that are stored and later called are treated as escaping
-  // We don't expect a static exclusivity diagnostic here, but the issue
-  // will be caught at run time
-  _ = c(&i) // no-error
+  _ = c(&i) // expected-error {{overlapping accesses to 'i', but modification requires exclusive access; consider copying to a local variable}}
 }
 
 

test/SILOptimizer/init_accessors.sil

@@ -39,9 +39,9 @@ bb0(%0 : $Int, %1 : $*Test):
 }
 
 // CHECK-LABEL: sil hidden [ossa] @$s14init_accessors4TestV1vACSi_tcfC : $@convention(method) (Int, @thin Test.Type) -> Test
+// CHECK: [[SELF_REF:%.*]] = begin_access [modify] [unknown] [[SELF:%.*]] : $*Test
 // CHECK: [[INIT_REF:%.*]] = function_ref @$s14init_accessors4TestV1xSivi : $@convention(thin) (Int) -> @out Int
-// CHECK: [[SELF_REF:%.*]] = begin_access [modify] [dynamic] [[SELF:%.*]] : $*Test
-// CHECK-NEXT: [[FIELD_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*Test, #Test._x
+// CHECK: [[FIELD_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*Test, #Test._x
 // CHECK-NEXT: {{.*}} = apply [[INIT_REF]]([[FIELD_REF]], %0) : $@convention(thin) (Int) -> @out Int
 sil hidden [ossa] @$s14init_accessors4TestV1vACSi_tcfC : $@convention(method) (Int, @thin Test.Type) -> Test {
 bb0(%0 : $Int, %1 : $@thin Test.Type):

test/SILOptimizer/init_accessors.swift

@@ -41,10 +41,11 @@ struct TestInit {
   }
 
   // CHECK-LABEL: sil hidden [ossa] @$s14init_accessors8TestInitV1x1yACSi_SitcfC : $@convention(method) (Int, Int, @thin TestInit.Type) -> TestInit
+  // CHECK: end_access
+  // CHECK: [[SELF_VALUE:%.*]] = begin_access [modify] [static] {{.*}} : $*TestInit
   // CHECK: // function_ref TestInit.point.init
   // CHECK-NEXT: [[INIT_ACCESSOR_FN:%.*]] = function_ref @$s14init_accessors8TestInitV5pointSi_Sitvi : $@convention(thin) (Int, Int, @inout Int, @thin TestInit.Type) -> (@out Int, @out (Int, Int))
   // CHECK-NEXT: [[INIT_ACCESSOR:%.*]] = partial_apply [callee_guaranteed] [on_stack] [[INIT_ACCESSOR_FN]](%2) : $@convention(thin) (Int, Int, @inout Int, @thin TestInit.Type) -> (@out Int, @out (Int, Int))
-  // CHECK: [[SELF_VALUE:%.*]] = begin_access [modify] [dynamic] {{.*}} : $*TestInit
   // CHECK: [[Y_REF:%.*]] = struct_element_addr [[SELF_VALUE]] : $*TestInit, #TestInit.y
   // CHECK-NEXT: [[FULL_REF:%.*]] = struct_element_addr [[SELF_VALUE]] : $*TestInit, #TestInit.full
   // CHECK-NEXT: ([[X_VAL:%.*]], [[Y_VAL:%.*]]) = destructure_tuple {{.*}} : $(Int, Int)
@@ -71,10 +72,13 @@ struct TestSetter {
   }
 
   // CHECK-LABEL: sil hidden [ossa] @$s14init_accessors10TestSetterV1x1yACSi_SitcfC : $@convention(method) (Int, Int, @thin TestSetter.Type) -> TestSetter
+  // CHECK: [[AS:%.*]] = alloc_stack
+  // CHECK: end_access
+  // CHECK: end_access
+  // CHECK: [[SELF:%.*]] = begin_access [modify] [static] [[AS]] : $*TestSetter
   // CHECK: [[INIT_ACCESSOR_FN:%.*]] = function_ref @$s14init_accessors10TestSetterV5pointSi_Sitvi : $@convention(thin) (Int, Int, @inout Int, @inout Int, @thin TestSetter.Type) -> ()
   // CHECK: [[INIT_ACCESSOR:%.*]] = partial_apply [callee_guaranteed] [on_stack] [[INIT_ACCESSOR_FN]](%2) : $@convention(thin) (Int, Int, @inout Int, @inout Int, @thin TestSetter.Type) -> ()
-  // CHECK: [[SELF:%.*]] = begin_access [modify] [dynamic] %14 : $*TestSetter
-  // CHECK-NEXT: ([[X:%.*]], [[Y:%.*]]) = destructure_tuple {{.*}} : $(Int, Int)
+  // CHECK:      ([[X:%.*]], [[Y:%.*]]) = destructure_tuple {{.*}} : $(Int, Int)
   // CHECK-NEXT: [[X_REF:%.*]] = struct_element_addr [[SELF]] : $*TestSetter, #TestSetter.x
   // CHECK-NEXT: [[Y_REF:%.*]] = struct_element_addr [[SELF]] : $*TestSetter, #TestSetter.y
   // CHECK-NEXT: {{.*}} = apply [[INIT_ACCESSOR]]([[X]], [[Y]], [[X_REF]], [[Y_REF]]) : $@noescape @callee_guaranteed (Int, Int, @inout Int, @inout Int) -> ()
@@ -204,17 +208,18 @@ struct TestNoInitAndInit {
 
   // CHECK-LABEL: sil hidden [ossa] @$s14init_accessors013TestNoInitAndE0V1x1yACSi_SitcfC : $@convention(method) (Int, Int, @thin TestNoInitAndInit.Type) -> TestNoInitAndInit
   //
+  // CHECK: end_access
+  // CHECK: [[SELF_REF:%.*]] = begin_access [modify] [static] {{.*}} : $*TestNoInitAndInit
   // CHECK: [[INIT_REF_FN:%.*]] = function_ref @$s14init_accessors013TestNoInitAndE0V6pointXSivi : $@convention(thin) (Int, @inout Int, @thin TestNoInitAndInit.Type) -> ()
   // CHECK: [[INIT_REF:%.*]] = partial_apply [callee_guaranteed] [on_stack] [[INIT_REF_FN]](%2) : $@convention(thin) (Int, @inout Int, @thin TestNoInitAndInit.Type) -> ()
-  // CHECK: [[SELF_REF:%.*]] = begin_access [modify] [dynamic] {{.*}} : $*TestNoInitAndInit
-  // CHECK-NEXT: [[X_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*TestNoInitAndInit, #TestNoInitAndInit.x
+  // CHECK:      [[X_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*TestNoInitAndInit, #TestNoInitAndInit.x
   // CHECK-NEXT: {{.*}} = apply [[INIT_REF]](%0, [[X_REF]]) : $@noescape @callee_guaranteed (Int, @inout Int) -> ()
   // CHECK-NEXT: end_access [[SELF_REF]] : $*TestNoInitAndInit
   //
+  // CHECK: [[SELF_REF:%.*]] = begin_access [modify] [static] {{.*}} : $*TestNoInitAndInit
   // CHECK: [[INIT_REF_FN:%.*]] = function_ref @$s14init_accessors013TestNoInitAndE0V6pointYSivi : $@convention(thin) (Int, @thin TestNoInitAndInit.Type) -> @out Int
   // CHECK: [[INIT_REF:%.*]] = partial_apply [callee_guaranteed] [on_stack] [[INIT_REF_FN]](%2) : $@convention(thin) (Int, @thin TestNoInitAndInit.Type) -> @out Int
-  // CHECK: [[SELF_REF:%.*]] = begin_access [modify] [dynamic] {{.*}} : $*TestNoInitAndInit
-  // CHECK-NEXT: [[Y_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*TestNoInitAndInit, #TestNoInitAndInit.y
+  // CHECK:      [[Y_REF:%.*]] = struct_element_addr [[SELF_REF]] : $*TestNoInitAndInit, #TestNoInitAndInit.y
   // CHECK-NEXT: {{.*}} = apply [[INIT_REF]]([[Y_REF]], %1) : $@noescape @callee_guaranteed (Int) -> @out Int
   // CHECK-NEXT: end_access [[SELF_REF]] : $*TestNoInitAndInit
   init(x: Int, y: Int) {