Merge pull request #84045 from MAJKFL/new-sil-licm-pass-copy-ownership

Ownership support for LICM

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/InitializeStaticGlobals.swift

@@ -225,7 +225,7 @@ private indirect enum GlobalInitValue {
       fatalError("cannot materialize undefined init value")
 
     case .constant(let value):
-      return cloner.cloneRecursivelyToGlobal(value: value)
+      return cloner.cloneRecursively(globalInitValue: value)
 
     case .aggregate(let fields):
       if type.isStruct {

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift

@@ -17,6 +17,10 @@ let loopInvariantCodeMotionPass = FunctionPass(name: "loop-invariant-code-motion
   for loop in context.loopTree.loops {
     optimizeTopLevelLoop(topLevelLoop: loop, context)
   }
+  
+  if context.needFixStackNesting {
+    context.fixStackNesting(in: function)
+  }
 }
 
 private func optimizeTopLevelLoop(topLevelLoop: Loop, _ context: FunctionPassContext) {
@@ -57,7 +61,7 @@ private struct MovableInstructions {
   var loadsAndStores: [Instruction] = []
   var hoistUp: [Instruction] = []
   var sinkDown: [Instruction] = []
-  var scopedInsts: [Instruction] = []
+  var scopedInsts: [ScopedInstruction] = []
 }
 
 /// Analyzed instructions inside the currently processed loop.
@@ -80,10 +84,10 @@ private struct AnalyzedInstructions {
   /// * an apply to the addressor of the global
   /// * a builtin "once" of the global initializer
   var globalInitCalls: Stack<Instruction>
-  var readOnlyApplies: Stack<ApplyInst>
+  var readOnlyApplies: Stack<FullApplySite>
   var loads: Stack<LoadInst>
   var stores: Stack<StoreInst>
-  var beginAccesses: Stack<BeginAccessInst>
+  var scopedInsts: Stack<UnaryInstruction>
   var fullApplies: Stack<FullApplySite>
   
   /// `true` if the loop has instructions which (may) read from memory, which are not in `Loads` and not in `sideEffects`.
@@ -97,10 +101,10 @@ private struct AnalyzedInstructions {
     self.blockSideEffectBottomMarker = loopSideEffects.top
     
     self.globalInitCalls = Stack<Instruction>(context)
-    self.readOnlyApplies = Stack<ApplyInst>(context)
+    self.readOnlyApplies = Stack<FullApplySite>(context)
     self.loads = Stack<LoadInst>(context)
     self.stores = Stack<StoreInst>(context)
-    self.beginAccesses = Stack<BeginAccessInst>(context)
+    self.scopedInsts = Stack<UnaryInstruction>(context)
     self.fullApplies = Stack<FullApplySite>(context)
   }
   
@@ -110,7 +114,7 @@ private struct AnalyzedInstructions {
     loopSideEffects.deinitialize()
     loads.deinitialize()
     stores.deinitialize()
-    beginAccesses.deinitialize()
+    scopedInsts.deinitialize()
     fullApplies.deinitialize()
   }
   
@@ -134,8 +138,6 @@ private func analyzeLoopAndSplitLoads(loop: Loop, _ context: FunctionPassContext
 
   analyzeInstructions(in: loop, &analyzedInstructions, &movableInstructions, context)
 
-  collectHoistableReadOnlyApplies(analyzedInstructions, &movableInstructions, context)
-
   collectHoistableGlobalInitCalls(in: loop, analyzedInstructions, &movableInstructions, context)
 
   collectProjectableAccessPathsAndSplitLoads(in: loop, &analyzedInstructions, &movableInstructions, context)
@@ -158,60 +160,46 @@ private func analyzeInstructions(
     analyzedInstructions.markBeginOfBlock()
     
     for inst in bb.instructions {
-      // TODO: Remove once support for values with ownership implemented.
-      if inst.hasOwnershipOperandsOrResults {
-        analyzedInstructions.analyzeSideEffects(ofInst: inst)
-        
-        // Collect fullApplies to be checked in analyzeBeginAccess
-        if let fullApply = inst as? FullApplySite {
-          analyzedInstructions.fullApplies.append(fullApply)
-        }
-        
-        continue
-      }
-      
       switch inst {
       case is FixLifetimeInst:
         break // We can ignore the side effects of FixLifetimes
       case let loadInst as LoadInst:
         analyzedInstructions.loads.append(loadInst)
+      case let uncheckedOwnershipConversionInst as UncheckedOwnershipConversionInst:
+        analyzedInstructions.analyzeSideEffects(ofInst: uncheckedOwnershipConversionInst)
       case let storeInst as StoreInst:
-        switch storeInst.storeOwnership {
-        case .assign, .initialize:
-          continue  // TODO: Add support
-        case .unqualified, .trivial:
-          break
-        }
         analyzedInstructions.stores.append(storeInst)
         analyzedInstructions.analyzeSideEffects(ofInst: storeInst)
       case let beginAccessInst as BeginAccessInst:
-        analyzedInstructions.beginAccesses.append(beginAccessInst)
+        analyzedInstructions.scopedInsts.append(beginAccessInst)
         analyzedInstructions.analyzeSideEffects(ofInst: beginAccessInst)
+      case let beginBorrowInst as BeginBorrowInstruction:
+        analyzedInstructions.analyzeSideEffects(ofInst: beginBorrowInst)
       case let refElementAddrInst as RefElementAddrInst:
         movableInstructions.speculativelyHoistable.append(refElementAddrInst)
       case let condFailInst as CondFailInst:
         analyzedInstructions.analyzeSideEffects(ofInst: condFailInst)
-      case let applyInst as ApplyInst:
-        if applyInst.isSafeReadOnlyApply(context.calleeAnalysis) {
-          analyzedInstructions.readOnlyApplies.append(applyInst)
-        } else if let callee = applyInst.referencedFunction,
+      case let fullApply as FullApplySite:
+        if fullApply.isSafeReadOnlyApply(context.calleeAnalysis) {
+          analyzedInstructions.readOnlyApplies.append(fullApply)
+        } else if let callee = fullApply.referencedFunction,
                   callee.isGlobalInitFunction, // Calls to global inits are different because we don't care about side effects which are "after" the call in the loop.
-                  !applyInst.globalInitMayConflictWith(
+                  !fullApply.globalInitMayConflictWith(
                     blockSideEffectSegment: analyzedInstructions.sideEffectsOfCurrentBlock,
                     context.aliasAnalysis
                   ) {
           // Check against side-effects within the same block.
           // Side-effects in other blocks are checked later (after we
           // scanned all blocks of the loop) in `collectHoistableGlobalInitCalls`.
-          analyzedInstructions.globalInitCalls.append(applyInst)
+          analyzedInstructions.globalInitCalls.append(fullApply)
         }
         
+        analyzedInstructions.fullApplies.append(fullApply)
+        
         // Check for array semantics and side effects - same as default
         fallthrough
       default:
         switch inst {
-        case let fullApply as FullApplySite:
-          analyzedInstructions.fullApplies.append(fullApply)
         case let builtinInst as BuiltinInst:
           switch builtinInst.id {
           case .Once, .OnceWithContext:
@@ -236,27 +224,6 @@ private func analyzeInstructions(
   }
 }
 
-/// Process collected read only applies. Moves them to `hoistUp` if they don't conflict with any side effects.
-private func collectHoistableReadOnlyApplies(
-  _ analyzedInstructions: AnalyzedInstructions,
-  _ movableInstructions: inout MovableInstructions,
-  _ context: FunctionPassContext
-) {
-  var counter = 0
-  for readOnlyApply in analyzedInstructions.readOnlyApplies {
-    // Avoid quadratic complexity in corner cases. Usually, this limit will not be exceeded.
-    if counter * analyzedInstructions.loopSideEffects.count < 8000,
-       readOnlyApply.isSafeReadOnlyApply(
-         for: analyzedInstructions.loopSideEffects,
-         context.aliasAnalysis,
-         context.calleeAnalysis
-       ) {
-      movableInstructions.hoistUp.append(readOnlyApply)
-    }
-    counter += 1
-  }
-}
-
 /// Process collected global init calls. Moves them to `hoistUp` if they don't conflict with any side effects.
 private func collectHoistableGlobalInitCalls(
   in loop: Loop,
@@ -320,13 +287,9 @@ private func collectMovableInstructions(
   _ context: FunctionPassContext
 ) {
   var loadInstCounter = 0
+  var readOnlyApplyCounter = 0
   for bb in loop.loopBlocks {
     for inst in bb.instructions {
-      // TODO: Remove once support for values with ownership implemented.
-      if inst.hasOwnershipOperandsOrResults {
-        continue
-      }
-      
       switch inst {
       case let fixLifetimeInst as FixLifetimeInst:
         guard fixLifetimeInst.parentBlock.dominates(loop.preheader!, context.dominatorTree) else {
@@ -339,19 +302,20 @@ private func collectMovableInstructions(
       case let loadInst as LoadInst:
         // Avoid quadratic complexity in corner cases. Usually, this limit will not be exceeded.
         if loadInstCounter * analyzedInstructions.loopSideEffects.count < 8000,
-           !analyzedInstructions.loopSideEffectsMayWriteTo(address: loadInst.operand.value, context.aliasAnalysis),
-           loadInst.loadOwnership != .take, loadInst.loadOwnership != .copy { // TODO: Add support for take and copy loads.
+           !analyzedInstructions.loopSideEffectsMayWriteTo(address: loadInst.operand.value, context.aliasAnalysis) {
           movableInstructions.hoistUp.append(loadInst)
         }
         
         loadInstCounter += 1
         
         movableInstructions.loadsAndStores.append(loadInst)
+      case is UncheckedOwnershipConversionInst:
+        break // TODO: Add support
       case let storeInst as StoreInst:
         switch storeInst.storeOwnership {
-        case .assign, .initialize:
+        case .assign:
           continue  // TODO: Add support
-        case .unqualified, .trivial:
+        case .unqualified, .trivial, .initialize:
           break
         }
         movableInstructions.loadsAndStores.append(storeInst)
@@ -362,9 +326,33 @@ private func collectMovableInstructions(
         // must - after hoisting - also be executed before said access.
         movableInstructions.hoistUp.append(condFailInst)
       case let beginAccessInst as BeginAccessInst:
-        if beginAccessInst.canBeHoisted(outOf: loop, analyzedInstructions: analyzedInstructions, context) {
+        if beginAccessInst.canScopedInstructionBeHoisted(outOf: loop, analyzedInstructions: analyzedInstructions, context) {
           movableInstructions.scopedInsts.append(beginAccessInst)
         }
+      case let beginBorrowInst as BeginBorrowInstruction:
+        if !beginBorrowInst.isLexical && beginBorrowInst.canScopedInstructionBeHoisted(outOf: loop, analyzedInstructions: analyzedInstructions, context) {
+          movableInstructions.scopedInsts.append(beginBorrowInst)
+        }
+      case let fullApplySite as FullApplySite:
+        guard analyzedInstructions.readOnlyApplies.contains(where: { $0 == fullApplySite }) else {
+          break
+        }
+        
+        // Avoid quadratic complexity in corner cases. Usually, this limit will not be exceeded.
+        if readOnlyApplyCounter * analyzedInstructions.loopSideEffects.count < 8000,
+           fullApplySite.isSafeReadOnlyApply(
+             for: analyzedInstructions.loopSideEffects,
+             context.aliasAnalysis,
+             context.calleeAnalysis
+           ) {
+          if let beginApplyInst = fullApplySite as? BeginApplyInst {
+            movableInstructions.scopedInsts.append(beginApplyInst)
+          } else {
+            movableInstructions.hoistUp.append(fullApplySite)
+          }
+          
+          readOnlyApplyCounter += 1
+        }
       default:
         break
       }
@@ -433,7 +421,7 @@ private extension AnalyzedInstructions {
     if (loopSideEffects.contains { sideEffect in
       switch sideEffect {
       case let storeInst as StoreInst:
-        if storeInst.isNonInitializingStoreTo(accessPath) {
+        if storeInst.storesTo(accessPath) {
           return false
         }
       case let loadInst as LoadInst:
@@ -450,13 +438,13 @@ private extension AnalyzedInstructions {
     }
         
     if (loads.contains { loadInst in
-      loadInst.mayRead(fromAddress: storeAddr, aliasAnalysis) && loadInst.loadOwnership != .take && !loadInst.overlaps(accessPath: accessPath)
+      loadInst.mayRead(fromAddress: storeAddr, aliasAnalysis) && !loadInst.overlaps(accessPath: accessPath)
     }) {
       return false
     }
           
     if (stores.contains { storeInst in
-      storeInst.mayWrite(toAddress: storeAddr, aliasAnalysis) && !storeInst.isNonInitializingStoreTo(accessPath)
+      storeInst.mayWrite(toAddress: storeAddr, aliasAnalysis) && !storeInst.storesTo(accessPath)
     }) {
       return false
     }
@@ -661,18 +649,29 @@ private extension MovableInstructions {
     
     var changed = false
 
-    for specialInst in scopedInsts {
-      guard let beginAccessInst = specialInst as? BeginAccessInst else {
-        continue
+    for scopedInst in scopedInsts {
+      if let storeBorrowInst = scopedInst as? StoreBorrowInst {
+        _ = storeBorrowInst.allocStack.hoist(outOf: loop, context)
+        
+        var sankFirst = false
+        for deallocStack in storeBorrowInst.allocStack.deallocations {
+          if sankFirst {
+            context.erase(instruction: deallocStack)
+          } else {
+            sankFirst = deallocStack.sink(outOf: loop, context)
+          }
+        }
+        
+        context.notifyInvalidatedStackNesting()
       }
 
-      guard specialInst.hoist(outOf: loop, context) else {
+      guard scopedInst.hoist(outOf: loop, context) else {
         continue
       }
 
       // We only want to sink the first end_access and erase the rest to not introduce duplicates.
       var sankFirst = false
-      for endAccess in beginAccessInst.endAccessInstructions {
+      for endAccess in scopedInst.endInstructions {
         if sankFirst {
           context.erase(instruction: endAccess)
         } else {
@@ -693,10 +692,10 @@ private extension MovableInstructions {
   ) -> Bool {
     // Initially load the value in the loop pre header.
     let builder = Builder(before: loop.preheader!.terminator, context)
-    var storeAddr: Value?
+    var firstStore: StoreInst?
     
     // If there are multiple stores in a block, only the last one counts.
-    for case let storeInst as StoreInst in loadsAndStores where storeInst.isNonInitializingStoreTo(accessPath) {
+    for case let storeInst as StoreInst in loadsAndStores where storeInst.storesTo(accessPath) {
       // If a store just stores the loaded value, bail. The operand (= the load)
       // will be removed later, so it cannot be used as available value.
       // This corner case is surprisingly hard to handle, so we just give up.
@@ -705,9 +704,9 @@ private extension MovableInstructions {
         return false
       }
       
-      if storeAddr == nil {
-        storeAddr = storeInst.destination
-      } else if storeInst.destination.type != storeAddr!.type {
+      if firstStore == nil {
+        firstStore = storeInst
+      } else if storeInst.destination.type != firstStore!.destination.type {
         // This transformation assumes that the values of all stores in the loop
         // must be interchangeable. It won't work if stores different types
         // because of casting or payload extraction even though they have the
@@ -716,26 +715,26 @@ private extension MovableInstructions {
       }
     }
     
-    guard let storeAddr else {
+    guard let firstStore else {
       return false
     }
     
     var ssaUpdater = SSAUpdater(
-      function: storeAddr.parentFunction,
-      type: storeAddr.type.objectType,
-      ownership: .none,
+      function: firstStore.parentFunction,
+      type: firstStore.destination.type.objectType,
+      ownership: firstStore.source.ownership,
       context
     )
     
     // Set all stored values as available values in the ssaUpdater.
-    for case let storeInst as StoreInst in loadsAndStores where storeInst.isNonInitializingStoreTo(accessPath) {
+    for case let storeInst as StoreInst in loadsAndStores where storeInst.storesTo(accessPath) {
       ssaUpdater.addAvailableValue(storeInst.source, in: storeInst.parentBlock)
     }
     
     var cloner = Cloner(cloneBefore: loop.preheader!.terminator, context)
     defer { cloner.deinitialize() }
     
-    guard let initialAddr = (cloner.cloneRecursively(value: storeAddr) { srcAddr, cloner in
+    guard let initialAddr = (cloner.cloneRecursively(value: firstStore.destination) { srcAddr, cloner in
       switch srcAddr {
       case is AllocStackInst, is BeginBorrowInst, is MarkDependenceInst:
         return .stopCloning
@@ -754,7 +753,7 @@ private extension MovableInstructions {
       return false
     }
     
-    let ownership: LoadInst.LoadOwnership = loop.preheader!.terminator.parentFunction.hasOwnership ? .trivial : .unqualified
+    let ownership: LoadInst.LoadOwnership = firstStore.parentFunction.hasOwnership ? (firstStore.storeOwnership == .initialize ? .take : .trivial) : .unqualified
     
     let initialLoad = builder.createLoad(fromAddress: initialAddr, ownership: ownership)
     ssaUpdater.addAvailableValue(initialLoad, in: loop.preheader!)
@@ -774,7 +773,7 @@ private extension MovableInstructions {
         currentVal = nil
       }
       
-      if let storeInst = inst as? StoreInst, storeInst.isNonInitializingStoreTo(accessPath) {
+      if let storeInst = inst as? StoreInst, storeInst.storesTo(accessPath) {
         currentVal = storeInst.source
         context.erase(instruction: storeInst)
         changed = true
@@ -814,11 +813,10 @@ private extension MovableInstructions {
       
       let builder = Builder(before: exitBlock.instructions.first!, context)
       
-      let ownership: StoreInst.StoreOwnership = exitBlock.instructions.first!.parentFunction.hasOwnership ? .trivial : .unqualified
       builder.createStore(
         source: ssaUpdater.getValue(inMiddleOf: exitBlock),
         destination: initialAddr,
-        ownership: ownership
+        ownership: firstStore.storeOwnership
       )
       changed = true
     }
@@ -833,13 +831,6 @@ private extension MovableInstructions {
 }
 
 private extension Instruction {
-  var hasOwnershipOperandsOrResults: Bool {
-    guard parentFunction.hasOwnership else { return false }
-
-    return results.contains(where: { $0.ownership != .none })
-      || operands.contains(where: { $0.value.ownership != .none })
-  }
-  
   /// Returns `true` if this instruction follows the default hoisting heuristic which means it
   /// is not a terminator, allocation or deallocation and either a hoistable array semantics call or doesn't have memory effects.
   func canBeHoisted(outOf loop: Loop, _ context: FunctionPassContext) -> Bool {
@@ -861,7 +852,8 @@ private extension Instruction {
       break
     }
 
-    if memoryEffects == .noEffects {
+    if memoryEffects == .noEffects,
+       !results.contains(where: { $0.ownership == .owned }) {
       return true
     }
 
@@ -877,11 +869,16 @@ private extension Instruction {
     if canHoistArraySemanticsCall(to: terminator, context) {
       hoistArraySemanticsCall(before: terminator, context)
     } else {
-      move(before: terminator, context)
+      if let loadCopyInst = self as? LoadInst, loadCopyInst.loadOwnership == .copy {
+        hoist(loadCopyInst: loadCopyInst, outOf: loop, context)
+        return true
+      } else {
+        move(before: terminator, context)
+      }
     }
     
     if let singleValueInst = self as? SingleValueInstruction,
-       !(self is BeginAccessInst),
+       !(self is ScopedInstruction || self is AllocStackInst),
        let identicalInst = (loop.preheader!.instructions.first { otherInst in
          return singleValueInst != otherInst && singleValueInst.isIdenticalTo(otherInst)
     }) {
@@ -895,11 +892,30 @@ private extension Instruction {
     return true
   }
   
+  private func hoist(loadCopyInst: LoadInst, outOf loop: Loop, _ context: FunctionPassContext) {
+    if loop.hasNoExitBlocks {
+      return
+    }
+    
+    let preheaderBuilder = Builder(before: loop.preheader!.terminator, context)
+    let preheaderLoadBorrow = preheaderBuilder.createLoadBorrow(fromAddress: loadCopyInst.address)
+    
+    let headerBuilder = Builder(before: loadCopyInst, context)
+    let copyValue = headerBuilder.createCopyValue(operand: preheaderLoadBorrow)
+    loadCopyInst.replace(with: copyValue, context)
+    
+    for exitBlock in loop.exitBlocks where !context.deadEndBlocks.isDeadEnd(exitBlock) {
+      assert(exitBlock.hasSinglePredecessor, "Exiting edge should not be critical.")
+      
+      let exitBlockBuilder = Builder(before: exitBlock.instructions.first!, context)
+      exitBlockBuilder.createEndBorrow(of: preheaderLoadBorrow)
+    }
+  }
+  
   func sink(outOf loop: Loop, _ context: FunctionPassContext) -> Bool {
-    let exitBlocks = loop.exitBlocks
     var changed = false
 
-    for exitBlock in exitBlocks {
+    for exitBlock in loop.exitBlocks where !context.deadEndBlocks.isDeadEnd(exitBlock) {
       assert(exitBlock.hasSinglePredecessor, "Exiting edge should not be critical.")
       
       if changed {
@@ -969,11 +985,7 @@ private extension Instruction {
 
 private extension StoreInst {
   /// Returns a `true` if this store is a store to `accessPath`.
-  func isNonInitializingStoreTo(_ accessPath: AccessPath) -> Bool {
-    if self.storeOwnership == .initialize {
-      return false
-    }
-
+  func storesTo(_ accessPath: AccessPath) -> Bool {
     return accessPath == self.destination.accessPath
   }
 }
@@ -991,7 +1003,7 @@ private extension LoadInst {
   }
 }
 
-private extension ApplyInst {
+private extension FullApplySite {
   /// Returns `true` if this apply inst could be safely hoisted.
   func isSafeReadOnlyApply(_ calleeAnalysis: CalleeAnalysis) -> Bool {
     guard functionConvention.results.allSatisfy({ $0.convention == .unowned }) else {
@@ -1040,6 +1052,10 @@ private extension ApplyInst {
         is KeyPathInst, is DeallocStackInst, is DeallocStackRefInst,
         is DeallocRefInst:
         break
+      case let endApply as EndApplyInst:
+        if endApply.beginApply != self {
+          return false
+        }
       default:
         if sideEffect.mayWriteToMemory {
           return false
@@ -1051,54 +1067,89 @@ private extension ApplyInst {
   }
 }
 
-private extension BeginAccessInst {
+private extension ScopedInstruction {
   /// Returns `true` if this begin access is safe to hoist.
-  func canBeHoisted(
+  func canScopedInstructionBeHoisted(
     outOf loop: Loop,
     analyzedInstructions: AnalyzedInstructions,
     _ context: FunctionPassContext
   ) -> Bool {
-    guard endAccessInstructions.allSatisfy({ loop.contains(block: $0.parentBlock)}) else {
+    guard endInstructions.allSatisfy({ loop.contains(block: $0.parentBlock) && !($0 is TermInst) }) else {
       return false
     }
     
-    let areBeginAccessesSafe = analyzedInstructions.beginAccesses
-      .allSatisfy { otherBeginAccessInst in
-        guard self != otherBeginAccessInst else { return true }
+    // Instruction specific preconditions
+    switch self {
+    case is BeginAccessInst, is LoadBorrowInst:
+      guard (analyzedInstructions.scopedInsts
+        .allSatisfy { otherScopedInst in
+          guard self != otherScopedInst else { return true }
 
-        return self.accessPath.isDistinct(from: otherBeginAccessInst.accessPath)
-      }
-
-    guard areBeginAccessesSafe else { return false }
-    
-    var scope = InstructionRange(begin: self, ends: endAccessInstructions, context)
-    defer { scope.deinitialize() }
-
-    for fullApplyInst in analyzedInstructions.fullApplies {
-      guard mayWriteToMemory && fullApplyInst.mayReadOrWrite(address: address, context.aliasAnalysis) ||
-            !mayWriteToMemory && fullApplyInst.mayWrite(toAddress: address, context.aliasAnalysis) else {
-        continue
-      }
-
-      // After hoisting the begin/end_access the apply will be within the scope, so it must not have a conflicting access.
-      if !scope.contains(fullApplyInst) {
+          return operands.first!.value.accessPath.isDistinct(from: otherScopedInst.operand.value.accessPath)
+        }) else {
         return false
       }
+    default:
+      break
     }
+    
+    var scope = InstructionRange(begin: self, ends: endInstructions, context)
+    defer { scope.deinitialize() }
+    
+    // Instruction specific range related conditions
+    switch self {
+    case is BeginApplyInst:
+      return true // Has already been checked with other full applies.
+    case let loadBorrowInst as LoadBorrowInst:
+      for case let destroyAddrInst as DestroyAddrInst in analyzedInstructions.loopSideEffects {
+        if context.aliasAnalysis.mayAlias(loadBorrowInst.address, destroyAddrInst.destroyedAddress) {
+          if !scope.contains(destroyAddrInst) {
+            return false
+          }
+        }
+      }
+      
+      for storeInst in analyzedInstructions.stores {
+        if storeInst.mayWrite(toAddress: loadBorrowInst.address, context.aliasAnalysis) {
+          if !scope.contains(storeInst) {
+            return false
+          }
+        }
+      }
+      
+      fallthrough
+    case is BeginAccessInst:
+      for fullApplyInst in analyzedInstructions.fullApplies {
+        guard mayWriteToMemory && fullApplyInst.mayReadOrWrite(address: operands.first!.value, context.aliasAnalysis) ||
+              !mayWriteToMemory && fullApplyInst.mayWrite(toAddress: operands.first!.value, context.aliasAnalysis) else {
+          continue
+        }
 
-    switch accessPath.base {
-    case .class, .global:
-      for sideEffect in analyzedInstructions.loopSideEffects where sideEffect.mayRelease {
-        // Since a class might have a deinitializer, hoisting begin/end_access pair could violate
-        // exclusive access if the deinitializer accesses address used by begin_access.
-        if !scope.contains(sideEffect) {
+        // After hoisting the begin/end_access the apply will be within the scope, so it must not have a conflicting access.
+        if !scope.contains(fullApplyInst) {
           return false
         }
       }
+      
+      switch operands.first!.value.accessPath.base {
+      case .class, .global:
+        for sideEffect in analyzedInstructions.loopSideEffects where sideEffect.mayRelease {
+          // Since a class might have a deinitializer, hoisting begin/end_access pair could violate
+          // exclusive access if the deinitializer accesses address used by begin_access.
+          if !scope.contains(sideEffect) {
+            return false
+          }
+        }
 
-      return true
+        return true
+      default:
+        return true
+      }
+    case is BeginBorrowInst, is StoreBorrowInst:
+      // Ensure the value is produced outside the loop.
+      return !loop.contains(block: operands.first!.value.parentBlock)
     default:
-      return true
+      return false
     }
   }
 }

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/ObjectOutliner.swift

@@ -370,7 +370,7 @@ private func constructObject(of allocRef: AllocRefInstBase,
   // Create the initializers for the fields
   var objectArgs = [Value]()
   for store in storesToClassFields {
-    objectArgs.append(cloner.cloneRecursivelyToGlobal(value: store.source as! SingleValueInstruction))
+    objectArgs.append(cloner.cloneRecursively(globalInitValue: store.source as! SingleValueInstruction))
   }
   let globalBuilder = Builder(staticInitializerOf: global, context)
 
@@ -382,7 +382,7 @@ private func constructObject(of allocRef: AllocRefInstBase,
       for elementIdx in 0..<allocRef.numTailElements! {
         let tupleElems = (0..<numTailTupleElems).map { tupleIdx in
             let store = storesToTailElements[elementIdx * numTailTupleElems + tupleIdx]
-            return cloner.cloneRecursivelyToGlobal(value: store.source as! SingleValueInstruction)
+            return cloner.cloneRecursively(globalInitValue: store.source as! SingleValueInstruction)
         }
         let tuple = globalBuilder.createTuple(type: allocRef.tailAllocatedTypes[0], elements: tupleElems)
         objectArgs.append(tuple)
@@ -390,7 +390,7 @@ private func constructObject(of allocRef: AllocRefInstBase,
     } else {
       // The non-tuple element case.
       for store in storesToTailElements {
-        objectArgs.append(cloner.cloneRecursivelyToGlobal(value: store.source as! SingleValueInstruction))
+        objectArgs.append(cloner.cloneRecursively(globalInitValue: store.source as! SingleValueInstruction))
       }
     }
   }

SwiftCompilerSources/Sources/Optimizer/InstructionSimplification/SimplifyLoad.swift

@@ -107,7 +107,7 @@ extension LoadInst : OnoneSimplifiable, SILCombineSimplifiable {
     var cloner = Cloner(cloneBefore: self, context)
     defer { cloner.deinitialize() }
 
-    let initVal = cloner.cloneRecursivelyToGlobal(value: globalInitVal)
+    let initVal = cloner.cloneRecursively(globalInitValue: globalInitVal)
 
     uses.replaceAll(with: initVal, context)
     // Also erases a builtin "once" on which the global_addr depends on. This is fine

SwiftCompilerSources/Sources/SIL/Utilities/Cloner.swift

@@ -89,15 +89,15 @@ public struct Cloner<Context: MutatingContext> {
     }
     return cloned
   }
-
-  public mutating func cloneRecursivelyToGlobal(value: Value) -> Value {
-    guard let cloned = cloneRecursively(value: value, customGetCloned: { value, cloner in
+  
+  public mutating func cloneRecursively(globalInitValue: Value) -> Value {
+    guard let cloned = cloneRecursively(value: globalInitValue, customGetCloned: { value, cloner in
       guard let beginAccess = value as? BeginAccessInst else {
         return .defaultValue
       }
       
       // Skip access instructions, which might be generated for UnsafePointer globals which point to other globals.
-      let clonedOperand = cloner.cloneRecursivelyToGlobal(value: beginAccess.address)
+      let clonedOperand = cloner.cloneRecursively(globalInitValue: beginAccess.address)
       cloner.recordFoldedValue(beginAccess, mappedTo: clonedOperand)
       return .customValue(clonedOperand)
     }) else {

test/SILOptimizer/licm.sil

@@ -1615,11 +1615,14 @@ bb2:
   return %r1 : $()
 }
 
-// CHECK-LABEL: sil [ossa] @dont_hoist_non_trivial_load :
+// CHECK-LABEL: sil [ossa] @hoist_load_copy :
+// CHECK:        %1 = load_borrow %0
 // CHECK:       bb1:
-// CHECK-NEXT:   load [copy]
-// CHECK:       } // end sil function 'dont_hoist_non_trivial_load'
-sil [ossa] @dont_hoist_non_trivial_load : $@convention(thin) (@inout Builtin.NativeObject) -> () {
+// CHECK-NEXT:   %3 = copy_value %1
+// CHECK:       bb3:
+// CHECK-NEXT:   end_borrow %1
+// CHECK:       } // end sil function 'hoist_load_copy'
+sil [ossa] @hoist_load_copy : $@convention(thin) (@inout Builtin.NativeObject) -> () {
 bb0(%0 : $*Builtin.NativeObject):
   br bb1
 bb1:
@@ -1634,6 +1637,56 @@ bb3:
   return %r : $()
 }
 
+// CHECK-LABEL: sil [ossa] @hoist_load_take_store_init :
+// CHECK:        %2 = load [take] %0
+// CHECK:       bb1(%4 : @owned $S):
+// CHECK-NEXT:   fix_lifetime %4
+// CHECK:       bb3:
+// CHECK-NEXT:   store %7 to [init] %0
+// CHECK:       } // end sil function 'hoist_load_take_store_init'
+sil [ossa] @hoist_load_take_store_init : $@convention(thin) (@inout S, @guaranteed S) -> () {
+bb0(%0 : $*S, %new_val : @guaranteed $S):
+  br bb1
+bb1:
+  %2 = load [take] %0
+  fix_lifetime %2
+  destroy_value %2
+  %copied = copy_value %new_val
+  store %copied to [init] %0
+  cond_br undef, bb2, bb3
+bb2:
+  br bb1
+bb3:
+  %r = tuple ()
+  return %r : $()
+}
+
+sil @$foo_yield : $@yield_once @convention(method) (Int) -> @yields Int {
+[global: read]
+}
+
+// CHECK-LABEL: sil [ossa] @hoist_begin_apply : $@convention(thin) (Int) -> () {
+// CHECK:       bb0(%0 : $Int):
+// CHECK:         function_ref
+// CHECK:         begin_apply
+// CHECK:       bb3:
+// CHECK:         end_apply
+// CHECK:       } // end sil function 'hoist_begin_apply'
+sil [ossa] @hoist_begin_apply : $@convention(thin) (Int) -> () {
+bb0(%0 : $Int):
+  br bb1
+bb1:
+  %reader = function_ref @$foo_yield : $@yield_once @convention(method) (Int) -> @yields Int
+  (%value, %token) = begin_apply %reader(%0) : $@yield_once @convention(method) (Int) -> @yields Int
+  end_apply %token as $()
+  cond_br undef, bb2, bb3
+bb2:
+  br bb1
+bb3:
+  %r = tuple ()
+  return %r : $()
+}
+
 // CHECK-LABEL: sil [ossa] @hoist_trivial_load :
 // CHECK:         load [trivial]
 // CHECK-NEXT:  br bb1
@@ -1652,11 +1705,13 @@ bb3:
   return %r : $()
 }
 
-// CHECK-LABEL: sil [ossa] @dont_hoist_load_borrow :
-// CHECK:       bb1:
+// CHECK-LABEL: sil [ossa] @hoist_load_borrow :
+// CHECK:       bb0(%0 : $*Builtin.NativeObject):
 // CHECK-NEXT:    load_borrow
-// CHECK:       } // end sil function 'dont_hoist_load_borrow'
-sil [ossa] @dont_hoist_load_borrow : $@convention(thin) (@inout Builtin.NativeObject) -> () {
+// CHECK:       bb3:
+// CHECK-NEXT:    end_borrow
+// CHECK:       } // end sil function 'hoist_load_borrow'
+sil [ossa] @hoist_load_borrow : $@convention(thin) (@inout Builtin.NativeObject) -> () {
 bb0(%0 : $*Builtin.NativeObject):
   br bb1
 bb1:
@@ -1690,6 +1745,31 @@ bb3:
   return %r : $()
 }
 
+// CHECK-LABEL: sil [ossa] @hoist_store_borrow :
+// CHECK:       bb0(%0 : @guaranteed $S):
+// CHECK-NEXT:    alloc_stack
+// CHECK-NEXT:    store_borrow
+// CHECK:       bb3:
+// CHECK-NEXT:    end_borrow
+// CHECK-NEXT:    dealloc_stack
+// CHECK:       } // end sil function 'hoist_store_borrow'
+sil [ossa] @hoist_store_borrow : $@convention(thin) (@guaranteed S) -> () {
+bb0(%0 : @guaranteed $S):
+  br bb1
+bb1:
+  %s = alloc_stack $S
+  %2 = store_borrow %0 to %s
+  fix_lifetime %2
+  end_borrow %2
+  dealloc_stack %s
+  cond_br undef, bb2, bb3
+bb2:
+  br bb1
+bb3:
+  %r = tuple ()
+  return %r : $()
+}
+
 sil @foo : $@convention(thin) (@guaranteed { var Int }) -> ()
 
 // CHECK-LABEL: sil [ossa] @test_begin_access : $@convention(thin) (Int) -> () {