Skip to content

Fix utilities that may see phis of mark_dependence [nonescaping] #72359

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions lib/SIL/IR/SILInstruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1792,11 +1792,10 @@ bool SILInstruction::maySuspend() const {
return false;
}

static bool visitRecursivelyLifetimeEndingUses(
SILValue i,
bool &noUsers,
llvm::function_ref<bool(Operand *)> func)
{
static bool
visitRecursivelyLifetimeEndingUses(SILValue i, bool &noUsers,
llvm::function_ref<bool(Operand *)> func,
bool allowPhis) {
for (Operand *use : i->getConsumingUses()) {
noUsers = false;
if (isa<DestroyValueInst>(use->getUser())) {
Expand All @@ -1813,6 +1812,14 @@ static bool visitRecursivelyLifetimeEndingUses(
}
continue;
}
if (allowPhis) {
if (PhiOperand(use)) {
if (!func(use)) {
return false;
}
continue;
}
}
// FIXME: Handle store to indirect result

// There shouldn't be any dead-end consumptions of a nonescaping
Expand All @@ -1835,7 +1842,8 @@ static bool visitRecursivelyLifetimeEndingUses(
"forwarded to a destroy_value");
}
for (auto res : use->getUser()->getResults()) {
if (!visitRecursivelyLifetimeEndingUses(res, noUsers, func)) {
if (!visitRecursivelyLifetimeEndingUses(res, noUsers, func,
allowPhis)) {
return false;
}
}
Expand All @@ -1851,7 +1859,8 @@ PartialApplyInst::visitOnStackLifetimeEnds(
&& "only meaningful for OSSA stack closures");
bool noUsers = true;

if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func)) {
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func,
/*allowPhis*/ false)) {
return false;
}
return !noUsers;
Expand All @@ -1863,7 +1872,8 @@ visitNonEscapingLifetimeEnds(llvm::function_ref<bool (Operand *)> func) const {
&& "only meaningful for nonescaping dependencies");
bool noUsers = true;

if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func)) {
if (!visitRecursivelyLifetimeEndingUses(this, noUsers, func,
/*allowPhis*/ true)) {
return false;
}
return !noUsers;
Expand Down
22 changes: 21 additions & 1 deletion lib/SIL/Utils/OwnershipUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,11 @@ bool swift::findInnerTransitiveGuaranteedUses(
if (endUse->getOperandOwnership() == OperandOwnership::Reborrow) {
foundPointerEscape = true;
}
if (PhiOperand(endUse)) {
assert(endUse->getOperandOwnership() ==
OperandOwnership::ForwardingConsume);
foundPointerEscape = true;
}
leafUse(endUse);
return true;
})) {
Expand Down Expand Up @@ -385,11 +390,18 @@ bool swift::findExtendedUsesOfSimpleBorrowedValue(
break;

case OperandOwnership::TrivialUse:
case OperandOwnership::ForwardingConsume:
case OperandOwnership::DestroyingConsume:
recordUse(use);
break;

case OperandOwnership::ForwardingConsume: {
if (PhiOperand(use)) {
return false;
}
recordUse(use);
break;
}

case OperandOwnership::ForwardingUnowned:
case OperandOwnership::PointerEscape:
case OperandOwnership::Reborrow:
Expand Down Expand Up @@ -468,6 +480,11 @@ bool swift::findUsesOfSimpleValue(SILValue value,
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
return false;
}
if (PhiOperand(use)) {
assert(use->getOperandOwnership() ==
OperandOwnership::ForwardingConsume);
return false;
}
usePoints->push_back(end);
return true;
})) {
Expand Down Expand Up @@ -971,6 +988,9 @@ bool BorrowedValue::visitExtendedScopeEndingUses(
reborrows.insert(borrowedValue.value);
return true;
}
if (auto phiOp = PhiOperand(scopeEndingUse)) {
return false;
}
return visitor(scopeEndingUse);
};

Expand Down
5 changes: 5 additions & 0 deletions lib/SIL/Utils/PrunedLiveness.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,11 @@ PrunedLiveRange<LivenessWithDefs>::updateForBorrowingOperand(Operand *operand) {
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
return false;
}
if (PhiOperand(end)) {
assert(end->getOperandOwnership() ==
OperandOwnership::ForwardingConsume);
return false;
}
updateForUse(end->getUser(), /*lifetimeEnding*/ false);
return true;
})) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,11 @@ bool Implementation::gatherUses(SILValue value) {
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
return false;
}
if (PhiOperand(end)) {
assert(end->getOperandOwnership() ==
OperandOwnership::ForwardingConsume);
return false;
}
LLVM_DEBUG(llvm::dbgs() << " ++ Scope-ending use: ";
end->getUser()->print(llvm::dbgs()));
liveness.updateForUse(end->getUser(), *leafRange,
Expand Down
5 changes: 5 additions & 0 deletions lib/SILOptimizer/Utils/LexicalDestroyFolding.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,11 @@ bool findBorroweeUsage(Context const &context, BorroweeUsage &usage) {
if (end->getOperandOwnership() == OperandOwnership::Reborrow) {
return false;
}
if (PhiOperand(end)) {
assert(end->getOperandOwnership() ==
OperandOwnership::ForwardingConsume);
return false;
}
recordUse(end);
return true;
})) {
Expand Down
55 changes: 32 additions & 23 deletions test/SIL/lifetime_dependence_buffer_view_test.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,10 @@
// RUN: -disable-experimental-parser-round-trip \
// RUN: -enable-experimental-feature NonescapableTypes \
// RUN: -enable-experimental-feature NoncopyableGenerics \
// RUN: -enable-experimental-lifetime-dependence-inference \
// RUN: -disable-lifetime-dependence-diagnostics

// REQUIRES: asserts
// REQUIRES: swift_in_compiler
// REQUIRES: noncopyable_generics

// FIXME(NCG): This requires nonescaping Optionals.
// XFAIL: *

// TODO: Use real Range
public struct FakeRange<Bound> {
Expand Down Expand Up @@ -67,7 +62,7 @@ public struct BufferView<Element> : ~Escapable {
private var baseAddress: UnsafeRawPointer { start._rawValue }
// TODO: Enable diagnostics once this initializer's store to temporary is handled
// 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> {
public init<Owner>(
public init<Owner: ~Copyable & ~Escapable>(
baseAddress: UnsafeRawPointer,
count: Int,
dependsOn owner: borrowing Owner
Expand All @@ -77,7 +72,7 @@ public struct BufferView<Element> : ~Escapable {
)
}
// 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> {
init<Owner>(
init<Owner: ~Copyable & ~Escapable>(
start index: BufferViewIndex<Element>,
count: Int,
dependsOn owner: borrowing Owner
Expand Down Expand Up @@ -113,6 +108,11 @@ extension BufferView {

public var startIndex: Index { start }
public var endIndex: Index { start.advanced(by: count) }

@inlinable @inline(__always)
public func distance(from start: Index, to end: Index) -> Int {
start.distance(to: end)
}

public subscript(position: Index) -> Element {
get {
Expand All @@ -134,30 +134,39 @@ extension BufferView {
)
}
}

borrowing public func prefix(upTo index: BufferViewIndex<Element>) -> _borrow(self) Self {
index == startIndex
? Self(start: start, count: 0, dependsOn: copy self)
: prefix(through: index.advanced(by: -1))
}

borrowing public func prefix(through index: Index) -> _borrow(self) Self {
let nc = distance(from: startIndex, to: index) &+ 1
return Self(start: start, count: nc, dependsOn: copy self)
}

consuming public func prefix(_ maxLength: Int) -> _consume(self) Self {
precondition(maxLength >= 0, "Can't have a prefix of negative length.")
let nc = maxLength < count ? maxLength : count
return Self(start: start, count: nc, dependsOn: self)
}
}

extension Array {
// var view: BufferView<Element> {
// withUnsafeBufferPointer {
// return BufferView(unsafeBuffer: $0, storage: self)
// }
// }
// TODO: Implementation of getter should not need a temporary
// rdar://123071321
// CHECK: sil hidden @$sSa31lifetime_dependence_scope_fixupE4viewAA10BufferViewVyxGvg : $@convention(method) <Element> (@guaranteed Array<Element>) -> _scope(0) @owned BufferView<Element> {
var view: BufferView<Element> {
var _view : BufferView<Element>? // FIXME(NCG): This is not a thing. How did this work?
withUnsafePointer(to:self) {
_view = BufferView(baseAddress: $0, count: self.count, dependsOn: self)
extension ContiguousArray {
public var view: BufferView<Element> {
borrowing _read {
yield BufferView(
baseAddress: _baseAddressIfContiguous!, count: count, dependsOn: self
)
}
return _view!
}
}

public func array_view_element(a: [Int] , i: BufferViewIndex<Int>) -> Int {
public func array_view_element(a: ContiguousArray<Int> , i: BufferViewIndex<Int>) -> Int {
a.view[i]
}

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