Skip to content

LifetimeDependence: handle dependent values in throwing calls. #72777

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
Apr 4, 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
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,8 @@ struct InstructionRange : CustomStringConvertible, NoReflectionChildren {
}

static func beginningInstruction(for value: Value) -> Instruction {
if let def = value.definingInstruction {
if let def = value.definingInstructionOrTerminator {
return def
} else if let result = TerminatorResult(value) {
return result.terminator
}
assert(Phi(value) != nil || value is FunctionArgument)
return value.parentBlock.instructions.first!
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,13 @@ extension LifetimeDependentApply {
/// dependent value within each scope.
private func insertDependencies(for apply: LifetimeDependentApply,
_ context: FunctionPassContext ) {
precondition(apply.applySite.results.count > 0,
"a lifetime-dependent instruction must have at least one result")

let bases = findDependenceBases(of: apply, context)
let builder = Builder(after: apply.applySite, context)
for dependentValue in apply.applySite.resultOrYields {
let builder = Builder(before: dependentValue.nextInstruction, context)
insertMarkDependencies(value: dependentValue, initializer: nil,
bases: bases, builder: builder, context)
}
let builder = Builder(after: apply.applySite, context)
for resultOper in apply.applySite.indirectResultOperands {
let accessBase = resultOper.value.accessBase
guard let (initialAddress, initializingStore) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ extension LifetimeDependence {
if value.isEscapable {
return nil
}
if (value.definingInstruction as! FullApplySite).hasResultDependence {
if (value.definingInstructionOrTerminator as! FullApplySite).hasResultDependence {
return nil
}
assert(value.ownership == .owned, "apply result must be owned")
Expand Down Expand Up @@ -766,11 +766,12 @@ extension LifetimeDependenceUseDefWalker {

/// Walk down dependent values.
///
/// Delegates value def-use walking to the ForwardingUseDefWalker and
/// overrides copy, move, borrow, and mark_dependence.
/// First classifies all values using OwnershipUseVisitor. Delegates forwarding uses to the ForwardingUseDefWalker.
/// Transitively follows OwnershipTransitionInstructions (copy, move, borrow, and mark_dependence). Transitively
/// follows interior pointers using AddressUseVisitor. Handles stores to and loads from local variables using
/// LocalVariableReachabilityCache.
///
/// Ignores trivial values (~Escapable types are never
/// trivial. Escapable types may only be lifetime-depenent values if
/// Ignores trivial values (~Escapable types are never trivial. Escapable types may only be lifetime-depenent values if
/// they are non-trivial).
///
/// Skips uses within nested borrow scopes.
Expand Down
11 changes: 7 additions & 4 deletions SwiftCompilerSources/Sources/SIL/ForwardingInstruction.swift
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,9 @@ extension Value {
// If this value is produced by a ForwardingInstruction, return that instruction. This is convenient for following the forwarded value chain.
// Unlike definingInstruction, a value's forwardingInstruction is not necessarily a valid insertion point.
public var forwardingInstruction: ForwardingInstruction? {
if let inst = definingInstruction {
if let inst = definingInstructionOrTerminator {
return inst as? ForwardingInstruction
}
if let termResult = TerminatorResult(self) {
return termResult.terminator as? ForwardingInstruction
}
return nil
}
}
Expand Down Expand Up @@ -319,6 +316,12 @@ extension UncheckedValueCastInst : ConversionInstruction {
public var canForwardOwnedValues: Bool { true }
}

extension DropDeinitInst : ConversionInstruction {
public var preservesRepresentation: Bool { true }
public var canForwardGuaranteedValues: Bool { false }
public var canForwardOwnedValues: Bool { true }
}

// -----------------------------------------------------------------------------
// other forwarding instructions

Expand Down
9 changes: 9 additions & 0 deletions SwiftCompilerSources/Sources/SIL/Value.swift
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,15 @@ extension Value {
}
}

public var definingInstructionOrTerminator: Instruction? {
if let def = definingInstruction {
return def
} else if let result = TerminatorResult(self) {
return result.terminator
}
return nil
}

public var nextInstruction: Instruction {
if self is Argument {
return parentBlock.instructions.first!
Expand Down
1 change: 1 addition & 0 deletions include/swift/SIL/InstWrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ struct ConversionOperation {
case SILInstructionKind::CopyableToMoveOnlyWrapperValueInst:
case SILInstructionKind::MoveOnlyWrapperToCopyableValueInst:
case SILInstructionKind::MoveOnlyWrapperToCopyableBoxInst:
case SILInstructionKind::DropDeinitInst:
return true;
default:
return false;
Expand Down
11 changes: 9 additions & 2 deletions include/swift/SIL/SILInstruction.h
Original file line number Diff line number Diff line change
Expand Up @@ -8679,6 +8679,11 @@ class DestroyValueInst
}

public:
/// True if this destroy fully deinitializes the type by invoking the
/// user-defined deinitializer if present. This returns false if a prior
/// drop_deinit is present.
bool isFullDeinitialization();

/// If true, then all references within the destroyed value will be
/// overwritten with a sentinel. This is used in debug builds when shortening
/// non-trivial value lifetimes to ensure the debugger cannot inspect invalid
Expand Down Expand Up @@ -8749,11 +8754,12 @@ class MoveValueInst
/// for details. See SILVerifier.cpp for constraints on valid uses.
class DropDeinitInst
: public UnaryInstructionBase<SILInstructionKind::DropDeinitInst,
SingleValueInstruction> {
OwnershipForwardingSingleValueInstruction> {
friend class SILBuilder;

DropDeinitInst(SILDebugLocation DebugLoc, SILValue operand)
: UnaryInstructionBase(DebugLoc, operand, operand->getType()) {}
: UnaryInstructionBase(DebugLoc, operand, operand->getType(),
OwnershipKind::Owned) {}
};

/// Equivalent to a copy_addr to [init] except that it is used for diagnostics
Expand Down Expand Up @@ -11134,6 +11140,7 @@ OwnershipForwardingSingleValueInstruction::classof(SILInstructionKind kind) {
case SILInstructionKind::ThinToThickFunctionInst:
case SILInstructionKind::UnconditionalCheckedCastInst:
case SILInstructionKind::FunctionExtractIsolationInst:
case SILInstructionKind::DropDeinitInst:
return true;
default:
return false;
Expand Down
4 changes: 4 additions & 0 deletions lib/SIL/IR/SILInstruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1885,6 +1885,10 @@ bool MarkDependenceInst::visitNonEscapingLifetimeEnds(
return !noUsers;
}

bool DestroyValueInst::isFullDeinitialization() {
return !isa<DropDeinitInst>(lookThroughOwnershipInsts(getOperand()));
}

PartialApplyInst *
DestroyValueInst::getNonescapingClosureAllocation() const {
SILValue operand = getOperand();
Expand Down
11 changes: 7 additions & 4 deletions lib/SILOptimizer/Utils/CanonicalizeInstruction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -547,10 +547,13 @@ static SILBasicBlock::iterator
eliminateUnneededForwardingUnarySingleValueInst(SingleValueInstruction *inst,
CanonicalizeInstruction &pass) {
auto next = std::next(inst->getIterator());

for (auto *use : getNonDebugUses(inst))
if (!isa<DestroyValueInst>(use->getUser()))
return next;
for (auto *use : getNonDebugUses(inst)) {
if (auto *destroy = dyn_cast<DestroyValueInst>(use->getUser())) {
if (destroy->isFullDeinitialization())
continue;
}
return next;
}
deleteAllDebugUses(inst, pass.callbacks);
SILValue op = inst->getOperand(0);
inst->replaceAllUsesWith(op);
Expand Down
16 changes: 16 additions & 0 deletions test/SILOptimizer/lifetime_dependence_diagnostics.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,3 +80,19 @@ func bv_borrow_borrow(bv: borrowing BV) -> dependsOn(scoped bv) BV {
func ncint_capture(ncInt: inout NCInt) {
takeClosure { _ = ncInt.i }
}

func neint_throws(ncInt: borrowing NCInt) throws -> NEInt {
return NEInt(owner: ncInt)
}

// CHECK-LABEL: sil hidden @$s4test9neint_try5ncIntAA5NEIntVAA5NCIntVYls_tKF : $@convention(thin) (@guaranteed NCInt) -> _scope(1) (@owned NEInt, @error any Error) {
// CHECK: try_apply %{{.*}}(%0) : $@convention(thin) (@guaranteed NCInt) -> _scope(1) (@owned NEInt, @error any Error), normal bb1, error bb2
// CHECK: bb1([[R:%.*]] : $NEInt):
// CHECK: [[MD:%.*]] = mark_dependence [nonescaping] %5 : $NEInt on %0 : $NCInt
// CHECK: return [[MD]] : $NEInt
// CHECK: bb2([[E:%.*]] : $any Error):
// CHECK: throw [[E]] : $any Error
// CHECK-LABEL: } // end sil function '$s4test9neint_try5ncIntAA5NEIntVAA5NCIntVYls_tKF'
func neint_try(ncInt: borrowing NCInt) throws -> NEInt {
try neint_throws(ncInt: ncInt)
}
23 changes: 22 additions & 1 deletion test/SILOptimizer/lifetime_dependence_util.sil
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,15 @@ sil_stage canonical

import Builtin

@_marker public protocol Escapable {}
@_marker protocol Copyable: ~Escapable {}
@_marker protocol Escapable: ~Copyable {}

struct NCNEInt: ~Copyable, ~Escapable {
var i: Builtin.Int64
@_unsafeNonescapableResult
init() {}
deinit {}
}

enum FakeOptional<T> {
case none
Expand Down Expand Up @@ -324,3 +332,16 @@ bb0(%0 : @owned $NE):
%99 = tuple()
return %99 : $()
}

// CHECK-LABEL: begin running test 1 of 1 on testInteriorDropDeinit: lifetime_dependence_use with: %0
// CHECK: LifetimeDependence uses of: %0 = argument of bb0 : $NCNEInt
// CHECK-NEXT: Leaf use: operand #0 of destroy_value
// CHECK-LABEL: end running test 1 of 1 on testInteriorDropDeinit: lifetime_dependence_use with: %0
sil [ossa] @testInteriorDropDeinit : $@convention(thin) (@owned NCNEInt) -> () {
bb0(%0 : @owned $NCNEInt):
specify_test "lifetime_dependence_use %0"
%nd = drop_deinit %0 : $NCNEInt
destroy_value %nd : $NCNEInt
%99 = tuple()
return %99 : $()
}
41 changes: 40 additions & 1 deletion test/SILOptimizer/ownership_liveness_unit.sil
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ sil_stage canonical

import Builtin

struct NCInt : ~Copyable {
var i: Builtin.Int64
deinit {}
}

enum FakeOptional<T> {
case none
case some(T)
Expand Down Expand Up @@ -243,7 +248,7 @@ exit(%reborrow : @guaranteed $C, %phi : @guaranteed $D):
}

// =============================================================================
// InteriorLiveness and visitAdjacentPhis
// InteriorLiveness
// =============================================================================

// CHECK-LABEL: testInteriorRefElementEscape: interior-liveness with: %0
Expand Down Expand Up @@ -312,6 +317,40 @@ bb0(%0 : $*D, %1 : @guaranteed $D):
return %99 : $()
}

// CHECK-LABEL: begin running test 1 of 2 on testInteriorDropDeinit: interior-liveness with: %0
// CHECK: Interior liveness: %0 = argument of bb0 : $NCInt
// CHECK-NEXT: bb0: LiveWithin
// CHECK-NEXT: lifetime-ending user: [[DD:%.*]] = drop_deinit %0 : $NCInt
// CHECK-NEXT: Complete liveness
// CHECK-NEXT: Unenclosed phis {
// CHECK-NEXT: }
// CHECK-NEXT: last user: [[DD]] = drop_deinit %0 : $NCInt
// CHECK-LABEL: end running test 1 of 2 on testInteriorDropDeinit: interior-liveness with: %0

// CHECK-LABEL: begin running test 2 of 2 on testInteriorDropDeinit: interior_liveness_swift with: %0
// CHECK: Interior liveness: %0 = argument of bb0 : $NCInt
// CHECK-NEXT: begin: [[DD:%.*]] = drop_deinit %0 : $NCInt
// CHECK-NEXT: ends: [[DD]] = drop_deinit %0 : $NCInt
// CHECK-NEXT: exits:
// CHECK-NEXT: interiors:
// CHECK-NEXT: Unenclosed phis {
// CHECK-NEXT: }
// CHECK-NEXT: last user: [[DD]] = drop_deinit %0 : $NCInt
// CHECK-LABEL: end running test 2 of 2 on testInteriorDropDeinit: interior_liveness_swift with: %0
sil [ossa] @testInteriorDropDeinit : $@convention(thin) (@owned NCInt) -> () {
bb0(%0 : @owned $NCInt):
specify_test "interior-liveness %0"
specify_test "interior_liveness_swift %0"
%nd = drop_deinit %0 : $NCInt
destroy_value %nd : $NCInt
%99 = tuple()
return %99 : $()
}

// =============================================================================
// InteriorLiveness and visitAdjacentPhis
// =============================================================================

// CHECK-LABEL: testInteriorReborrow: interior-liveness with: %borrow
// CHECK: Complete liveness
// CHECK-NEXT: Unenclosed phis {
Expand Down
13 changes: 13 additions & 0 deletions test/SILOptimizer/sil_combine_moveonly.sil
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,16 @@ bb0(%0 : $*T, %1 : $*Wrapper<T>):
%9 = tuple ()
return %9 : $()
}

// CHECK-LABEL: sil hidden [ossa] @testSDeinit : $@convention(method) (@owned S) -> () {
// CHECK: bb0(%0 : @owned $S):
// CHECK: [[DD:%.*]] = drop_deinit %0 : $S
// CHECK: destroy_value [[DD]] : $S
// CHECK-LABEL: } // end sil function 'testSDeinit'
sil hidden [ossa] @testSDeinit : $@convention(method) (@owned S) -> () {
bb0(%0 : @owned $S):
%1 = drop_deinit %0 : $S
destroy_value %1 : $S
%64 = tuple ()
return %64 : $()
}