Skip to content

Commit f9d310b

Browse files
meg-guptameg-gupta
committed
While verifiying mark_dependence [nonescaping], skip visiting phis
TODO: Handle verification of mark_dependence [nonescaping] with phi uses separately in the ownership verifier
1 parent bd1b99b commit f9d310b

File tree

7 files changed

+86
-32
lines changed

7 files changed

+86
-32
lines changed

‎lib/SIL/IR/SILInstruction.cpp

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1792,11 +1792,10 @@ bool SILInstruction::maySuspend() const {
17921792
return false;
17931793
}
17941794

1795-
static bool visitRecursivelyLifetimeEndingUses(
1796-
SILValue i,
1797-
bool &noUsers,
1798-
llvm::function_ref<bool(Operand *)> func)
1799-
{
1795+
static bool
1796+
visitRecursivelyLifetimeEndingUses(SILValue i, bool &noUsers,
1797+
llvm::function_ref<bool(Operand *)> func,
1798+
bool skipVisitingPhis) {
18001799
for (Operand *use : i->getConsumingUses()) {
18011800
noUsers = false;
18021801
if (isa<DestroyValueInst>(use->getUser())) {
@@ -1813,6 +1812,14 @@ static bool visitRecursivelyLifetimeEndingUses(
18131812
}
18141813
continue;
18151814
}
1815+
if (skipVisitingPhis) {
1816+
if (PhiOperand(use)) {
1817+
if (!func(use)) {
1818+
return false;
1819+
}
1820+
continue;
1821+
}
1822+
}
18161823
// FIXME: Handle store to indirect result
18171824

18181825
// There shouldn't be any dead-end consumptions of a nonescaping
@@ -1835,7 +1842,8 @@ static bool visitRecursivelyLifetimeEndingUses(
18351842
"forwarded to a destroy_value");
18361843
}
18371844
for (auto res : use->getUser()->getResults()) {
1838-
if (!visitRecursivelyLifetimeEndingUses(res, noUsers, func)) {
1845+
if (!visitRecursivelyLifetimeEndingUses(res, noUsers, func,
1846+
skipVisitingPhis)) {
18391847
return false;
18401848
}
18411849
}
@@ -1851,7 +1859,8 @@ PartialApplyInst::visitOnStackLifetimeEnds(
18511859
&& "only meaningful for OSSA stack closures");
18521860
bool noUsers = true;
18531861

1854-
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func)) {
1862+
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func,
1863+
/*skipVisitingPhis*/ false)) {
18551864
return false;
18561865
}
18571866
return !noUsers;
@@ -1863,7 +1872,8 @@ visitNonEscapingLifetimeEnds(llvm::function_ref<bool (Operand *)> func) const {
18631872
&& "only meaningful for nonescaping dependencies");
18641873
bool noUsers = true;
18651874

1866-
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func)) {
1875+
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func,
1876+
/*skipVisitingPhis*/ true)) {
18671877
return false;
18681878
}
18691879
return !noUsers;

‎lib/SIL/Utils/OwnershipUtils.cpp

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,11 @@ bool swift::findInnerTransitiveGuaranteedUses(
331331
if (endUse->getOperandOwnership() == OperandOwnership::Reborrow) {
332332
foundPointerEscape = true;
333333
}
334+
if (PhiOperand(endUse)) {
335+
assert(endUse->getOperandOwnership() ==
336+
OperandOwnership::ForwardingConsume);
337+
foundPointerEscape = true;
338+
}
334339
leafUse(endUse);
335340
return true;
336341
})) {
@@ -385,11 +390,17 @@ bool swift::findExtendedUsesOfSimpleBorrowedValue(
385390
break;
386391

387392
case OperandOwnership::TrivialUse:
388-
case OperandOwnership::ForwardingConsume:
389393
case OperandOwnership::DestroyingConsume:
390394
recordUse(use);
391395
break;
392396

397+
case OperandOwnership::ForwardingConsume: {
398+
if (PhiOperand(use)) {
399+
return false;
400+
}
401+
recordUse(use);
402+
}
403+
393404
case OperandOwnership::ForwardingUnowned:
394405
case OperandOwnership::PointerEscape:
395406
case OperandOwnership::Reborrow:
@@ -468,6 +479,11 @@ bool swift::findUsesOfSimpleValue(SILValue value,
468479
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
469480
return false;
470481
}
482+
if (PhiOperand(use)) {
483+
assert(use->getOperandOwnership() ==
484+
OperandOwnership::ForwardingConsume);
485+
return false;
486+
}
471487
usePoints->push_back(end);
472488
return true;
473489
})) {
@@ -971,6 +987,9 @@ bool BorrowedValue::visitExtendedScopeEndingUses(
971987
reborrows.insert(borrowedValue.value);
972988
return true;
973989
}
990+
if (auto phiOp = PhiOperand(scopeEndingUse)) {
991+
return false;
992+
}
974993
return visitor(scopeEndingUse);
975994
};
976995

‎lib/SIL/Utils/PrunedLiveness.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,11 @@ PrunedLiveRange<LivenessWithDefs>::updateForBorrowingOperand(Operand *operand) {
238238
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
239239
return false;
240240
}
241+
if (PhiOperand(end)) {
242+
assert(end->getOperandOwnership() ==
243+
OperandOwnership::ForwardingConsume);
244+
return false;
245+
}
241246
updateForUse(end->getUser(), /*lifetimeEnding*/ false);
242247
return true;
243248
})) {

‎lib/SIL/Verifier/SILOwnershipVerifier.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,7 @@ bool SILValueOwnershipChecker::gatherNonGuaranteedUsers(
261261
guaranteedPhiVerifier.verifyReborrows(
262262
cast<BeginBorrowInst>(op->getUser()));
263263
}
264+
// TODO: Verify mark_dependence [nonescaping]'s transitive phi uses.
264265
}
265266

266267
return foundError;

‎lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,11 @@ bool Implementation::gatherUses(SILValue value) {
462462
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
463463
return false;
464464
}
465+
if (PhiOperand(end)) {
466+
assert(end->getOperandOwnership() ==
467+
OperandOwnership::ForwardingConsume);
468+
return false;
469+
}
465470
LLVM_DEBUG(llvm::dbgs() << " ++ Scope-ending use: ";
466471
end->getUser()->print(llvm::dbgs()));
467472
liveness.updateForUse(end->getUser(), *leafRange,

‎lib/SILOptimizer/Utils/LexicalDestroyFolding.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,11 @@ bool findBorroweeUsage(Context const &context, BorroweeUsage &usage) {
636636
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
637637
return false;
638638
}
639+
if (PhiOperand(end)) {
640+
assert(end->getOperandOwnership() ==
641+
OperandOwnership::ForwardingConsume);
642+
return false;
643+
}
639644
recordUse(end);
640645
return true;
641646
})) {

‎test/SIL/lifetime_dependence_buffer_view_test.swift

Lines changed: 32 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,10 @@
22
// RUN: -disable-experimental-parser-round-trip \
33
// RUN: -enable-experimental-feature NonescapableTypes \
44
// RUN: -enable-experimental-feature NoncopyableGenerics \
5-
// RUN: -enable-experimental-lifetime-dependence-inference \
65
// RUN: -disable-lifetime-dependence-diagnostics
76

87
// REQUIRES: asserts
98
// REQUIRES: swift_in_compiler
10-
// REQUIRES: noncopyable_generics
11-
12-
// FIXME(NCG): This requires nonescaping Optionals.
13-
// XFAIL: *
149

1510
// TODO: Use real Range
1611
public struct FakeRange<Bound> {
@@ -67,7 +62,7 @@ public struct BufferView<Element> : ~Escapable {
6762
private var baseAddress: UnsafeRawPointer { start._rawValue }
6863
// TODO: Enable diagnostics once this initializer's store to temporary is handled
6964
// CHECK: sil @$s31lifetime_dependence_scope_fixup10BufferViewV11baseAddress5count9dependsOnACyxGSVYls_Siqd__htclufC : $@convention(method) <Element><Owner> (UnsafeRawPointer, Int, @in_guaranteed Owner, @thin BufferView<Element>.Type) -> _scope(1) @owned BufferView<Element> {
70-
public init<Owner>(
65+
public init<Owner: ~Copyable & ~Escapable>(
7166
baseAddress: UnsafeRawPointer,
7267
count: Int,
7368
dependsOn owner: borrowing Owner
@@ -77,7 +72,7 @@ public struct BufferView<Element> : ~Escapable {
7772
)
7873
}
7974
// CHECK: sil hidden @$s31lifetime_dependence_scope_fixup10BufferViewV5start5count9dependsOnACyxGAA0eF5IndexVyxGYls_Siqd__htclufC : $@convention(method) <Element><Owner> (BufferViewIndex<Element>, Int, @in_guaranteed Owner, @thin BufferView<Element>.Type) -> _scope(1) @owned BufferView<Element> {
80-
init<Owner>(
75+
init<Owner: ~Copyable & ~Escapable>(
8176
start index: BufferViewIndex<Element>,
8277
count: Int,
8378
dependsOn owner: borrowing Owner
@@ -113,6 +108,11 @@ extension BufferView {
113108

114109
public var startIndex: Index { start }
115110
public var endIndex: Index { start.advanced(by: count) }
111+
112+
@inlinable @inline(__always)
113+
public func distance(from start: Index, to end: Index) -> Int {
114+
start.distance(to: end)
115+
}
116116

117117
public subscript(position: Index) -> Element {
118118
get {
@@ -134,30 +134,39 @@ extension BufferView {
134134
)
135135
}
136136
}
137+
138+
borrowing public func prefix(upTo index: BufferViewIndex<Element>) -> _borrow(self) Self {
139+
index == startIndex
140+
? Self(start: start, count: 0, dependsOn: copy self)
141+
: prefix(through: index.advanced(by: -1))
142+
}
143+
144+
borrowing public func prefix(through index: Index) -> _borrow(self) Self {
145+
let nc = distance(from: startIndex, to: index) &+ 1
146+
return Self(start: start, count: nc, dependsOn: copy self)
147+
}
148+
149+
consuming public func prefix(_ maxLength: Int) -> _consume(self) Self {
150+
precondition(maxLength >= 0, "Can't have a prefix of negative length.")
151+
let nc = maxLength < count ? maxLength : count
152+
return Self(start: start, count: nc, dependsOn: self)
153+
}
137154
}
138155

139-
extension Array {
140-
// var view: BufferView<Element> {
141-
// withUnsafeBufferPointer {
142-
// return BufferView(unsafeBuffer: $0, storage: self)
143-
// }
144-
// }
145-
// TODO: Implementation of getter should not need a temporary
146-
// rdar://123071321
147-
// CHECK: sil hidden @$sSa31lifetime_dependence_scope_fixupE4viewAA10BufferViewVyxGvg : $@convention(method) <Element> (@guaranteed Array<Element>) -> _scope(0) @owned BufferView<Element> {
148-
var view: BufferView<Element> {
149-
var _view : BufferView<Element>? // FIXME(NCG): This is not a thing. How did this work?
150-
withUnsafePointer(to:self) {
151-
_view = BufferView(baseAddress: $0, count: self.count, dependsOn: self)
156+
extension ContiguousArray {
157+
public var view: BufferView<Element> {
158+
borrowing _read {
159+
yield BufferView(
160+
baseAddress: _baseAddressIfContiguous!, count: count, dependsOn: self
161+
)
152162
}
153-
return _view!
154163
}
155164
}
156165

157-
public func array_view_element(a: [Int] , i: BufferViewIndex<Int>) -> Int {
166+
public func array_view_element(a: ContiguousArray<Int> , i: BufferViewIndex<Int>) -> Int {
158167
a.view[i]
159168
}
160169

161-
public func array_view_slice_element(a: [Int] , sliceIdx: FakeRange<BufferViewIndex<Int>>, Idx: BufferViewIndex<Int>) -> Int {
170+
public func array_view_slice_element(a: ContiguousArray<Int> , sliceIdx: FakeRange<BufferViewIndex<Int>>, Idx: BufferViewIndex<Int>) -> Int {
162171
a.view[sliceIdx][Idx]
163172
}

0 commit comments

Comments
 (0)