Skip to content

Commit 31b3a91

Browse files
authored
Merge pull request #71703 from atrick/lifetime-extend
LifetimeDependenceScopeFixup: fix handling of returned dependence
2 parents fd93eea + eb3e9b0 commit 31b3a91

23 files changed

+1682
-296
lines changed

SwiftCompilerSources/Sources/Optimizer/DataStructures/InstructionRange.swift

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,16 +60,17 @@ struct InstructionRange : CustomStringConvertible, NoReflectionChildren {
6060
}
6161

6262
init(for value: Value, _ context: some Context) {
63-
var begin: Instruction
63+
self = InstructionRange(begin: InstructionRange.beginningInstruction(for: value), context)
64+
}
65+
66+
static func beginningInstruction(for value: Value) -> Instruction {
6467
if let def = value.definingInstruction {
65-
begin = def
68+
return def
6669
} else if let result = TerminatorResult(value) {
67-
begin = result.terminator
68-
} else {
69-
assert(Phi(value) != nil || value is FunctionArgument)
70-
begin = value.parentBlock.instructions.first!
70+
return result.terminator
7171
}
72-
self = InstructionRange(begin: begin, context)
72+
assert(Phi(value) != nil || value is FunctionArgument)
73+
return value.parentBlock.instructions.first!
7374
}
7475

7576
/// Insert a potential end instruction.

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift

Lines changed: 13 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ let lifetimeDependenceDiagnosticsPass = FunctionPass(
3232
if !context.options.hasFeature(.NonescapableTypes) {
3333
return
3434
}
35-
log("Diagnosing lifetime dependence in \(function.name)")
35+
log(" --- Diagnosing lifetime dependence in \(function.name)")
3636
log("\(function)")
3737

3838
for argument in function.arguments where !argument.type.isEscapable {
@@ -236,32 +236,22 @@ private struct LifetimeVariable {
236236
self = Self(accessBase: value.accessBase, context)
237237
return
238238
}
239-
if let firstIntroducer = getFirstBorrowIntroducer(of: value, context) {
239+
if let firstIntroducer = getFirstVariableIntroducer(of: value, context) {
240240
self = Self(introducer: firstIntroducer)
241241
return
242242
}
243243
self.varDecl = nil
244244
self.sourceLoc = nil
245245
}
246246

247-
// FUTURE: consider diagnosing multiple variable introducers. It's
248-
// unclear how more than one can happen.
249-
private func getFirstBorrowIntroducer(of value: Value,
250-
_ context: some Context)
251-
-> Value? {
252-
var introducers = Stack<Value>(context)
253-
gatherBorrowIntroducers(for: value, in: &introducers, context)
254-
return introducers.pop()
255-
}
256-
257-
private func getFirstLifetimeIntroducer(of value: Value,
258-
_ context: some Context)
259-
-> Value? {
247+
private func getFirstVariableIntroducer(of value: Value, _ context: some Context) -> Value? {
260248
var introducer: Value?
261-
_ = visitLifetimeIntroducers(for: value, context) {
249+
var useDefVisitor = VariableIntroducerUseDefWalker(context) {
262250
introducer = $0
263251
return .abortWalk
264252
}
253+
defer { useDefVisitor.deinitialize() }
254+
_ = useDefVisitor.walkUp(valueOrAddress: value)
265255
return introducer
266256
}
267257

@@ -283,12 +273,11 @@ private struct LifetimeVariable {
283273
private init(accessBase: AccessBase, _ context: some Context) {
284274
switch accessBase {
285275
case .box(let projectBox):
286-
if let box = getFirstLifetimeIntroducer(of: projectBox.box, context) {
287-
self = Self(introducer: box)
288-
}
289-
// We should always find an introducer since boxes are nontrivial.
290-
self.varDecl = nil
291-
self.sourceLoc = nil
276+
// Note: referenceRoot looks through `begin_borrow [var_decl]` and `move_value [var_decl]`. But the box should
277+
// never be produced by one of these, except when it is redundant with the `alloc_box` VarDecl. It does not seem
278+
// possible for a box to be moved/borrowed directly into another variable's box. Reassignment always loads/stores
279+
// the value.
280+
self = Self(introducer: projectBox.box.referenceRoot)
292281
case .stack(let allocStack):
293282
self = Self(introducer: allocStack)
294283
case .global(let globalVar):
@@ -328,13 +317,14 @@ private struct LifetimeVariable {
328317
private struct DiagnoseDependenceWalker {
329318
let context: Context
330319
var diagnostics: DiagnoseDependence
320+
let localReachabilityCache = LocalVariableReachabilityCache()
331321
var visitedValues: ValueSet
332322

333323
var function: Function { diagnostics.function }
334324

335325
init(_ diagnostics: DiagnoseDependence, _ context: Context) {
336-
self.diagnostics = diagnostics
337326
self.context = context
327+
self.diagnostics = diagnostics
338328
self.visitedValues = ValueSet(context)
339329
}
340330

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceInsertion.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ let lifetimeDependenceInsertionPass = FunctionPass(
3434
if !context.options.hasFeature(.NonescapableTypes) {
3535
return
3636
}
37-
log("Inserting lifetime dependence markers in \(function.name)")
37+
log(" --- Inserting lifetime dependence markers in \(function.name)")
3838

3939
for instruction in function.instructions {
4040
if let dependentApply = LifetimeDependentApply(instruction) {

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceScopeFixup.swift

Lines changed: 131 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ private func log(_ message: @autoclosure () -> String) {
3131
let lifetimeDependenceScopeFixupPass = FunctionPass(
3232
name: "lifetime-dependence-scope-fixup")
3333
{ (function: Function, context: FunctionPassContext) in
34-
log("Scope fixup for lifetime dependence in \(function.name)")
34+
log(" --- Scope fixup for lifetime dependence in \(function.name)")
35+
36+
let localReachabilityCache = LocalVariableReachabilityCache()
3537

3638
for instruction in function.instructions {
3739
guard let markDep = instruction as? MarkDependenceInst else {
@@ -40,98 +42,156 @@ let lifetimeDependenceScopeFixupPass = FunctionPass(
4042
guard let lifetimeDep = LifetimeDependence(markDep, context) else {
4143
continue
4244
}
43-
guard let beginAccess = extendAccessScopes(dependence: lifetimeDep,
44-
context) else {
45-
continue
45+
if let arg = extendAccessScopes(dependence: lifetimeDep, localReachabilityCache, context) {
46+
markDep.baseOperand.set(to: arg, context)
4647
}
47-
extendDependenceBase(dependenceInstruction: markDep,
48-
beginAccess: beginAccess, context)
4948
}
5049
}
5150

52-
// Extend all access scopes that enclose `dependence` and return the
53-
// outermost access.
51+
/// Extend all access scopes that enclose `dependence`. If dependence is on an access scope in the caller, then return
52+
/// the function argument that represents the dependence scope.
5453
private func extendAccessScopes(dependence: LifetimeDependence,
55-
_ context: FunctionPassContext) -> BeginAccessInst? {
54+
_ localReachabilityCache: LocalVariableReachabilityCache,
55+
_ context: FunctionPassContext) -> FunctionArgument? {
5656
log("Scope fixup for lifetime dependent instructions: \(dependence)")
5757

58-
guard case .access(let bai) = dependence.scope else {
58+
guard case .access(let beginAccess) = dependence.scope else {
59+
return nil
60+
}
61+
let function = beginAccess.parentFunction
62+
63+
// Get the range accessBase lifetime. The accessRange cannot exceed this without producing invalid SIL.
64+
guard var ownershipRange = AddressOwnershipLiveRange.compute(for: beginAccess.address, at: beginAccess,
65+
localReachabilityCache, context) else {
5966
return nil
6067
}
61-
var range = InstructionRange(begin: bai, context)
62-
var walker = LifetimeDependenceScopeFixupWalker(bai.parentFunction, context) {
63-
range.insert($0.instruction)
68+
defer { ownershipRange.deinitialize() }
69+
70+
var accessRange = InstructionRange(begin: beginAccess, context)
71+
defer {accessRange.deinitialize()}
72+
73+
var walker = LifetimeDependenceScopeFixupWalker(function, localReachabilityCache, context) {
74+
// Do not extend the accessRange past the ownershipRange.
75+
let dependentInst = $0.instruction
76+
if ownershipRange.coversUse(dependentInst) {
77+
accessRange.insert(dependentInst)
78+
}
6479
return .continueWalk
6580
}
6681
defer {walker.deinitialize()}
82+
6783
_ = walker.walkDown(root: dependence.dependentValue)
68-
defer {range.deinitialize()}
69-
70-
var beginAccess = bai
71-
while (true) {
72-
var endAcceses = [Instruction]()
73-
// Collect original end_access instructions
74-
for end in beginAccess.endInstructions {
75-
endAcceses.append(end)
76-
}
7784

78-
// Insert original end_access instructions to prevent access scope shortening
79-
range.insert(contentsOf: endAcceses)
80-
assert(!range.ends.isEmpty)
85+
// Lifetime dependenent uses may not be dominated by the access. The dependent value may be used by a phi or stored
86+
// into a memory location. The access may be conditional relative to such uses. If any use was not dominated, then
87+
// `accessRange` will include the function entry.
88+
let firstInst = function.entryBlock.instructions.first!
89+
if firstInst != beginAccess, accessRange.contains(firstInst) {
90+
return nil
91+
}
92+
if let arg = extendAccessScope(beginAccess: beginAccess, range: &accessRange, context) {
93+
// If the dependent value is returned, then return the FunctionArgument that it depends on.
94+
assert(walker.dependsOnCaller)
95+
return arg
96+
}
97+
return nil
98+
}
8199

82-
// Create new end_access at the end of extended uses
83-
for end in range.ends {
84-
let endBuilder = Builder(after: end, context)
85-
_ = endBuilder.createEndAccess(beginAccess: beginAccess)
100+
/// Extend this access scope to cover the dependent uses. Recursively extend outer accesses to maintain nesting.
101+
///
102+
/// Note that we cannot simply rewrite the `mark_dependence` to depend on an outer access scope. For 'read' access, this
103+
/// could let us avoid extending the inner scope, but that would not accomplish anything useful because inner 'read's
104+
/// can always be extended up to the extent of their outer 'read' (ignoring the special case when the dependence is on a
105+
/// caller scope, which is handled separately). A nested 'read' access can never interfere with another access in the
106+
/// same outer 'read', because it is impossible to nest a 'modify' access within a 'read'. For 'modify' accesses,
107+
/// however, the inner scope must be extended for correctness. A 'modify' access can interfere with other 'modify'
108+
/// accesss in the same scope. We rely on exclusivity diagnostics to report these interferences. For example:
109+
///
110+
/// sil @foo : $(@inout C) -> () {
111+
/// bb0(%0 : $*C):
112+
/// %a1 = begin_access [modify] %0
113+
/// %d = apply @getDependent(%a1)
114+
/// mark_dependence [unresolved] %d on %a1
115+
/// end_access %a1
116+
/// %a2 = begin_access [modify] %0
117+
/// ...
118+
/// end_access %a2
119+
/// apply @useDependent(%d) // exclusivity violation
120+
/// return
121+
/// }
122+
///
123+
/// The call to `@useDependent` is an exclusivity violation because it uses a value that depends on a 'modify'
124+
/// access. This scope fixup pass must extend '%a1' to cover the `@useDependent` but must not extend the base of the
125+
/// `mark_dependence` to the outer access `%0`. This ensures that exclusivity diagnostics correctly reports the
126+
/// violation, and that subsequent optimizations do not shrink the inner access `%a1`.
127+
private func extendAccessScope(beginAccess: BeginAccessInst, range: inout InstructionRange,
128+
_ context: FunctionPassContext) -> FunctionArgument? {
129+
var endAcceses = [Instruction]()
130+
// Collect the original end_access instructions and extend the range to to cover them. The resulting access scope must
131+
// cover the original scope because it may protect other memory operations.
132+
var requiresExtension = false
133+
for end in beginAccess.endInstructions {
134+
endAcceses.append(end)
135+
if range.contains(end) {
136+
// If any end_access is inside the new range, then all end_accesses must be rewritten.
137+
requiresExtension = true
138+
} else {
139+
range.insert(end)
86140
}
141+
}
142+
if !requiresExtension {
143+
return nil
144+
}
145+
assert(!range.ends.isEmpty)
87146

88-
// Delete original end_access instructions
89-
for endAccess in endAcceses {
90-
context.erase(instruction: endAccess)
147+
// Create new end_access at the end of extended uses
148+
var dependsOnCaller = false
149+
for end in range.ends {
150+
let location = end.location.autoGenerated
151+
if end is ReturnInst {
152+
dependsOnCaller = true
153+
let endAccess = Builder(before: end, location: location, context).createEndAccess(beginAccess: beginAccess)
154+
range.insert(endAccess)
155+
continue
91156
}
92-
93-
// TODO: Add SIL support for lifetime dependence and write unit test
94-
// for nested access scopes
95-
guard case let .scope(enclosingBeginAccess) = beginAccess.address.enclosingAccessScope else {
96-
break
157+
Builder.insert(after: end, location: location, context) {
158+
let endAccess = $0.createEndAccess(beginAccess: beginAccess)
159+
// This scope should be nested in any outer scopes.
160+
range.insert(endAccess)
97161
}
98-
beginAccess = enclosingBeginAccess
99162
}
100-
return beginAccess
101-
}
102-
103-
/// Rewrite the mark_dependence to depend on the outermost access
104-
/// scope now that the nested scopes have all been extended.
105-
private func extendDependenceBase(dependenceInstruction: MarkDependenceInst,
106-
beginAccess: BeginAccessInst,
107-
_ context: FunctionPassContext) {
108-
guard case let .base(accessBase) = beginAccess.address.enclosingAccessScope
109-
else {
110-
fatalError("this must be the outer-most access scope")
111-
}
112-
// If the outermost access is in the caller, then depende on the
113-
// address argument.
114-
let baseAddress: Value
115-
switch accessBase {
116-
case let .argument(arg):
117-
assert(arg.type.isAddress)
118-
baseAddress = arg
119-
default:
120-
baseAddress = beginAccess
121-
}
122-
dependenceInstruction.baseOperand.set(to: baseAddress, context)
163+
// Delete original end_access instructions
164+
for endAccess in endAcceses {
165+
context.erase(instruction: endAccess)
166+
}
167+
// TODO: Add SIL support for lifetime dependence and write unit test for nested access scopes
168+
switch beginAccess.address.enclosingAccessScope {
169+
case let .scope(enclosingBeginAccess):
170+
return extendAccessScope(beginAccess: enclosingBeginAccess, range: &range, context)
171+
case let .base(accessBase):
172+
if case let .argument(arg) = accessBase, dependsOnCaller {
173+
return arg
174+
}
175+
return nil
176+
}
123177
}
124178

125179
private struct LifetimeDependenceScopeFixupWalker : LifetimeDependenceDefUseWalker {
126180
let function: Function
127181
let context: Context
128182
let visitor: (Operand) -> WalkResult
183+
let localReachabilityCache: LocalVariableReachabilityCache
129184
var visitedValues: ValueSet
130185

131-
init(_ function: Function, _ context: Context, visitor: @escaping (Operand) -> WalkResult) {
186+
/// Set to true if the dependence is returned from the current function.
187+
var dependsOnCaller = false
188+
189+
init(_ function: Function, _ localReachabilityCache: LocalVariableReachabilityCache, _ context: Context,
190+
visitor: @escaping (Operand) -> WalkResult) {
132191
self.function = function
133192
self.context = context
134193
self.visitor = visitor
194+
self.localReachabilityCache = localReachabilityCache
135195
self.visitedValues = ValueSet(context)
136196
}
137197

@@ -157,16 +217,21 @@ private struct LifetimeDependenceScopeFixupWalker : LifetimeDependenceDefUseWalk
157217

158218
mutating func escapingDependence(on operand: Operand) -> WalkResult {
159219
_ = visitor(operand)
160-
return .abortWalk
220+
// Make a best-effort attempt to extend the access scope regardless of escapes. It is possible that some mandatory
221+
// pass between scope fixup and diagnostics will make it possible for the LifetimeDependenceDefUseWalker to analyze
222+
// this use.
223+
return .continueWalk
161224
}
162225

163-
mutating func returnedDependence(result: Operand) -> WalkResult {
164-
return .continueWalk
226+
mutating func returnedDependence(result operand: Operand) -> WalkResult {
227+
dependsOnCaller = true
228+
return visitor(operand)
165229
}
166230

167231
mutating func returnedDependence(address: FunctionArgument,
168232
using operand: Operand) -> WalkResult {
169-
return .continueWalk
233+
dependsOnCaller = true
234+
return visitor(operand)
170235
}
171236

172237
mutating func yieldedDependence(result: Operand) -> WalkResult {

0 commit comments

Comments
 (0)