Skip to content

LifetimeDependenceDiagnostics; remove a bootstrapping hack. #78599

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 4 commits into from
Jan 14, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -199,21 +199,6 @@ private struct DiagnoseDependence {
if function.hasUnsafeNonEscapableResult {
return .continueWalk
}
// FIXME: remove this condition once we have a Builtin.dependence,
// which developers should use to model the unsafe
// dependence. Builtin.lifetime_dependence will be lowered to
// mark_dependence [unresolved], which will be checked
// independently. Instead, of this function result check, allow
// isUnsafeApplyResult to be used be mark_dependence [unresolved]
// without checking its dependents.
//
// Allow returning an apply result (@_unsafeNonescapableResult) if
// the calling function has a dependence. This implicitly makes
// the unsafe nonescapable result dependent on the calling
// function's lifetime dependence arguments.
if dependence.isUnsafeApplyResult, function.hasResultDependence {
return .continueWalk
}
// Check that the parameter dependence for this result is the same
// as the current dependence scope.
if let arg = dependence.scope.parentValue as? FunctionArgument,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -225,16 +225,6 @@ extension LifetimeDependence {
self.dependentValue = value
}

var isUnsafeApplyResult: Bool {
if case let .owned(value) = scope {
if let apply = value.definingInstruction as? FullApplySite {
assert(!apply.hasResultDependence)
return true
}
}
return false
}

/// Construct LifetimeDependence from mark_dependence [unresolved] or mark_dependence [nonescaping].
///
/// For any LifetimeDependence constructed from a mark_dependence, its `dependentValue` will be the result of the
Expand Down
62 changes: 37 additions & 25 deletions lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -91,23 +91,28 @@ getFullyReferenceableStruct(SILType Ty) {
return SD;
}

static unsigned getNumSubElements(SILType T, SILModule &M,
static unsigned getNumSubElements(SILType T, SILFunction &F,
TypeExpansionContext context) {

if (auto TT = T.getAs<TupleType>()) {
unsigned NumElements = 0;
for (auto index : indices(TT.getElementTypes()))
NumElements +=
getNumSubElements(T.getTupleElementType(index), M, context);
getNumSubElements(T.getTupleElementType(index), F, context);
return NumElements;
}

if (auto *SD = getFullyReferenceableStruct(T)) {
unsigned NumElements = 0;
for (auto *D : SD->getStoredProperties())
NumElements +=
getNumSubElements(T.getFieldType(D, M, context), M, context);
return NumElements;
getNumSubElements(T.getFieldType(D, F.getModule(), context), F,
context);

// Returning NumElements == 0 implies that "empty" values can be
// materialized without ownership. This is only valid for trivial types.
if (NumElements > 0 || T.isTrivial(F))
return NumElements;
}

// If this isn't a tuple or struct, it is a single element.
Expand Down Expand Up @@ -152,7 +157,7 @@ static SILValue getAccessPathRoot(SILValue pointer) {
static unsigned computeSubelement(SILValue Pointer,
SingleValueInstruction *RootInst) {
unsigned SubElementNumber = 0;
SILModule &M = RootInst->getModule();
auto &F = *RootInst->getFunction();

while (1) {
// If we got to the root, we're done.
Expand All @@ -175,7 +180,7 @@ static unsigned computeSubelement(SILValue Pointer,
// Keep track of what subelement is being referenced.
for (unsigned i = 0, e = TEAI->getFieldIndex(); i != e; ++i) {
SubElementNumber +=
getNumSubElements(TT.getTupleElementType(i), M,
getNumSubElements(TT.getTupleElementType(i), F,
TypeExpansionContext(*RootInst->getFunction()));
}
Pointer = TEAI->getOperand();
Expand All @@ -191,7 +196,8 @@ static unsigned computeSubelement(SILValue Pointer,
if (D == SEAI->getField()) break;
auto context = TypeExpansionContext(*RootInst->getFunction());
SubElementNumber +=
getNumSubElements(ST.getFieldType(D, M, context), M, context);
getNumSubElements(ST.getFieldType(D, F.getModule(), context), F,
context);
}

Pointer = SEAI->getOperand();
Expand Down Expand Up @@ -370,10 +376,9 @@ static bool isFullyAvailable(SILType loadTy, unsigned firstElt,
if (!firstVal || firstVal.getType() != loadTy)
return false;

auto *function = firstVal.getValue()->getFunction();
auto &function = *firstVal.getValue()->getFunction();
return llvm::all_of(
range(getNumSubElements(loadTy, function->getModule(),
TypeExpansionContext(*function))),
range(getNumSubElements(loadTy, function, TypeExpansionContext(function))),
[&](unsigned index) -> bool {
auto &val = AvailableValues[firstElt + index];
return val.getValue() == firstVal.getValue() &&
Expand All @@ -395,7 +400,7 @@ static SILValue nonDestructivelyExtractSubElement(const AvailableValue &Val,
// Keep track of what subelement is being referenced.
SILType EltTy = ValTy.getTupleElementType(EltNo);
unsigned NumSubElt = getNumSubElements(
EltTy, B.getModule(), TypeExpansionContext(B.getFunction()));
EltTy, B.getFunction(), TypeExpansionContext(B.getFunction()));
if (SubElementNumber < NumSubElt) {
auto BorrowedVal = Val.emitBeginBorrow(B, Loc);
auto NewVal =
Expand All @@ -420,7 +425,7 @@ static SILValue nonDestructivelyExtractSubElement(const AvailableValue &Val,
auto fieldType = ValTy.getFieldType(
D, B.getModule(), TypeExpansionContext(B.getFunction()));
unsigned NumSubElt = getNumSubElements(
fieldType, B.getModule(), TypeExpansionContext(B.getFunction()));
fieldType, B.getFunction(), TypeExpansionContext(B.getFunction()));

if (SubElementNumber < NumSubElt) {
auto BorrowedVal = Val.emitBeginBorrow(B, Loc);
Expand Down Expand Up @@ -1227,7 +1232,8 @@ bool AvailableValueAggregator::canTake(SILType loadTy,
return llvm::all_of(indices(tt->getElements()), [&](unsigned eltNo) {
SILType eltTy = loadTy.getTupleElementType(eltNo);
unsigned numSubElt =
getNumSubElements(eltTy, M, TypeExpansionContext(B.getFunction()));
getNumSubElements(eltTy, B.getFunction(),
TypeExpansionContext(B.getFunction()));
bool success = canTake(eltTy, firstElt);
firstElt += numSubElt;
return success;
Expand All @@ -1238,7 +1244,7 @@ bool AvailableValueAggregator::canTake(SILType loadTy,
return llvm::all_of(sd->getStoredProperties(), [&](VarDecl *decl) -> bool {
auto context = TypeExpansionContext(B.getFunction());
SILType eltTy = loadTy.getFieldType(decl, M, context);
unsigned numSubElt = getNumSubElements(eltTy, M, context);
unsigned numSubElt = getNumSubElements(eltTy, B.getFunction(), context);
bool success = canTake(eltTy, firstElt);
firstElt += numSubElt;
return success;
Expand Down Expand Up @@ -1369,7 +1375,8 @@ SILValue AvailableValueAggregator::aggregateTupleSubElts(TupleType *TT,
for (unsigned EltNo : indices(TT->getElements())) {
SILType EltTy = LoadTy.getTupleElementType(EltNo);
unsigned NumSubElt =
getNumSubElements(EltTy, M, TypeExpansionContext(B.getFunction()));
getNumSubElements(EltTy, B.getFunction(),
TypeExpansionContext(B.getFunction()));

// If we are missing any of the available values in this struct element,
// compute an address to load from.
Expand Down Expand Up @@ -1406,7 +1413,7 @@ SILValue AvailableValueAggregator::aggregateStructSubElts(StructDecl *sd,
for (auto *decl : sd->getStoredProperties()) {
auto context = TypeExpansionContext(B.getFunction());
SILType eltTy = loadTy.getFieldType(decl, M, context);
unsigned numSubElt = getNumSubElements(eltTy, M, context);
unsigned numSubElt = getNumSubElements(eltTy, B.getFunction(), context);

// If we are missing any of the available values in this struct element,
// compute an address to load from.
Expand Down Expand Up @@ -1540,6 +1547,8 @@ class AvailableValueDataflowContext {
private:
SILModule &getModule() const { return TheMemory->getModule(); }

SILFunction &getFunction() const { return *TheMemory->getFunction(); }

void updateAvailableValues(
SILInstruction *Inst,
SmallBitVector &RequiredElts,
Expand Down Expand Up @@ -1673,7 +1682,7 @@ AvailableValueDataflowContext::computeAvailableValues(
return std::nullopt;

unsigned NumLoadSubElements = getNumSubElements(
LoadTy, getModule(), TypeExpansionContext(*TheMemory->getFunction()));
LoadTy, getFunction(), TypeExpansionContext(getFunction()));

LoadInfo loadInfo = {LoadTy, FirstElt, NumLoadSubElements};

Expand Down Expand Up @@ -1712,14 +1721,14 @@ static inline void updateAvailableValuesHelper(
SmallBitVector &conflictingValues,
function_ref<std::optional<AvailableValue>(unsigned)> defaultFunc,
function_ref<bool(AvailableValue &, unsigned)> isSafeFunc) {
auto &mod = theMemory->getModule();
unsigned startSubElt = computeSubelement(address, theMemory);

// TODO: Is this needed now?
assert(startSubElt != ~0U && "Store within enum projection not handled");
auto &f = *theMemory->getFunction();
for (unsigned i : range(getNumSubElements(
address->getType().getObjectType(), mod,
TypeExpansionContext(*theMemory->getFunction())))) {
address->getType().getObjectType(), f,
TypeExpansionContext(f)))) {
// If this element is not required, don't fill it in.
if (!requiredElts[startSubElt + i])
continue;
Expand Down Expand Up @@ -1849,7 +1858,8 @@ void AvailableValueDataflowContext::updateAvailableValues(

bool AnyRequired = false;
for (unsigned i : range(getNumSubElements(
ValTy, getModule(), TypeExpansionContext(*CAI->getFunction())))) {
ValTy, getFunction(),
TypeExpansionContext(getFunction())))) {
// If this element is not required, don't fill it in.
AnyRequired = RequiredElts[StartSubElt+i];
if (AnyRequired) break;
Expand Down Expand Up @@ -1883,7 +1893,7 @@ void AvailableValueDataflowContext::updateAvailableValues(
// potentially bailing out (because it is address-only).
bool AnyRequired = false;
for (unsigned i : range(getNumSubElements(
ValTy, getModule(), TypeExpansionContext(*MD->getFunction())))) {
ValTy, getFunction(), TypeExpansionContext(getFunction())))) {
// If this element is not required, don't fill it in.
AnyRequired = RequiredElts[StartSubElt+i];
if (AnyRequired) break;
Expand Down Expand Up @@ -2171,7 +2181,7 @@ void AvailableValueDataflowContext::updateMarkDependenceValues(
}
SILType valueTy = md->getValue()->getType().getObjectType();
unsigned numMDSubElements = getNumSubElements(
valueTy, getModule(), TypeExpansionContext(*TheMemory->getFunction()));
valueTy, getFunction(), TypeExpansionContext(getFunction()));

// Update each required subelement of the mark_dependence value.
for (unsigned subIdx = firstMDElt; subIdx < firstMDElt + numMDSubElements;
Expand Down Expand Up @@ -2299,7 +2309,8 @@ class OptimizeAllocLoads {
: Module(memory->getModule()), TheMemory(memory),
MemoryType(getMemoryType(memory)),
NumMemorySubElements(getNumSubElements(
MemoryType, Module, TypeExpansionContext(*memory->getFunction()))),
MemoryType, *memory->getFunction(),
TypeExpansionContext(*memory->getFunction()))),
Uses(uses), deleter(deleter), deadEndBlocks(deadEndBlocks),
DataflowContext(TheMemory, NumMemorySubElements,
OptimizationMode::PreserveAlloc, uses,
Expand Down Expand Up @@ -2680,7 +2691,8 @@ class OptimizeDeadAlloc {
: Module(memory->getModule()), TheMemory(memory),
MemoryType(getMemoryType(memory)),
NumMemorySubElements(getNumSubElements(
MemoryType, Module, TypeExpansionContext(*memory->getFunction()))),
MemoryType, *memory->getFunction(),
TypeExpansionContext(*memory->getFunction()))),
Uses(uses), Releases(releases), deadEndBlocks(deadEndBlocks),
deleter(deleter), domInfo(domInfo),
DataflowContext(TheMemory, NumMemorySubElements,
Expand Down
36 changes: 31 additions & 5 deletions test/SIL/explicit_lifetime_dependence_specifiers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,30 @@

import Builtin

@_unsafeNonescapableResult
@lifetime(borrow source)
internal func _overrideLifetime<
T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable
>(
_ dependent: consuming T, borrowing source: borrowing U
) -> T {
// TODO: Remove @_unsafeNonescapableResult. Instead, the unsafe dependence
// should be expressed by a builtin that is hidden within the function body.
dependent
}

@_unsafeNonescapableResult
@lifetime(source)
internal func _overrideLifetime<
T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable
>(
_ dependent: consuming T, copying source: borrowing U
) -> T {
// TODO: Remove @_unsafeNonescapableResult. Instead, the unsafe dependence
// should be expressed by a builtin that is hidden within the function body.
dependent
}

struct BufferView : ~Escapable {
let ptr: UnsafeRawBufferPointer
// CHECK-LABEL: sil hidden @$s39explicit_lifetime_dependence_specifiers10BufferViewVyACSWcfC : $@convention(method) (UnsafeRawBufferPointer, @thin BufferView.Type) -> @lifetime(borrow 0) @owned BufferView {
Expand All @@ -22,7 +46,7 @@ struct BufferView : ~Escapable {
}
self.ptr = ptr
}
@_unsafeNonescapableResult
@lifetime(borrow ptr)
init(independent ptr: UnsafeRawBufferPointer) {
self.ptr = ptr
}
Expand Down Expand Up @@ -71,7 +95,8 @@ func derive(_ x: borrowing BufferView) -> BufferView {
// CHECK-LABEL: sil hidden @$s39explicit_lifetime_dependence_specifiers16consumeAndCreateyAA10BufferViewVADnF : $@convention(thin) (@owned BufferView) -> @lifetime(copy 0) @owned BufferView {
@lifetime(x)
func consumeAndCreate(_ x: consuming BufferView) -> BufferView {
return BufferView(independent: x.ptr)
let bv = BufferView(independent: x.ptr)
return _overrideLifetime(bv, copying: x)
}

// CHECK-LABEL: sil hidden @$s39explicit_lifetime_dependence_specifiers17deriveThisOrThat1yAA10BufferViewVAD_ADtF : $@convention(thin) (@guaranteed BufferView, @guaranteed BufferView) -> @lifetime(copy 1, borrow 0) @owned BufferView {
Expand All @@ -89,7 +114,8 @@ func deriveThisOrThat2(_ this: borrowing BufferView, _ that: consuming BufferVie
if (Int.random(in: 1..<100) == 0) {
return BufferView(independent: this.ptr)
}
return BufferView(independent: that.ptr)
let bv = BufferView(independent: that.ptr)
return _overrideLifetime(bv, copying: that)
}

func use(_ x: borrowing BufferView) {}
Expand Down Expand Up @@ -123,12 +149,12 @@ struct Container : ~Escapable {
// CHECK-LABEL: sil hidden @$s39explicit_lifetime_dependence_specifiers16getConsumingViewyAA06BufferG0VAA9ContainerVnF : $@convention(thin) (@owned Container) -> @lifetime(copy 0) @owned BufferView {
@lifetime(x)
func getConsumingView(_ x: consuming Container) -> BufferView {
return BufferView(independent: x.ptr)
let bv = BufferView(independent: x.ptr)
return _overrideLifetime(bv, copying: x)
}

// CHECK-LABEL: sil hidden @$s39explicit_lifetime_dependence_specifiers16getBorrowingViewyAA06BufferG0VAA9ContainerVF : $@convention(thin) (@guaranteed Container) -> @lifetime(borrow 0) @owned BufferView {
@lifetime(borrow x)
func getBorrowingView(_ x: borrowing Container) -> BufferView {
return BufferView(independent: x.ptr)
}

29 changes: 23 additions & 6 deletions test/SIL/implicit_lifetime_dependence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,27 @@

// REQUIRES: swift_feature_LifetimeDependence

@_unsafeNonescapableResult
@lifetime(borrow source)
internal func _overrideLifetime<
T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable
>(
_ dependent: consuming T, borrowing source: borrowing U
) -> T {
// TODO: Remove @_unsafeNonescapableResult. Instead, the unsafe dependence
// should be expressed by a builtin that is hidden within the function body.
dependent
}

@_unsafeNonescapableResult
@lifetime(source)
func unsafeLifetime<T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable>(
dependent: consuming T, dependsOn source: borrowing U)
-> T {
internal func _overrideLifetime<
T: ~Copyable & ~Escapable, U: ~Copyable & ~Escapable
>(
_ dependent: consuming T, copying source: borrowing U
) -> T {
// TODO: Remove @_unsafeNonescapableResult. Instead, the unsafe dependence
// should be expressed by a builtin that is hidden within the function body.
dependent
}

Expand All @@ -21,7 +37,7 @@ struct BufferView : ~Escapable {
self.ptr = ptr
self.c = c
}
@_unsafeNonescapableResult
@lifetime(borrow ptr)
init(independent ptr: UnsafeRawBufferPointer, _ c: Int) {
self.ptr = ptr
self.c = c
Expand Down Expand Up @@ -74,7 +90,8 @@ func derive(_ unused: Int, _ x: borrowing BufferView) -> BufferView {

// CHECK-LABEL: sil hidden @$s28implicit_lifetime_dependence16consumeAndCreateyAA10BufferViewVADnF : $@convention(thin) (@owned BufferView) -> @lifetime(copy 0) @owned BufferView {
func consumeAndCreate(_ x: consuming BufferView) -> BufferView {
return BufferView(independent: x.ptr, x.c)
let bv = BufferView(independent: x.ptr, x.c)
return _overrideLifetime(bv, copying: x)
}

func use(_ x: borrowing BufferView) {}
Expand Down Expand Up @@ -175,7 +192,7 @@ struct GenericBufferView<Element> : ~Escapable {
count: bounds.upperBound.distance(to:bounds.lowerBound) / MemoryLayout<Element>.stride
)
// assuming that bounds is within self
return unsafeLifetime(dependent: result, dependsOn: self)
return _overrideLifetime(result, copying: self)
}
}
}
Expand Down
Loading