From 04c9fd73fe7ba8eb685e71f3d37db99e185ba3ae Mon Sep 17 00:00:00 2001 From: Jakub Florek Date: Fri, 12 Sep 2025 10:42:18 +0100 Subject: [PATCH 1/2] Add more advanced stack support. # Conflicts: # SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift --- .../LoopInvariantCodeMotion.swift | 81 ++++++++++++++----- 1 file changed, 63 insertions(+), 18 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift index 8012c8d93cd58..2bde0c3eb9c56 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift @@ -403,7 +403,7 @@ private extension AnalyzedInstructions { mutating func analyzeSideEffects(ofInst inst: Instruction) { if inst.mayHaveSideEffects { loopSideEffects.append(inst) - } else if inst.mayReadFromMemory { + } else if inst.mayReadFromMemory && !(inst is MarkDependenceInst) { hasOtherMemReadingInsts = true } } @@ -570,13 +570,14 @@ private extension AnalyzedInstructions { return false } - guard !loadInst.isDeleted, loadInst.operand.value.accessPath.contains(accessPath) else { + guard !loadInst.isDeleted, + let projectionPath = loadInst.operand.value.accessPath.getProjection(to: accessPath), + !projectionPath.isEmpty else { newLoads.push(loadInst) continue } - guard let projectionPath = loadInst.operand.value.accessPath.getProjection(to: accessPath), - let splitLoads = loadInst.trySplit(alongPath: projectionPath, context) else { + guard let splitLoads = loadInst.trySplit(alongPath: projectionPath, context) else { newLoads.push(loadInst) return false } @@ -653,19 +654,11 @@ private extension MovableInstructions { var changed = false 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) - } + if let storeBorrowInst = scopedInst as? StoreBorrowInst, + loop.contains(block: storeBorrowInst.allocStack.parentBlock) { + guard storeBorrowInst.allocStack.hoist(outOf: loop, context) else { + continue } - - context.notifyInvalidatedStackNesting() } guard scopedInst.hoist(outOf: loop, context) else { @@ -738,14 +731,19 @@ private extension MovableInstructions { defer { cloner.deinitialize() } guard let initialAddr = (cloner.cloneRecursively(value: firstStore.destination) { srcAddr, cloner in + let addressDominatesPreheader = srcAddr.parentBlock.dominates(loop.preheader!, context.dominatorTree) + switch srcAddr { case is AllocStackInst, is BeginBorrowInst, is MarkDependenceInst: - return .stopCloning + // Use only as a base. + if !addressDominatesPreheader { + return .stopCloning + } default: break } // Clone projections until the address dominates preheader. - if srcAddr.parentBlock.dominates(loop.preheader!, context.dominatorTree) { + if addressDominatesPreheader { cloner.recordFoldedValue(srcAddr, mappedTo: srcAddr) return .customValue(srcAddr) } else { @@ -756,6 +754,34 @@ private extension MovableInstructions { return false } + if initialAddr is AllocStackInst { + var currentBlock: BasicBlock? + var currentVal: Value? + + for inst in loadsAndStores { + let block = inst.parentBlock + + if block != currentBlock { + currentBlock = block + currentVal = nil + } + + if let storeInst = inst as? StoreInst, storeInst.storesTo(accessPath) { + currentVal = storeInst.source + continue + } + + guard let loadInst = inst as? LoadInst, + loadInst.loadsFrom(accessPath) else { + continue + } + + if currentVal == nil { + return false + } + } + } + let ownership: LoadInst.LoadOwnership = firstStore.parentFunction.hasOwnership ? (firstStore.storeOwnership == .initialize ? .take : .trivial) : .unqualified let initialLoad = builder.createLoad(fromAddress: initialAddr, ownership: ownership) @@ -838,6 +864,8 @@ private extension Instruction { /// 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 { switch self { + case is AllocStackInst: + return true case is TermInst, is Allocation, is Deallocation: return false case is ApplyInst: @@ -876,6 +904,23 @@ private extension Instruction { hoist(loadCopyInst: loadCopyInst, outOf: loop, context) return true } else { + if let allocStack = self as? AllocStackInst { + if allocStack.deallocations.allSatisfy({ loop.contains(block: $0.parentBlock) }) { + var sankFirst = false + for deallocStack in allocStack.deallocations { + if sankFirst { + context.erase(instruction: deallocStack) + } else { + sankFirst = deallocStack.sink(outOf: loop, context) + } + } + + context.notifyInvalidatedStackNesting() + } else { + return false + } + } + move(before: terminator, context) } } From ee76830c4ca87e18e1cfee4937053c74e9a18442 Mon Sep 17 00:00:00 2001 From: Jakub Florek Date: Wed, 24 Sep 2025 18:28:35 +0100 Subject: [PATCH 2/2] Fix too conservative bail when hoisting and sinking load and store instructions. --- .../LoopInvariantCodeMotion.swift | 52 ++++++++++--------- 1 file changed, 27 insertions(+), 25 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift index 2bde0c3eb9c56..522bc5667aea3 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift @@ -68,6 +68,7 @@ private struct MovableInstructions { private struct AnalyzedInstructions { /// Side effects of the loop. var loopSideEffects: StackWithCount + var memoryReadingInsts: Stack private var blockSideEffectBottomMarker: StackWithCount.Marker @@ -90,14 +91,12 @@ private struct AnalyzedInstructions { var scopedInsts: Stack var fullApplies: Stack - /// `true` if the loop has instructions which (may) read from memory, which are not in `Loads` and not in `sideEffects`. - var hasOtherMemReadingInsts = false - /// `true` if one of the side effects might release. lazy var sideEffectsMayRelease = loopSideEffects.contains(where: { $0.mayRelease }) init (_ context: FunctionPassContext) { self.loopSideEffects = StackWithCount(context) + self.memoryReadingInsts = Stack(context) self.blockSideEffectBottomMarker = loopSideEffects.top self.globalInitCalls = Stack(context) @@ -109,6 +108,7 @@ private struct AnalyzedInstructions { } mutating func deinitialize() { + memoryReadingInsts.deinitialize() readOnlyApplies.deinitialize() globalInitCalls.deinitialize() loopSideEffects.deinitialize() @@ -255,26 +255,24 @@ private func collectProjectableAccessPathsAndSplitLoads( _ movableInstructions: inout MovableInstructions, _ context: FunctionPassContext ) { - if !analyzedInstructions.hasOtherMemReadingInsts { - for storeInst in analyzedInstructions.stores { - let accessPath = storeInst.destination.accessPath - if accessPath.isLoopInvariant(loop: loop), - analyzedInstructions.isOnlyLoadedAndStored( - accessPath: accessPath, - storeAddr: storeInst.destination, - context.aliasAnalysis - ), - !movableInstructions.loadAndStoreAccessPaths.contains(accessPath), - // This is not a requirement for functional correctness, but we don't want to - // _speculatively_ load and store the value (outside of the loop). - analyzedInstructions.storesCommonlyDominateExits(of: loop, storingTo: accessPath, context), - analyzedInstructions.splitLoads( - storeAddr: storeInst.destination, - accessPath: accessPath, - context - ) { - movableInstructions.loadAndStoreAccessPaths.append(accessPath) - } + for storeInst in analyzedInstructions.stores { + let accessPath = storeInst.destination.accessPath + if accessPath.isLoopInvariant(loop: loop), + analyzedInstructions.isOnlyLoadedAndStored( + accessPath: accessPath, + storeAddr: storeInst.destination, + context.aliasAnalysis + ), + !movableInstructions.loadAndStoreAccessPaths.contains(accessPath), + // This is not a requirement for functional correctness, but we don't want to + // _speculatively_ load and store the value (outside of the loop). + analyzedInstructions.storesCommonlyDominateExits(of: loop, storingTo: accessPath, context), + analyzedInstructions.splitLoads( + storeAddr: storeInst.destination, + accessPath: accessPath, + context + ) { + movableInstructions.loadAndStoreAccessPaths.append(accessPath) } } } @@ -403,8 +401,8 @@ private extension AnalyzedInstructions { mutating func analyzeSideEffects(ofInst inst: Instruction) { if inst.mayHaveSideEffects { loopSideEffects.append(inst) - } else if inst.mayReadFromMemory && !(inst is MarkDependenceInst) { - hasOtherMemReadingInsts = true + } else if inst.mayReadFromMemory { + memoryReadingInsts.append(inst) } } @@ -418,6 +416,10 @@ private extension AnalyzedInstructions { storeAddr: Value, _ aliasAnalysis: AliasAnalysis ) -> Bool { + if memoryReadingInsts.contains(where: { $0.mayReadOrWrite(address: storeAddr, aliasAnalysis) }) { + return false + } + if (loopSideEffects.contains { sideEffect in switch sideEffect { case let storeInst as StoreInst: