Skip to content

Commit 2f6c4ad

Browse files
authored
Merge pull request #72513 from atrick/markdep_escape
SIL: Fix handling of mark_dependence [nonescaping] in several utilities
2 parents 8b4821f + 560a89b commit 2f6c4ad

27 files changed

+368
-168
lines changed

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceDiagnostics.swift

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,9 @@ import SIL
1414

1515
private let verbose = false
1616

17-
private func log(_ message: @autoclosure () -> String) {
17+
private func log(prefix: Bool = true, _ message: @autoclosure () -> String) {
1818
if verbose {
19-
print("### \(message())")
19+
print((prefix ? "### " : "") + message())
2020
}
2121
}
2222

@@ -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(prefix: false, "\n--- Diagnosing lifetime dependence in \(function.name)")
3636
log("\(function)")
3737

3838
for argument in function.arguments
@@ -297,6 +297,8 @@ private struct LifetimeVariable {
297297
// TODO: bridge VarDecl for FunctionConvention.Yields
298298
self.varDecl = nil
299299
self.sourceLoc = result.parentInstruction.location.sourceLoc
300+
case .storeBorrow(let sb):
301+
self = .init(dependent: sb.source, context)
300302
case .pointer(let ptrToAddr):
301303
self.varDecl = nil
302304
self.sourceLoc = ptrToAddr.location.sourceLoc

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceInsertion.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ import SIL
2222

2323
private let verbose = false
2424

25-
private func log(_ message: @autoclosure () -> String) {
25+
private func log(prefix: Bool = true, _ message: @autoclosure () -> String) {
2626
if verbose {
27-
print("### \(message())")
27+
print((prefix ? "### " : "") + message())
2828
}
2929
}
3030

@@ -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(prefix: false, "\n--- 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: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ import SIL
2222

2323
private let verbose = false
2424

25-
private func log(_ message: @autoclosure () -> String) {
25+
private func log(prefix: Bool = true, _ message: @autoclosure () -> String) {
2626
if verbose {
27-
print("### \(message())")
27+
print((prefix ? "### " : "") + message())
2828
}
2929
}
3030

@@ -34,7 +34,7 @@ let lifetimeDependenceScopeFixupPass = FunctionPass(
3434
if !context.options.hasFeature(.NonescapableTypes) {
3535
return
3636
}
37-
log(" --- Scope fixup for lifetime dependence in \(function.name)")
37+
log(prefix: false, "\n--- Scope fixup for lifetime dependence in \(function.name)")
3838

3939
let localReachabilityCache = LocalVariableReachabilityCache()
4040

SwiftCompilerSources/Sources/Optimizer/ModulePasses/StackProtection.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ private extension AccessBase {
455455

456456
var isStackAllocated: IsStackAllocatedResult {
457457
switch self {
458-
case .stack:
458+
case .stack, .storeBorrow:
459459
return .yes
460460
case .box, .global:
461461
return .no

SwiftCompilerSources/Sources/Optimizer/Utilities/AddressUtils.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,8 @@ enum AddressOwnershipLiveRange : CustomStringConvertible {
458458
default:
459459
return nil
460460
}
461+
case .storeBorrow(let sb):
462+
return computeValueLiveRange(of: sb.source, context)
461463
case .pointer, .unidentified:
462464
return nil
463465
}

SwiftCompilerSources/Sources/Optimizer/Utilities/BorrowUtils.swift

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,11 @@ enum BorrowingInstruction : CustomStringConvertible, Hashable {
206206
/// incoming value dominates or is consumed by an outer adjacent
207207
/// phi. See InteriorLiveness.
208208
///
209+
/// FIXME: To generate conservatively correct liveness, this should return
210+
/// .abortWalk if this is a mark_dependence and the scope-ending use is not
211+
/// the last in the function (e.g. a store rather than a destroy or return).
212+
/// The client needs to use LifetimeDependenceDefUseWalker to do better.
213+
///
209214
/// TODO: to hande reborrow-extended uses, migrate ExtendedLiveness
210215
/// to SwiftCompilerSources.
211216
///

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -307,22 +307,21 @@ extension LifetimeDependence.Scope {
307307
/// forwarded (via struct/tuple) from multiple guaranteed values.
308308
init?(base: Value, _ context: some Context) {
309309
if base.type.isAddress {
310-
if let scope = Self(address: base, context) {
311-
self = scope
312-
return
310+
guard let scope = Self(address: base, context) else {
311+
return nil
313312
}
314-
return nil
313+
self = scope
314+
return
315315
}
316316
switch base.ownership {
317317
case .owned:
318318
self = .owned(base)
319319
return
320320
case .guaranteed:
321-
if let scope = Self(guaranteed: base, context) {
322-
self = scope
323-
return
321+
guard let scope = Self(guaranteed: base, context) else {
322+
return nil
324323
}
325-
return nil
324+
self = scope
326325
case .none:
327326
// lifetime dependence requires a nontrivial value"
328327
return nil
@@ -339,23 +338,23 @@ extension LifetimeDependence.Scope {
339338
switch accessBase {
340339
case let .box(projectBox):
341340
// Note: the box may be in a borrow scope.
342-
if let scope = Self(base: projectBox.operand.value, context) {
343-
self = scope
341+
guard let scope = Self(base: projectBox.operand.value, context) else {
342+
return nil
344343
}
345-
return nil
344+
self = scope
346345
case let .stack(allocStack):
347-
if let scope = Self(allocation: allocStack, context) {
348-
self = scope
346+
guard let scope = Self(allocation: allocStack, context) else {
347+
return nil
349348
}
350-
return nil
349+
self = scope
351350
case .global:
352351
self = .unknown(address)
353352
case .class, .tail:
354353
let refElt = address as! UnaryInstruction
355-
if let scope = Self(guaranteed: refElt.operand.value, context) {
356-
self = scope
354+
guard let scope = Self(guaranteed: refElt.operand.value, context) else {
355+
return nil
357356
}
358-
return nil
357+
self = scope
359358
case let .argument(arg):
360359
if arg.convention.isIndirectIn {
361360
self = .initialized(initialAddress: arg, initializingStore: nil)
@@ -370,6 +369,11 @@ extension LifetimeDependence.Scope {
370369
}
371370
case let .yield(result):
372371
self = Self(yield: result)
372+
case .storeBorrow(let sb):
373+
guard let scope = Self(base: sb.source, context) else {
374+
return nil
375+
}
376+
self = scope
373377
case .pointer, .unidentified:
374378
self = .unknown(address)
375379
}
@@ -1085,7 +1089,8 @@ extension LifetimeDependenceDefUseWalker {
10851089
return returnedDependence(address: arg, using: operand)
10861090
}
10871091
break
1088-
case .global, .class, .tail, .yield, .pointer, .unidentified:
1092+
case .global, .class, .tail, .yield, .storeBorrow, .pointer, .unidentified:
1093+
// An address produced by .storeBorrow should never be stored into.
10891094
break
10901095
}
10911096
if let allocation = allocation {

SwiftCompilerSources/Sources/SIL/Instruction.swift

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1230,7 +1230,15 @@ final public class BeginBorrowInst : SingleValueInstruction, UnaryInstruction, B
12301230

12311231
final public class LoadBorrowInst : SingleValueInstruction, LoadInstruction, BorrowIntroducingInstruction {}
12321232

1233-
final public class StoreBorrowInst : SingleValueInstruction, StoringInstruction, BorrowIntroducingInstruction { }
1233+
final public class StoreBorrowInst : SingleValueInstruction, StoringInstruction, BorrowIntroducingInstruction {
1234+
var allocStack: AllocStackInst {
1235+
var dest = destination
1236+
if let mark = dest as? MarkUnresolvedNonCopyableValueInst {
1237+
dest = mark.operand.value
1238+
}
1239+
return dest as! AllocStackInst
1240+
}
1241+
}
12341242

12351243
final public class BeginAccessInst : SingleValueInstruction, UnaryInstruction {
12361244
// The raw values must match SILAccessKind.

SwiftCompilerSources/Sources/SIL/Utilities/AccessUtils.swift

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@ public enum AccessBase : CustomStringConvertible, Hashable {
6969
/// An indirect result of a `begin_apply`.
7070
case yield(MultipleValueInstructionResult)
7171

72+
/// store_borrow is never the base of a formal access, but calling Value.enclosingScope on an arbitrary address will
73+
/// return it as the accessBase. A store_borrow always stores into an alloc_stack, but it is handled separately
74+
/// because it may be useful for clients to know which value was stored in the temporary stack location for the
75+
/// duration of this borrow scope.
76+
case storeBorrow(StoreBorrowInst)
77+
7278
/// An address which is derived from a `Builtin.RawPointer`.
7379
case pointer(PointerToAddressInst)
7480

@@ -90,6 +96,8 @@ public enum AccessBase : CustomStringConvertible, Hashable {
9096
} else {
9197
self = .unidentified
9298
}
99+
case let sb as StoreBorrowInst:
100+
self = .storeBorrow(sb)
93101
default:
94102
self = .unidentified
95103
}
@@ -105,6 +113,7 @@ public enum AccessBase : CustomStringConvertible, Hashable {
105113
case .tail(let rta): return "tail - \(rta.instance)"
106114
case .argument(let arg): return "argument - \(arg)"
107115
case .yield(let result): return "yield - \(result)"
116+
case .storeBorrow(let sb): return "storeBorrow - \(sb)"
108117
case .pointer(let p): return "pointer - \(p)"
109118
}
110119
}
@@ -114,7 +123,7 @@ public enum AccessBase : CustomStringConvertible, Hashable {
114123
switch self {
115124
case .class, .tail:
116125
return true
117-
case .box, .stack, .global, .argument, .yield, .pointer, .unidentified:
126+
case .box, .stack, .global, .argument, .yield, .storeBorrow, .pointer, .unidentified:
118127
return false
119128
}
120129
}
@@ -125,7 +134,7 @@ public enum AccessBase : CustomStringConvertible, Hashable {
125134
case .box(let pbi): return pbi.box
126135
case .class(let rea): return rea.instance
127136
case .tail(let rta): return rta.instance
128-
case .stack, .global, .argument, .yield, .pointer, .unidentified:
137+
case .stack, .global, .argument, .yield, .storeBorrow, .pointer, .unidentified:
129138
return nil
130139
}
131140
}
@@ -153,7 +162,7 @@ public enum AccessBase : CustomStringConvertible, Hashable {
153162
switch self {
154163
case .class(let rea): return rea.fieldIsLet
155164
case .global(let g): return g.isLet
156-
case .box, .stack, .tail, .argument, .yield, .pointer, .unidentified:
165+
case .box, .stack, .tail, .argument, .yield, .storeBorrow, .pointer, .unidentified:
157166
return false
158167
}
159168
}
@@ -164,7 +173,7 @@ public enum AccessBase : CustomStringConvertible, Hashable {
164173
case .box(let pbi): return pbi.box.referenceRoot is AllocBoxInst
165174
case .class(let rea): return rea.instance.referenceRoot is AllocRefInstBase
166175
case .tail(let rta): return rta.instance.referenceRoot is AllocRefInstBase
167-
case .stack: return true
176+
case .stack, .storeBorrow: return true
168177
case .global, .argument, .yield, .pointer, .unidentified:
169178
return false
170179
}
@@ -173,7 +182,7 @@ public enum AccessBase : CustomStringConvertible, Hashable {
173182
/// True, if the kind of storage of the access is known (e.g. a class property, or global variable).
174183
public var hasKnownStorageKind: Bool {
175184
switch self {
176-
case .box, .class, .tail, .stack, .global:
185+
case .box, .class, .tail, .stack, .storeBorrow, .global:
177186
return true
178187
case .argument, .yield, .pointer, .unidentified:
179188
return false
@@ -203,6 +212,8 @@ public enum AccessBase : CustomStringConvertible, Hashable {
203212
return arg1 == arg2
204213
case (.yield(let baResult1), .yield(let baResult2)):
205214
return baResult1 == baResult2
215+
case (.storeBorrow(let sb1), .storeBorrow(let sb2)):
216+
return sb1 == sb2
206217
case (.pointer(let p1), .pointer(let p2)):
207218
return p1 == p2
208219
default:
@@ -260,6 +271,15 @@ public enum AccessBase : CustomStringConvertible, Hashable {
260271
case (_, .argument(let otherArg)):
261272
return argIsDistinct(otherArg, from: self)
262273

274+
case (.storeBorrow(let arg), .storeBorrow(let otherArg)):
275+
return arg.allocStack != otherArg.allocStack
276+
277+
// A StoreBorrow location can only be used by other StoreBorrows.
278+
case (.storeBorrow, _):
279+
return true
280+
case (_, .storeBorrow):
281+
return true
282+
263283
default:
264284
// As we already handled pairs of the same kind, here we handle pairs with different kinds.
265285
// Different storage kinds cannot alias, regardless where the storage comes from.
@@ -591,7 +611,7 @@ extension ValueUseDefWalker where Path == SmallProjectionPath {
591611
return walkUp(value: rea.instance, path: path.push(.classField, index: rea.fieldIndex)) != .abortWalk
592612
case .tail(let rta):
593613
return walkUp(value: rta.instance, path: path.push(.tailElements, index: 0)) != .abortWalk
594-
case .stack, .global, .argument, .yield, .pointer, .unidentified:
614+
case .stack, .global, .argument, .yield, .storeBorrow, .pointer, .unidentified:
595615
return false
596616
}
597617
}

include/swift/SIL/OwnershipUseVisitor.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,13 @@ bool OwnershipUseVisitor<Impl>::visitInnerBorrow(Operand *borrowingOperand) {
258258
return false;
259259

260260
return BorrowingOperand(borrowingOperand)
261-
.visitScopeEndingUses([&](Operand *borrowEnd) {
262-
return visitInnerBorrowScopeEnd(borrowEnd);
263-
});
261+
.visitScopeEndingUses(
262+
[&](Operand *borrowEnd) {
263+
return visitInnerBorrowScopeEnd(borrowEnd);
264+
},
265+
[&](Operand *unknownUse) {
266+
return asImpl().handlePointerEscape(unknownUse);
267+
});
264268
}
265269

266270
template <typename Impl>

include/swift/SIL/OwnershipUtils.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -360,12 +360,16 @@ struct BorrowingOperand {
360360
/// over a region of code instead of just for a single instruction, visit
361361
/// those uses.
362362
///
363-
/// Returns false and early exits if the visitor \p func returns false.
363+
/// Returns false and early exits if the \p visitScopeEnd or \p
364+
/// visitUnknownUse returns false.
364365
///
365366
/// For an instantaneous borrow, such as apply, this visits no uses. For
366367
/// begin_apply it visits the end_apply uses. For borrow introducers, it
367368
/// visits the end of the introduced borrow scope.
368-
bool visitScopeEndingUses(function_ref<bool(Operand *)> func) const;
369+
bool visitScopeEndingUses(function_ref<bool(Operand *)> visitScopeEnd,
370+
function_ref<bool(Operand *)> visitUnknownUse
371+
= [](Operand *){ return false; })
372+
const;
369373

370374
/// Returns true for borrows that create a local borrow scope but have no
371375
/// scope-ending uses (presumably all paths are dead-end blocks). This does
@@ -381,7 +385,10 @@ struct BorrowingOperand {
381385
/// BorrowingOperand.
382386
///
383387
/// Returns false and early exits if the visitor \p func returns false.
384-
bool visitExtendedScopeEndingUses(function_ref<bool(Operand *)> func) const;
388+
bool visitExtendedScopeEndingUses(
389+
function_ref<bool(Operand *)> func,
390+
function_ref<bool(Operand *)> visitUnknownUse
391+
= [](Operand *){ return false; }) const;
385392

386393
/// Returns true if this borrow scope operand consumes guaranteed
387394
/// values and produces a new scope afterwards.

include/swift/SIL/PrunedLiveness.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,9 @@ class PrunedLiveBlocks {
289289
/// scope. Reborrows within nested borrows scopes are already summarized by the
290290
/// outer borrow scope.
291291
enum class InnerBorrowKind {
292-
Contained, // any borrows are fully contained within this live range
293-
Reborrowed // at least one immediately nested borrow is reborrowed
292+
Contained, // any borrows are fully contained within this live range
293+
Reborrowed, // at least one immediately nested borrow is reborrowed
294+
Escaped // the end of the borrow scope is indeterminate
294295
};
295296

296297
inline InnerBorrowKind meet(InnerBorrowKind lhs, InnerBorrowKind rhs) {

include/swift/SIL/SILInstruction.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8546,8 +8546,9 @@ class MarkDependenceInst
85468546
}
85478547

85488548
/// Visit the instructions that end the lifetime of an OSSA on-stack closure.
8549-
bool visitNonEscapingLifetimeEnds(llvm::function_ref<bool (Operand*)> func)
8550-
const;
8549+
bool visitNonEscapingLifetimeEnds(
8550+
llvm::function_ref<bool (Operand*)> visitScopeEnd,
8551+
llvm::function_ref<bool (Operand*)> visitUnknownUse) const;
85518552
};
85528553

85538554
/// Promote an Objective-C block that is on the stack to the heap, or simply

0 commit comments

Comments
 (0)