From e88096e9718adb26c45fe38453efa0e2e4629641 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Mon, 6 Oct 2025 17:39:15 +0200 Subject: [PATCH 1/2] LoopInvariantCodeMotion: fix check for hoisting `load_borrow` instructions We need to check aliasing for all kind of side-effect instructions, not just stores and destroys --- .../LoopInvariantCodeMotion.swift | 26 ++++++++--------- test/SILOptimizer/licm.sil | 28 +++++++++++++++++++ 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift index 6683098f8db07..7072ff6ff0922 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift @@ -1206,23 +1206,21 @@ private extension ScopedInstruction { 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 sideEffectInst in analyzedInstructions.loopSideEffects { + if let endBorrow = sideEffectInst as? EndBorrowInst, + let begin = endBorrow.borrow as? LoadBorrowInst, + begin == self + { + continue } - } - - for storeInst in analyzedInstructions.stores { - if storeInst.mayWrite(toAddress: loadBorrowInst.address, context.aliasAnalysis) { - if !scope.contains(storeInst) { - return false - } + if sideEffectInst.mayWrite(toAddress: loadBorrowInst.address, context.aliasAnalysis), + !scope.contains(sideEffectInst) + { + return false } } - - fallthrough + return true + case is BeginAccessInst: for fullApplyInst in analyzedInstructions.fullApplies { guard mayWriteToMemory && fullApplyInst.mayReadOrWrite(address: operands.first!.value, context.aliasAnalysis) || diff --git a/test/SILOptimizer/licm.sil b/test/SILOptimizer/licm.sil index 60b23fe3321a3..4c1548d4043b9 100644 --- a/test/SILOptimizer/licm.sil +++ b/test/SILOptimizer/licm.sil @@ -1771,6 +1771,34 @@ bb3: return %r : $() } +// CHECK-LABEL: sil [ossa] @dont_hoist_load_borrow : +// CHECK: bb1: +// CHECK-NEXT: copy_addr +// CHECK-NEXT: load_borrow +// CHECK: } // end sil function 'dont_hoist_load_borrow' +sil [ossa] @dont_hoist_load_borrow : $@convention(thin) (@in_guaranteed String) -> () { +bb0(%0 : $*String): + %1 = alloc_stack $String + br bb1 + +bb1: + copy_addr %0 to [init] %1 + %3 = load_borrow %1 + fix_lifetime %3 + end_borrow %3 + %6 = load [take] %1 + destroy_value %6 + cond_br undef, bb2, bb3 + +bb2: + br bb1 + +bb3: + dealloc_stack %1 + %r = tuple() + return %r : $() +} + // CHECK-LABEL: sil [ossa] @dont_hoist_struct : // CHECK: bb1: // CHECK-NEXT: struct $NonCopyable From 55cdc31936820fd21e9aa27df0f3d4a37a8f67f8 Mon Sep 17 00:00:00 2001 From: Erik Eckstein Date: Mon, 6 Oct 2025 17:54:59 +0200 Subject: [PATCH 2/2] LoopInvariantCodeMotion: a small refactoring which is now possible as we removed the "fallthrough" from the previous case --- .../FunctionPasses/LoopInvariantCodeMotion.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift index 7072ff6ff0922..ee4d03629e8c3 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LoopInvariantCodeMotion.swift @@ -1221,10 +1221,10 @@ private extension ScopedInstruction { } return true - case is BeginAccessInst: + case let beginAccess as 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 { + guard mayWriteToMemory && fullApplyInst.mayReadOrWrite(address: beginAccess.address, context.aliasAnalysis) || + !mayWriteToMemory && fullApplyInst.mayWrite(toAddress: beginAccess.address, context.aliasAnalysis) else { continue } @@ -1234,7 +1234,7 @@ private extension ScopedInstruction { } } - switch operands.first!.value.accessPath.base { + switch beginAccess.address.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