Skip to content

Commit 552b665

Browse files
authored
Merge pull request #85334 from eeckstein/mandatory-destroy-hoisting
Optimizer: make destroy hoisting a mandatory pass
2 parents 66e7a78 + 04688a6 commit 552b665

31 files changed

+809
-79
lines changed

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ swift_compiler_sources(Optimizer
2929
LoopInvariantCodeMotion.swift
3030
ObjectOutliner.swift
3131
ObjCBridgingOptimization.swift
32+
MandatoryDestroyHoisting.swift
3233
MergeCondFails.swift
3334
NamedReturnValueOptimization.swift
3435
RedundantLoadElimination.swift
Lines changed: 263 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,263 @@
1+
//===--- MandatoryDestroyHoisting.swift ------------------------------------==//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2025 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import SIL
14+
15+
/// Hoists `destroy_value` instructions for non-lexical values.
16+
///
17+
/// ```
18+
/// %1 = some_ownedValue
19+
/// ...
20+
/// last_use(%1)
21+
/// ... // other instructions
22+
/// destroy_value %1
23+
/// ```
24+
/// ->
25+
/// ```
26+
/// %1 = some_ownedValue
27+
/// ...
28+
/// last_use(%1)
29+
/// destroy_value %1 // <- moved after the last use
30+
/// ... // other instructions
31+
/// ```
32+
///
33+
/// In contrast to non-mandatory optimization passes, this is the only pass which hoists destroys
34+
/// over deinit-barriers. This ensures consistent behavior in -Onone and optimized builds.
35+
///
36+
///
37+
let mandatoryDestroyHoisting = FunctionPass(name: "mandatory-destroy-hoisting") {
38+
(function: Function, context: FunctionPassContext) in
39+
40+
var endAccesses = Stack<EndAccessInst>(context)
41+
defer { endAccesses.deinitialize() }
42+
endAccesses.append(contentsOf: function.instructions.compactMap{ $0 as? EndAccessInst })
43+
44+
for block in function.blocks {
45+
for arg in block.arguments {
46+
hoistDestroys(of: arg, endAccesses: endAccesses, context)
47+
if !context.continueWithNextSubpassRun() {
48+
return
49+
}
50+
}
51+
for inst in block.instructions {
52+
for result in inst.results {
53+
hoistDestroys(of: result, endAccesses: endAccesses, context)
54+
if !context.continueWithNextSubpassRun(for: inst) {
55+
return
56+
}
57+
}
58+
}
59+
}
60+
}
61+
62+
private func hoistDestroys(of value: Value, endAccesses: Stack<EndAccessInst>, _ context: FunctionPassContext) {
63+
guard value.ownership == .owned,
64+
65+
// We must not violate side-effect dependencies of non-copyable deinits.
66+
// Therefore we don't handle non-copyable values.
67+
!value.type.isMoveOnly,
68+
69+
// Just a shortcut to avoid all the computations if there is no destroy at all.
70+
!value.uses.users(ofType: DestroyValueInst.self).isEmpty,
71+
72+
// Hoisting destroys is only legal for non-lexical lifetimes.
73+
!value.isInLexicalLiverange(context),
74+
75+
// Avoid compromimsing debug-info in Onone builds for source-level variables with non-lexical lifetimes.
76+
// For example COW types, like Array, which are "eager-move" and therefore not lexical.
77+
!needPreserveDebugInfo(of: value, context)
78+
else {
79+
return
80+
}
81+
82+
guard var liverange = Liverange(of: value, context) else {
83+
return
84+
}
85+
defer { liverange.deinitialize() }
86+
87+
// We must not move a destroy into an access scope, because the deinit can have an access scope as well.
88+
// And that would cause a false exclusivite error at runtime.
89+
liverange.extendWithAccessScopes(of: endAccesses)
90+
91+
var aliveDestroys = insertNewDestroys(of: value, in: liverange)
92+
defer { aliveDestroys.deinitialize() }
93+
94+
removeOldDestroys(of: value, ignoring: aliveDestroys, context)
95+
}
96+
97+
private func insertNewDestroys(of value: Value, in liverange: Liverange) -> InstructionSet {
98+
var aliveDestroys = InstructionSet(liverange.context)
99+
100+
if liverange.nonDestroyingUsers.isEmpty {
101+
// Handle the corner case where the value has no use at all (beside the destroy).
102+
immediatelyDestroy(value: value, ifIn: liverange, &aliveDestroys)
103+
return aliveDestroys
104+
}
105+
// Insert new destroys at the end of the pruned liverange.
106+
for user in liverange.nonDestroyingUsers {
107+
insertDestroy(of: value, after: user, ifIn: liverange, &aliveDestroys)
108+
}
109+
// Also, we need new destroys at exit edges from the pruned liverange.
110+
for exitInst in liverange.prunedLiverange.exits {
111+
insertDestroy(of: value, before: exitInst, ifIn: liverange, &aliveDestroys)
112+
}
113+
return aliveDestroys
114+
}
115+
116+
private func removeOldDestroys(of value: Value, ignoring: InstructionSet, _ context: FunctionPassContext) {
117+
for destroy in value.uses.users(ofType: DestroyValueInst.self) {
118+
if !ignoring.contains(destroy) {
119+
context.erase(instruction: destroy)
120+
}
121+
}
122+
}
123+
124+
private func insertDestroy(of value: Value,
125+
before insertionPoint: Instruction,
126+
ifIn liverange: Liverange,
127+
_ aliveDestroys: inout InstructionSet
128+
) {
129+
guard liverange.isOnlyInExtendedLiverange(insertionPoint) else {
130+
return
131+
}
132+
if let existingDestroy = insertionPoint as? DestroyValueInst, existingDestroy.destroyedValue == value {
133+
aliveDestroys.insert(existingDestroy)
134+
return
135+
}
136+
let builder = Builder(before: insertionPoint, liverange.context)
137+
let newDestroy = builder.createDestroyValue(operand: value)
138+
aliveDestroys.insert(newDestroy)
139+
}
140+
141+
private func insertDestroy(of value: Value,
142+
after insertionPoint: Instruction,
143+
ifIn liverange: Liverange,
144+
_ aliveDestroys: inout InstructionSet
145+
) {
146+
if let next = insertionPoint.next {
147+
insertDestroy(of: value, before: next, ifIn: liverange, &aliveDestroys)
148+
} else {
149+
for succ in insertionPoint.parentBlock.successors {
150+
insertDestroy(of: value, before: succ.instructions.first!, ifIn: liverange, &aliveDestroys)
151+
}
152+
}
153+
}
154+
155+
private func immediatelyDestroy(value: Value, ifIn liverange: Liverange, _ aliveDestroys: inout InstructionSet) {
156+
if let arg = value as? Argument {
157+
insertDestroy(of: value, before: arg.parentBlock.instructions.first!, ifIn: liverange, &aliveDestroys)
158+
} else {
159+
insertDestroy(of: value, after: value.definingInstruction!, ifIn: liverange, &aliveDestroys)
160+
}
161+
}
162+
163+
private func needPreserveDebugInfo(of value: Value, _ context: FunctionPassContext) -> Bool {
164+
if value.parentFunction.shouldOptimize {
165+
// No need to preserve debug info in optimized builds.
166+
return false
167+
}
168+
// Check if the value is associated to a source-level variable.
169+
if let inst = value.definingInstruction {
170+
return inst.findVarDecl() != nil
171+
}
172+
if let arg = value as? Argument {
173+
return arg.findVarDecl() != nil
174+
}
175+
return false
176+
}
177+
178+
/// Represents the "extended" liverange of a value which is the range after the last uses until the
179+
/// final destroys of the value.
180+
///
181+
/// ```
182+
/// %1 = definition -+ -+
183+
/// ... | pruned liverange |
184+
/// last_use(%1) -+ -+ | full liverange
185+
/// ... no uses of %1 | extended liverange |
186+
/// destroy_value %1 -+ -+
187+
/// ```
188+
private struct Liverange {
189+
var nonDestroyingUsers: Stack<Instruction>
190+
var prunedLiverange: InstructionRange
191+
var fullLiverange: InstructionRange
192+
let context: FunctionPassContext
193+
194+
init?(of value: Value, _ context: FunctionPassContext) {
195+
guard let users = Stack(usersOf: value, context) else {
196+
return nil
197+
}
198+
self.nonDestroyingUsers = users
199+
200+
self.prunedLiverange = InstructionRange(for: value, context)
201+
prunedLiverange.insert(contentsOf: nonDestroyingUsers)
202+
203+
self.fullLiverange = InstructionRange(for: value, context)
204+
fullLiverange.insert(contentsOf: value.users)
205+
206+
self.context = context
207+
}
208+
209+
func isOnlyInExtendedLiverange(_ instruction: Instruction) -> Bool {
210+
fullLiverange.inclusiveRangeContains(instruction) && !prunedLiverange.inclusiveRangeContains(instruction)
211+
}
212+
213+
mutating func extendWithAccessScopes(of endAccesses: Stack<EndAccessInst>) {
214+
var changed: Bool
215+
// We need to do this repeatedly because if access scopes are not nested properly, an overlapping scope
216+
// can make a non-overlapping scope also overlapping, e.g.
217+
// ```
218+
// %1 = begin_access // overlapping
219+
// last_use %value
220+
// %2 = begin_access // initially not overlapping, but overlapping because of scope %1
221+
// end_access %1
222+
// end_access %2
223+
// destroy_value %value
224+
// ```
225+
repeat {
226+
changed = false
227+
for endAccess in endAccesses {
228+
if isOnlyInExtendedLiverange(endAccess), !isOnlyInExtendedLiverange(endAccess.beginAccess) {
229+
prunedLiverange.insert(endAccess)
230+
nonDestroyingUsers.append(endAccess)
231+
changed = true
232+
}
233+
}
234+
} while changed
235+
}
236+
237+
mutating func deinitialize() {
238+
fullLiverange.deinitialize()
239+
prunedLiverange.deinitialize()
240+
nonDestroyingUsers.deinitialize()
241+
}
242+
}
243+
244+
private extension Stack where Element == Instruction {
245+
init?(usersOf value: Value, _ context: FunctionPassContext) {
246+
var users = Stack<Instruction>(context)
247+
248+
var visitor = InteriorUseWalker(definingValue: value, ignoreEscape: false, visitInnerUses: true, context) {
249+
if $0.instruction is DestroyValueInst, $0.value == value {
250+
return .continueWalk
251+
}
252+
users.append($0.instruction)
253+
return .continueWalk
254+
}
255+
defer { visitor.deinitialize() }
256+
257+
guard visitor.visitUses() == .continueWalk else {
258+
users.deinitialize()
259+
return nil
260+
}
261+
self = users
262+
}
263+
}

SwiftCompilerSources/Sources/Optimizer/PassManager/PassRegistration.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ private func registerSwiftPasses() {
8080
registerPass(computeSideEffects, { computeSideEffects.run($0) })
8181
registerPass(diagnoseInfiniteRecursion, { diagnoseInfiniteRecursion.run($0) })
8282
registerPass(destroyHoisting, { destroyHoisting.run($0) })
83+
registerPass(mandatoryDestroyHoisting, { mandatoryDestroyHoisting.run($0) })
8384
registerPass(initializeStaticGlobalsPass, { initializeStaticGlobalsPass.run($0) })
8485
registerPass(objCBridgingOptimization, { objCBridgingOptimization.run($0) })
8586
registerPass(objectOutliner, { objectOutliner.run($0) })

SwiftCompilerSources/Sources/Optimizer/Utilities/OptUtils.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,10 +72,10 @@ extension Value {
7272
switch v {
7373
case let fw as ForwardingInstruction:
7474
worklist.pushIfNotVisited(contentsOf: fw.definedOperands.values)
75+
case let ot as OwnershipTransitionInstruction where !(ot is CopyingInstruction):
76+
worklist.pushIfNotVisited(ot.operand.value)
7577
case let bf as BorrowedFromInst:
7678
worklist.pushIfNotVisited(bf.borrowedValue)
77-
case let bb as BeginBorrowInst:
78-
worklist.pushIfNotVisited(bb.borrowedValue)
7979
case let arg as Argument:
8080
if let phi = Phi(arg) {
8181
worklist.pushIfNotVisited(contentsOf: phi.incomingValues)

include/swift/SILOptimizer/PassManager/Passes.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ PASS(BooleanLiteralFolding, "boolean-literal-folding",
7171
"Constant folds initializers of boolean literals")
7272
PASS(DestroyHoisting, "destroy-hoisting",
7373
"Hoist destroy_value instructions")
74+
PASS(MandatoryDestroyHoisting, "mandatory-destroy-hoisting",
75+
"Hoist destroy_value instructions for non-lexical values")
7476
PASS(DeadEndBlockDumper, "dump-deadendblocks",
7577
"Tests the DeadEndBlocks utility")
7678
PASS(EscapeInfoDumper, "dump-escape-info",

include/swift/SILOptimizer/Utils/OSSACanonicalizeOwned.h

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,11 +480,16 @@ class OSSACanonicalizeOwned final {
480480
}
481481

482482
bool respectsDeinitBarriers() const {
483-
if (!currentDef->isLexical())
483+
auto &module = currentDef->getFunction()->getModule();
484+
485+
// The move-only checker (which runs in raw SIL) relies on ignoring deinit
486+
// barriers for non-lexical lifetimes.
487+
// Optimizations, on the other hand, should always respect deinit barriers.
488+
if (module.getStage() == SILStage::Raw && !currentDef->isLexical())
484489
return false;
490+
485491
if (currentDef->getFunction()->forceEnableLexicalLifetimes())
486492
return true;
487-
auto &module = currentDef->getFunction()->getModule();
488493
return module.getASTContext().SILOpts.supportsLexicalLifetimes(module);
489494
}
490495

lib/SILOptimizer/PassManager/PassPipeline.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,8 @@ static void addMandatoryDiagnosticOptPipeline(SILPassPipelinePlan &P) {
266266
P.addOnoneSimplification();
267267
P.addInitializeStaticGlobals();
268268

269+
P.addMandatoryDestroyHoisting();
270+
269271
// MandatoryPerformanceOptimizations might create specializations that are not
270272
// used, and by being unused they are might have unspecialized applies.
271273
// Eliminate them via the DeadFunctionAndGlobalElimination in embedded Swift

lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -408,19 +408,16 @@ static bool tryJoinIfDestroyConsumingUseInSameBlock(
408408
return true;
409409
}
410410

411-
// The lifetime of the original ends after the lifetime of the copy. If the
412-
// original is lexical, its lifetime must not be shortened through deinit
413-
// barriers.
414-
if (cvi->getOperand()->isLexical()) {
415-
// At this point, visitedInsts contains all the instructions between the
416-
// consuming use of the copy and the destroy. If any of those instructions
417-
// is a deinit barrier, it would be illegal to shorten the original lexical
418-
// value's lifetime to end at that consuming use. Bail if any are.
419-
if (llvm::any_of(visitedInsts, [](auto *inst) {
420-
return mayBeDeinitBarrierNotConsideringSideEffects(inst);
421-
}))
422-
return false;
423-
}
411+
// The lifetime of the original ends after the lifetime of the copy.
412+
// Its lifetime must not be shortened through deinit barriers.
413+
// At this point, visitedInsts contains all the instructions between the
414+
// consuming use of the copy and the destroy. If any of those instructions
415+
// is a deinit barrier, it would be illegal to shorten the original lexical
416+
// value's lifetime to end at that consuming use. Bail if any are.
417+
if (llvm::any_of(visitedInsts, [](auto *inst) {
418+
return mayBeDeinitBarrierNotConsideringSideEffects(inst);
419+
}))
420+
return false;
424421

425422
// If we reached this point, isUseBetweenInstAndBlockEnd succeeded implying
426423
// that we found destroy_value to be after our consuming use. Noting that

lib/SILOptimizer/Transforms/DestroyAddrHoisting.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -990,7 +990,7 @@ void DestroyAddrHoisting::hoistDestroys(
990990
if (!continueWithNextSubpassRun(asi))
991991
return;
992992
changed |= ::hoistDestroys(asi,
993-
/*ignoreDeinitBarriers=*/!asi->isLexical(),
993+
/*ignoreDeinitBarriers=*/false,
994994
remainingDestroyAddrs, deleter, calleeAnalysis);
995995
}
996996
// Arguments enclose everything.

0 commit comments

Comments
 (0)