From 8304db56f863aea8220dbd504c2256c9c8022339 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Thu, 15 May 2025 23:45:10 -0700 Subject: [PATCH 1/5] Fix DestructorAnalysis: handle enum, FixedArray, and generic types. This checks the members of a value type to determine whether its synthesized deinitializer can have side effects. Add support for enum and fixed array. Add support for generic types. The old code was inexplicably failing to apply the aggregate's type substitutions to the members. --- .../Analysis/DestructorAnalysis.cpp | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/SILOptimizer/Analysis/DestructorAnalysis.cpp b/lib/SILOptimizer/Analysis/DestructorAnalysis.cpp index 2fdada2012a03..635110580d462 100644 --- a/lib/SILOptimizer/Analysis/DestructorAnalysis.cpp +++ b/lib/SILOptimizer/Analysis/DestructorAnalysis.cpp @@ -68,6 +68,14 @@ bool DestructorAnalysis::isSafeType(CanType Ty) { return cacheResult(Ty, true); if (Ty->is()) return cacheResult(Ty, true); + if (Ty->is()) + return cacheResult(Ty, true); + + if (auto fixedArrTy = dyn_cast(Ty)) { + auto subMap = Ty->getContextSubstitutionMap(); + Type eltTy = fixedArrTy->getElementType().subst(subMap); + return !isSafeType(eltTy->getCanonicalType()); + } // A struct is safe if // * either it implements the _DestructorSafeContainer protocol and @@ -81,8 +89,10 @@ bool DestructorAnalysis::isSafeType(CanType Ty) { return cacheResult(Ty, true); // Check the stored properties. + auto subMap = Ty->getContextSubstitutionMap(); for (auto SP : Struct->getStoredProperties()) { - if (!isSafeType(SP->getInterfaceType()->getCanonicalType())) + Type spTy = SP->getInterfaceType().subst(subMap); + if (!isSafeType(spTy->getCanonicalType())) return cacheResult(Ty, false); } return cacheResult(Ty, true); @@ -96,7 +106,17 @@ bool DestructorAnalysis::isSafeType(CanType Ty) { return cacheResult(Ty, true); } - // TODO: enum types. + if (auto *enumDecl = Ty->getEnumOrBoundGenericEnum()) { + auto subMap = Ty->getContextSubstitutionMap(); + for (auto *enumEltDecl : enumDecl->getAllElements()) { + if (auto payloadInterfaceTy = enumEltDecl->getPayloadInterfaceType()) { + Type payloadTy = payloadInterfaceTy.subst(subMap); + if (!isSafeType(payloadTy->getCanonicalType())) + return cacheResult(Ty, false); + } + } + return cacheResult(Ty, true); + } return cacheResult(Ty, false); } From d1be6f29b719822696e7d30739d38780e20bf0a3 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Thu, 15 May 2025 23:50:28 -0700 Subject: [PATCH 2/5] SwiftCompilerSources: bridge DestructorAnalysis. --- .../Sources/Optimizer/Analysis/CMakeLists.txt | 1 + .../Analysis/DestructorAnalysis.swift | 26 +++++++++++++++++++ .../Optimizer/PassManager/Context.swift | 5 ++++ .../swift/SILOptimizer/OptimizerBridging.h | 8 ++++++ .../SILOptimizer/OptimizerBridgingImpl.h | 13 ++++++++++ 5 files changed, 53 insertions(+) create mode 100644 SwiftCompilerSources/Sources/Optimizer/Analysis/DestructorAnalysis.swift diff --git a/SwiftCompilerSources/Sources/Optimizer/Analysis/CMakeLists.txt b/SwiftCompilerSources/Sources/Optimizer/Analysis/CMakeLists.txt index bedf3296a61df..a575c6debf7e7 100644 --- a/SwiftCompilerSources/Sources/Optimizer/Analysis/CMakeLists.txt +++ b/SwiftCompilerSources/Sources/Optimizer/Analysis/CMakeLists.txt @@ -10,5 +10,6 @@ swift_compiler_sources(Optimizer AliasAnalysis.swift CalleeAnalysis.swift DeadEndBlocksAnalysis.swift + DestructorAnalysis.swift DominatorTree.swift PostDominatorTree.swift) diff --git a/SwiftCompilerSources/Sources/Optimizer/Analysis/DestructorAnalysis.swift b/SwiftCompilerSources/Sources/Optimizer/Analysis/DestructorAnalysis.swift new file mode 100644 index 0000000000000..fc9e1003fbecc --- /dev/null +++ b/SwiftCompilerSources/Sources/Optimizer/Analysis/DestructorAnalysis.swift @@ -0,0 +1,26 @@ +//===--- DestructorAnalysis.swift - the dead-end blocks analysis ----------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2025 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +import OptimizerBridging +import SIL + +struct DestructorAnalysis { + let bridged: BridgedDestructorAnalysis + + // TODO: swift::isDestructorSideEffectFree() also handles other cases, such as Arrays and closures. + func deinitMayHaveEffects(type: Type, in function: Function) -> Bool { + if type.isBox { + return type.getBoxFields(in: function).contains(where: { deinitMayHaveEffects(type: $0, in: function) }) + } + return bridged.mayStoreToMemoryOnDestruction(type.bridged) + } +} diff --git a/SwiftCompilerSources/Sources/Optimizer/PassManager/Context.swift b/SwiftCompilerSources/Sources/Optimizer/PassManager/Context.swift index 54323de328f71..bad96555347fb 100644 --- a/SwiftCompilerSources/Sources/Optimizer/PassManager/Context.swift +++ b/SwiftCompilerSources/Sources/Optimizer/PassManager/Context.swift @@ -317,6 +317,11 @@ struct FunctionPassContext : MutatingContext { return DeadEndBlocksAnalysis(bridged: bridgeDEA) } + var destructorAnalysis: DestructorAnalysis { + let bridgeAnalysis = _bridged.getDestructorAnalysis() + return DestructorAnalysis(bridged: bridgeAnalysis) + } + var dominatorTree: DominatorTree { let bridgedDT = _bridged.getDomTree() return DominatorTree(bridged: bridgedDT) diff --git a/include/swift/SILOptimizer/OptimizerBridging.h b/include/swift/SILOptimizer/OptimizerBridging.h index 5cd1133f79d15..43d564e4d43ca 100644 --- a/include/swift/SILOptimizer/OptimizerBridging.h +++ b/include/swift/SILOptimizer/OptimizerBridging.h @@ -43,6 +43,7 @@ class AliasAnalysis; class BasicCalleeAnalysis; class CalleeList; class DeadEndBlocks; +class DestructorAnalysis; class DominanceInfo; class PostDominanceInfo; class BasicBlockSet; @@ -116,6 +117,12 @@ struct BridgedDeadEndBlocksAnalysis { BRIDGED_INLINE bool isDeadEnd(BridgedBasicBlock block) const; }; +struct BridgedDestructorAnalysis { + swift::DestructorAnalysis * _Nonnull analysis; + + BRIDGED_INLINE bool mayStoreToMemoryOnDestruction(BridgedType type) const; +}; + struct BridgedDomTree { swift::DominanceInfo * _Nonnull di; @@ -213,6 +220,7 @@ struct BridgedPassContext { SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedAliasAnalysis getAliasAnalysis() const; SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedCalleeAnalysis getCalleeAnalysis() const; SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedDeadEndBlocksAnalysis getDeadEndBlocksAnalysis() const; + SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedDestructorAnalysis getDestructorAnalysis() const; SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedDomTree getDomTree() const; SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedPostDomTree getPostDomTree() const; SWIFT_IMPORT_UNSAFE BRIDGED_INLINE BridgedDeclObj getSwiftArrayDecl() const; diff --git a/include/swift/SILOptimizer/OptimizerBridgingImpl.h b/include/swift/SILOptimizer/OptimizerBridgingImpl.h index 7393f69822cd3..7c7203a67311f 100644 --- a/include/swift/SILOptimizer/OptimizerBridgingImpl.h +++ b/include/swift/SILOptimizer/OptimizerBridgingImpl.h @@ -23,6 +23,7 @@ #include "swift/SILOptimizer/Analysis/AliasAnalysis.h" #include "swift/SILOptimizer/Analysis/BasicCalleeAnalysis.h" #include "swift/SILOptimizer/Analysis/DeadEndBlocksAnalysis.h" +#include "swift/SILOptimizer/Analysis/DestructorAnalysis.h" #include "swift/SILOptimizer/Analysis/DominanceAnalysis.h" #include "swift/SILOptimizer/OptimizerBridging.h" #include "swift/SILOptimizer/PassManager/PassManager.h" @@ -74,6 +75,14 @@ bool BridgedDeadEndBlocksAnalysis::isDeadEnd(BridgedBasicBlock block) const { return deb->isDeadEnd(block.unbridged()); } +//===----------------------------------------------------------------------===// +// BridgedDestructorAnalysis +//===----------------------------------------------------------------------===// + +bool BridgedDestructorAnalysis::mayStoreToMemoryOnDestruction(BridgedType type) const { + return analysis->mayStoreToMemoryOnDestruction(type.unbridged()); +} + //===----------------------------------------------------------------------===// // BridgedDomTree, BridgedPostDomTree //===----------------------------------------------------------------------===// @@ -205,6 +214,10 @@ BridgedDeadEndBlocksAnalysis BridgedPassContext::getDeadEndBlocksAnalysis() cons return {dba->get(invocation->getFunction())}; } +BridgedDestructorAnalysis BridgedPassContext::getDestructorAnalysis() const { + return {invocation->getPassManager()->getAnalysis()}; +} + BridgedDomTree BridgedPassContext::getDomTree() const { auto *da = invocation->getPassManager()->getAnalysis(); return {da->get(invocation->getFunction())}; From 4016e95d6e357725f43ad5978229b7c7adc9767a Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Thu, 15 May 2025 23:52:35 -0700 Subject: [PATCH 3/5] Add DestructorAnalysis to LifetimeDependenceDefUseWalker. --- .../FunctionPasses/LifetimeDependenceDiagnostics.swift | 7 +++++-- .../FunctionPasses/LifetimeDependenceScopeFixup.swift | 9 ++++++--- .../Optimizer/Utilities/LifetimeDependenceUtils.swift | 7 ++++++- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift index f0d4bd63c6c7c..ba40b6a5932d6 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift @@ -449,14 +449,17 @@ extension DependentAddressUseDefWalker: AddressUseDefWalker { /// TODO: handle stores to singly initialized temporaries like copies using a standard reaching-def analysis. private struct DiagnoseDependenceWalker { let context: Context + let destructorAnalysis: DestructorAnalysis + var diagnostics: DiagnoseDependence let localReachabilityCache = LocalVariableReachabilityCache() var visitedValues: ValueSet - var function: Function { diagnostics.function } + var function: Function { get { diagnostics.function } } - init(_ diagnostics: DiagnoseDependence, _ context: Context) { + init(_ diagnostics: DiagnoseDependence, _ context: FunctionPassContext) { self.context = context + self.destructorAnalysis = context.destructorAnalysis self.diagnostics = diagnostics self.visitedValues = ValueSet(context) } diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift index 817a6958aa713..01b758769a6ae 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift @@ -1012,8 +1012,10 @@ private extension BeginApplyInst { /// /// Set 'dependsOnCaller' if a use escapes the function. private struct LifetimeDependentUseWalker : LifetimeDependenceDefUseWalker { - let function: Function let context: Context + let function: Function + let destructorAnalysis: DestructorAnalysis + let visitor: (Operand) -> WalkResult let localReachabilityCache: LocalVariableReachabilityCache var visitedValues: ValueSet @@ -1021,10 +1023,11 @@ private struct LifetimeDependentUseWalker : LifetimeDependenceDefUseWalker { /// Set to true if the dependence is returned from the current function. var dependsOnCaller = false - init(_ function: Function, _ localReachabilityCache: LocalVariableReachabilityCache, _ context: Context, + init(_ function: Function, _ localReachabilityCache: LocalVariableReachabilityCache, _ context: FunctionPassContext, visitor: @escaping (Operand) -> WalkResult) { - self.function = function self.context = context + self.function = function + self.destructorAnalysis = context.destructorAnalysis self.visitor = visitor self.localReachabilityCache = localReachabilityCache self.visitedValues = ValueSet(context) diff --git a/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift b/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift index c5c075461a918..11d1e5db01192 100644 --- a/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift +++ b/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift @@ -543,6 +543,8 @@ protocol LifetimeDependenceDefUseWalker : ForwardingDefUseWalker, AddressUseVisitor { var function: Function { get } + var destructorAnalysis: DestructorAnalysis { get } + /// Dependence tracking through local variables. var localReachabilityCache: LocalVariableReachabilityCache { get } @@ -1028,12 +1030,15 @@ let lifetimeDependenceScopeTest = FunctionTest("lifetime_dependence_scope") { private struct LifetimeDependenceUsePrinter : LifetimeDependenceDefUseWalker { let context: Context let function: Function + let destructorAnalysis: DestructorAnalysis + let localReachabilityCache = LocalVariableReachabilityCache() var visitedValues: ValueSet - init(function: Function, _ context: Context) { + init(function: Function, _ context: FunctionPassContext) { self.context = context self.function = function + self.destructorAnalysis = context.destructorAnalysis self.visitedValues = ValueSet(context) } From dccaac6b7b955010bbf5a63491a25889d6858ca3 Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Thu, 15 May 2025 23:53:48 -0700 Subject: [PATCH 4/5] [NFC] LifetimeDependence: fix internal debug output --- .../Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift index 01b758769a6ae..fdcea826bc0f6 100644 --- a/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift +++ b/SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift @@ -329,7 +329,7 @@ private struct ScopeExtension { // violation, and that subsequent optimizations do not shrink the inner access `%a1`. extension ScopeExtension { mutating func extendScopes(dependence: LifetimeDependence) -> Bool { - log("Scope fixup for lifetime dependent instructions: \(dependence)") + log("Scope fixup for lifetime dependent instructions:\n\(dependence)") gatherExtensions(dependence: dependence) From 6943ff3375c70ef486e3f9b1c67d7dcd520ce57d Mon Sep 17 00:00:00 2001 From: Andrew Trick Date: Thu, 15 May 2025 23:54:24 -0700 Subject: [PATCH 5/5] Improve LifetimeDependenceDefUseWalker: consult DestructorAnalysis. Fixes rdar://150828355 (Overlapping access error after the last use of a lifetime-dependent span variable) --- .../Utilities/LifetimeDependenceUtils.swift | 17 +++++++++++------ .../lifetime_dependence/semantics.swift | 13 +++++++++++-- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift b/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift index 11d1e5db01192..0871d5633672e 100644 --- a/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift +++ b/SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift @@ -660,14 +660,19 @@ extension LifetimeDependenceDefUseWalker { // Catch .instantaneousUse operations that are dependence leaf uses. return leafUse(of: operand) - case is DestroyValueInst, is EndLifetimeInst, is DeallocRefInst, - is DeallocBoxInst, is DeallocExistentialBoxInst, - is BeginCOWMutationInst, is EndCOWMutationInst, - is EndInitLetRefInst, is DeallocPartialRefInst, is BeginDeallocRefInst: - // Catch .destroyingConsume operations that are dependence leaf - // uses. + case is DestroyValueInst: + // Handles dependent value destroys. + // + // Ignore destroys on a type whose members recursively only have default deinits. + if !destructorAnalysis.deinitMayHaveEffects(type: operand.value.type, in: function) { + return .continueWalk + } return leafUse(of: operand) + case is DeallocBoxInst, is DeallocExistentialBoxInst, is DeallocPartialRefInst, is DeallocRefInst, + is EndLifetimeInst: + return .continueWalk + case let si as StoringInstruction where si.sourceOperand == operand: return visitStoredUses(of: operand, into: si.destinationOperand.value) diff --git a/test/SILOptimizer/lifetime_dependence/semantics.swift b/test/SILOptimizer/lifetime_dependence/semantics.swift index cd6afd4d49a07..679f6342d319e 100644 --- a/test/SILOptimizer/lifetime_dependence/semantics.swift +++ b/test/SILOptimizer/lifetime_dependence/semantics.swift @@ -357,9 +357,10 @@ func testScopedOfInheritedWithLet(_ arg: [Int] ) { // TODO: should be // ✅ Safe: 'copySpan' result should be borrowed over `result` // rdar://128821299 ([nonescaping] extend borrowed arguments that are the source of a scoped dependence) let result = reborrowSpan(copySpan(span)) // expected-error {{lifetime-dependent variable 'result' escapes its scope}} - // expected-note @-1{{it depends on the lifetime of this parent value}} + // expected-note @-1{{it depends on the lifetime of this parent value}} + // expected-note @-2{{this use of the lifetime-dependent value is out of scope}} _ = result -} // expected-note {{this use of the lifetime-dependent value is out of scope}} +} // Test a scoped dependence on an inherited inout argument. // @@ -456,6 +457,14 @@ func testInoutMutableBorrow(a: inout [Int]) -> MutableSpan { a.mutableSpan() } +func testInoutTemporaryBorrow(a: inout [Int]) { + let span = a.span() + parse(span) + // 'read' access ends here even though span is still alive + // because span has no user-defined deinit. + a.append(10) +} + // ============================================================================= // Scoped dependence on property access // =============================================================================