diff --git a/include/swift/SIL/OwnershipUseVisitor.h b/include/swift/SIL/OwnershipUseVisitor.h index 856e88ae9f12a..56fe705fba4a4 100644 --- a/include/swift/SIL/OwnershipUseVisitor.h +++ b/include/swift/SIL/OwnershipUseVisitor.h @@ -283,11 +283,23 @@ bool OwnershipUseVisitor::visitInnerBorrowScopeEnd(Operand *borrowEnd) { case OperandOwnership::EndBorrow: return handleUsePoint(borrowEnd, UseLifetimeConstraint::NonLifetimeEnding); - case OperandOwnership::Reborrow: + case OperandOwnership::Reborrow: { if (!asImpl().handleInnerReborrow(borrowEnd)) return false; return handleUsePoint(borrowEnd, UseLifetimeConstraint::NonLifetimeEnding); + } + case OperandOwnership::DestroyingConsume: { + // partial_apply [on_stack] can introduce borrowing operand and can have + // destroy_value consumes. + auto *pai = dyn_cast(borrowEnd->get()); + // TODO: When we have ForwardingInstruction abstraction, walk the use-def + // chain to ensure we have a partial_apply [on_stack] def. + assert(pai && pai->isOnStack() || + OwnershipForwardingMixin::get( + cast(borrowEnd->get()))); + return handleUsePoint(borrowEnd, UseLifetimeConstraint::NonLifetimeEnding); + } default: llvm_unreachable("expected borrow scope end"); @@ -388,8 +400,9 @@ bool OwnershipUseVisitor::visitGuaranteedUse(Operand *use) { case OperandOwnership::ForwardingUnowned: case OperandOwnership::UnownedInstantaneousUse: case OperandOwnership::BitwiseEscape: - case OperandOwnership::EndBorrow: return handleUsePoint(use, UseLifetimeConstraint::NonLifetimeEnding); + case OperandOwnership::EndBorrow: + return handleUsePoint(use, UseLifetimeConstraint::LifetimeEnding); case OperandOwnership::Reborrow: if (!asImpl().handleOuterReborrow(use)) diff --git a/include/swift/SIL/OwnershipUtils.h b/include/swift/SIL/OwnershipUtils.h index e037973b0f62f..241b3257d1a4e 100644 --- a/include/swift/SIL/OwnershipUtils.h +++ b/include/swift/SIL/OwnershipUtils.h @@ -477,6 +477,9 @@ struct BorrowingOperand { /// valid BorrowedValue instance. BorrowedValue getBorrowIntroducingUserResult(); + /// Return the borrowing operand's value. + SILValue getScopeIntroducingUserResult(); + /// Compute the implicit uses that this borrowing operand "injects" into the /// set of its operands uses. /// diff --git a/include/swift/SIL/SILInstruction.h b/include/swift/SIL/SILInstruction.h index b9e67d5265a87..0773c2a57832b 100644 --- a/include/swift/SIL/SILInstruction.h +++ b/include/swift/SIL/SILInstruction.h @@ -1261,6 +1261,11 @@ class OwnershipForwardingMixin { /// /// \p inst is an OwnershipForwardingMixin static bool isAddressOnly(SILInstruction *inst); + + // Call \p visitor on all forwarded results of the current forwarding + // operation. + static bool visitForwardedValues(SILInstruction *inst, + function_ref visitor); }; /// A single value inst that forwards a static ownership from its first operand. diff --git a/lib/SIL/IR/SILInstructions.cpp b/lib/SIL/IR/SILInstructions.cpp index 97335283b86f5..ca254b68a7957 100644 --- a/lib/SIL/IR/SILInstructions.cpp +++ b/lib/SIL/IR/SILInstructions.cpp @@ -3276,6 +3276,31 @@ bool OwnershipForwardingMixin::isAddressOnly(SILInstruction *inst) { return inst->getOperand(0)->getType().isAddressOnly(*inst->getFunction()); } +bool OwnershipForwardingMixin::visitForwardedValues( + SILInstruction *inst, function_ref visitor) { + if (auto *svi = dyn_cast(inst)) { + return visitor(svi); + } + if (auto *mvri = dyn_cast(inst)) { + return llvm::all_of(mvri->getResults(), [&](SILValue value) { + if (value->getOwnershipKind() == OwnershipKind::None) + return true; + return visitor(value); + }); + } + auto *ti = cast(inst); + assert(ti->mayHaveTerminatorResult()); + return llvm::all_of(ti->getSuccessorBlocks(), [&](SILBasicBlock *succBlock) { + // If we do not have any arguments, then continue. + if (succBlock->args_empty()) + return true; + + auto args = succBlock->getSILPhiArguments(); + assert(args.size() == 1 && "Transforming terminator with multiple args?!"); + return visitor(args[0]); + }); +} + // This may be called in an invalid SIL state. SILCombine creates new // terminators in non-terminator position and defers deleting the original // terminator until after all modification. diff --git a/lib/SIL/Utils/OSSALifetimeCompletion.cpp b/lib/SIL/Utils/OSSALifetimeCompletion.cpp index 462e9a913585e..cf3426a672c90 100644 --- a/lib/SIL/Utils/OSSALifetimeCompletion.cpp +++ b/lib/SIL/Utils/OSSALifetimeCompletion.cpp @@ -129,7 +129,7 @@ static bool endLifetimeAtUnreachableBlocks(SILValue value, changed = true; } for (auto *successor : block->getSuccessorBlocks()) { - deadEndBlocks.push(successor); + deadEndBlocks.pushIfNotVisited(successor); } } return changed; diff --git a/lib/SIL/Utils/OwnershipLiveness.cpp b/lib/SIL/Utils/OwnershipLiveness.cpp index 51cb6cde26a21..bf4717cfc36a4 100644 --- a/lib/SIL/Utils/OwnershipLiveness.cpp +++ b/lib/SIL/Utils/OwnershipLiveness.cpp @@ -140,8 +140,10 @@ struct InteriorLivenessVisitor : /// Handles begin_borrow, load_borrow, store_borrow, begin_apply. bool handleInnerBorrow(BorrowingOperand borrowingOperand) { if (handleInnerScopeCallback) { - handleInnerScopeCallback( - borrowingOperand.getBorrowIntroducingUserResult().value); + auto value = borrowingOperand.getScopeIntroducingUserResult(); + if (value) { + handleInnerScopeCallback(value); + } } return true; } diff --git a/lib/SIL/Utils/OwnershipUtils.cpp b/lib/SIL/Utils/OwnershipUtils.cpp index b54166270ad3d..88fcfebcb30c5 100644 --- a/lib/SIL/Utils/OwnershipUtils.cpp +++ b/lib/SIL/Utils/OwnershipUtils.cpp @@ -751,6 +751,30 @@ BorrowedValue BorrowingOperand::getBorrowIntroducingUserResult() { llvm_unreachable("covered switch"); } +SILValue BorrowingOperand::getScopeIntroducingUserResult() { + switch (kind) { + case BorrowingOperandKind::Invalid: + case BorrowingOperandKind::Yield: + case BorrowingOperandKind::Apply: + case BorrowingOperandKind::TryApply: + return SILValue(); + + case BorrowingOperandKind::BeginAsyncLet: + case BorrowingOperandKind::PartialApplyStack: + case BorrowingOperandKind::BeginBorrow: + return cast(op->getUser()); + + case BorrowingOperandKind::BeginApply: + return cast(op->getUser())->getTokenResult(); + + case BorrowingOperandKind::Branch: { + PhiOperand phiOp(op); + return phiOp.getValue(); + } + } + llvm_unreachable("covered switch"); +} + void BorrowingOperand::getImplicitUses( SmallVectorImpl &foundUses) const { // FIXME: this visitScopeEndingUses should never return false once dead diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp index 3d3286b2f2e6c..d1a2084639e20 100644 --- a/lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp +++ b/lib/SILOptimizer/Mandatory/MoveOnlyChecker.cpp @@ -30,6 +30,7 @@ #include "swift/SIL/FieldSensitivePrunedLiveness.h" #include "swift/SIL/InstructionUtils.h" #include "swift/SIL/MemAccessUtils.h" +#include "swift/SIL/OSSALifetimeCompletion.h" #include "swift/SIL/OwnershipUtils.h" #include "swift/SIL/PrunedLiveness.h" #include "swift/SIL/SILArgument.h" @@ -86,6 +87,7 @@ struct MoveOnlyChecker { } void checkObjects(); + void completeObjectLifetimes(ArrayRef); void checkAddresses(); }; @@ -109,10 +111,75 @@ void MoveOnlyChecker::checkObjects() { return; } + completeObjectLifetimes(moveIntroducersToProcess.getArrayRef()); + MoveOnlyObjectChecker checker{diagnosticEmitter, domTree, poa, allocator}; madeChange |= checker.check(moveIntroducersToProcess); } +void MoveOnlyChecker::completeObjectLifetimes( + ArrayRef insts) { + // TODO: Delete once OSSALifetimeCompletion is run as part of SILGenCleanup. + OSSALifetimeCompletion completion(fn, domTree); + + // Collect all values derived from each mark_unresolved_non_copyable_value + // instruction via ownership instructions and phis. + ValueWorklist transitiveValues(fn); + for (auto *inst : insts) { + transitiveValues.push(inst); + } + while (auto value = transitiveValues.pop()) { + for (auto *use : value->getUses()) { + auto *user = use->getUser(); + switch (user->getKind()) { + case SILInstructionKind::BeginBorrowInst: + case SILInstructionKind::CopyValueInst: + case SILInstructionKind::MoveValueInst: + transitiveValues.pushIfNotVisited(cast(user)); + break; + case SILInstructionKind::BranchInst: { + PhiOperand po(use); + transitiveValues.pushIfNotVisited(po.getValue()); + break; + } + default: { + auto *forward = OwnershipForwardingMixin::get(user); + if (!forward) + continue; + OwnershipForwardingMixin::visitForwardedValues( + user, [&transitiveValues](auto forwarded) { + transitiveValues.pushIfNotVisited(forwarded); + return true; + }); + break; + } + } + } + } + // Complete the lifetime of each collected value. This is a subset of the + // work that SILGenCleanup will do. + for (auto *block : poa->get(fn)->getPostOrder()) { + for (SILInstruction &inst : reverse(*block)) { + for (auto result : inst.getResults()) { + if (!transitiveValues.isVisited(result)) + continue; + if (completion.completeOSSALifetime(result) == + LifetimeCompletion::WasCompleted) { + madeChange = true; + } + } + } + for (SILArgument *arg : block->getArguments()) { + if (!transitiveValues.isVisited(arg)) + continue; + if (completion.completeOSSALifetime(arg) == + LifetimeCompletion::WasCompleted) { + madeChange = true; + } + } + } +} + void MoveOnlyChecker::checkAddresses() { unsigned diagCount = diagnosticEmitter.getDiagnosticCount(); SmallSetVector moveIntroducersToProcess; diff --git a/test/SILOptimizer/ossa_lifetime_completion.sil b/test/SILOptimizer/ossa_lifetime_completion.sil index c07b5af962896..453665ea04b36 100644 --- a/test/SILOptimizer/ossa_lifetime_completion.sil +++ b/test/SILOptimizer/ossa_lifetime_completion.sil @@ -6,6 +6,11 @@ import Builtin class C {} +public enum FakeOptional { + case none + case some(T) +} + // CHECK-LABEL: begin running test 1 of 1 on eagerConsumneOwnedArg: ossa-lifetime-completion with: @argument // CHECK-LABEL: OSSA lifetime completion: %0 = argument of bb0 : $C // CHECK: sil [ossa] @eagerConsumneOwnedArg : $@convention(thin) (@owned C) -> () { @@ -45,3 +50,165 @@ bb3: %r = tuple () return %r : $() } + +// CHECK-LABEL: sil [ossa] @borrowTest : $@convention(method) (@owned C) -> () { +// CHECK: bb1: +// CHECK-NEXT: end_borrow +// CHECK-NEXT: br bb3 +// CHECK-LABEL: } // end sil function 'borrowTest' +sil [ossa] @borrowTest : $@convention(method) (@owned C) -> () { +bb0(%0 : @owned $C): + test_specification "ossa-lifetime-completion @instruction[0]" + %borrow = begin_borrow %0 : $C + cond_br undef, bb1, bb2 + +bb1: + end_borrow %borrow : $C + br bb3 + +bb2: + end_borrow %borrow : $C + br bb3 + +bb3: + destroy_value %0 : $C + %r = tuple () + return %r : $() +} + +// CHECK-LABEL: sil [ossa] @enumTest : $@convention(method) (@guaranteed FakeOptional) -> () { +// CHECK: bb2 +// CHECK: destroy_value +// CHECK: br bb3 +// CHECK-LABEL: } // end sil function 'enumTest' +sil [ossa] @enumTest : $@convention(method) (@guaranteed FakeOptional) -> () { +bb0(%0 : @guaranteed $FakeOptional): + test_specification "ossa-lifetime-completion @instruction[0]" + %copy = copy_value %0 : $FakeOptional + %borrow = begin_borrow %copy : $FakeOptional + switch_enum %borrow : $FakeOptional, case #FakeOptional.some!enumelt: bb1, case #FakeOptional.none!enumelt: bb2 + +bb1(%some : @guaranteed $C): + end_borrow %borrow : $FakeOptional + destroy_value %copy : $FakeOptional + br bb3 + +bb2: + end_borrow %borrow : $FakeOptional + br bb3 + +bb3: + %r = tuple () + return %r : $() +} + +sil @use_guaranteed : $@convention(thin) (@guaranteed C) -> () + +sil [ossa] @argTest : $@convention(method) (@owned C) -> () { +bb0(%0 : @owned $C): + test_specification "ossa-lifetime-completion @argument" + debug_value %0 : $C + cond_br undef, bb1, bb2 + +bb1: + br bb4 + +bb2: + br bb3 + +bb3: + %3 = function_ref @use_guaranteed : $@convention(thin) (@guaranteed C) -> () + %4 = apply %3(%0) : $@convention(thin) (@guaranteed C) -> () + destroy_value %0 : $C + %r = tuple () + return %r : $() + +bb4: + unreachable +} + +// Ensure no assert fires while inserting dead end blocks to the worklist +sil [ossa] @testLexicalLifetimeCompletion : $@convention(thin) (@owned C) -> () { +bb0(%0 : @owned $C): + test_specification "ossa-lifetime-completion @argument" + debug_value %0 : $C, let, name "newElements", argno 1 + cond_br undef, bb1, bb2 + +bb1: + cond_br undef, bb3, bb4 + +bb2: + cond_br undef, bb9, bb10 + +bb3: + br bb8 + +bb4: + cond_br undef, bb5, bb6 + +bb5: + br bb7 + +bb6: + br bb7 + +bb7: + unreachable + +bb8: + %77 = apply undef(%0) : $@convention(method) (@guaranteed C) -> () + destroy_value %0 : $C + %79 = tuple () + return %79 : $() + +bb9: + br bb8 + +bb10: + br bb8 +} + +sil @foo : $@convention(thin) (@guaranteed C) -> () + +// Ensure no assert fires while handling lifetime end of partial_apply +sil [ossa] @testPartialApplyStack1 : $@convention(thin) (@guaranteed C) -> () { +bb0(%0 : @guaranteed $C): + test_specification "ossa-lifetime-completion @instruction[0]" + %8 = copy_value %0 : $C + %9 = begin_borrow %8 : $C + %80 = function_ref @foo : $@convention(thin) (@guaranteed C) -> () + %81 = partial_apply [callee_guaranteed] [on_stack] %80(%9) : $@convention(thin) (@guaranteed C) -> () + cond_br undef, bb1, bb2 + +bb1: + destroy_value %81 : $@noescape @callee_guaranteed () -> () + br bb3 + +bb2: + unreachable + +bb3: + end_borrow %9 : $C + destroy_value %8 : $C + %180 = tuple () + return %180 : $() +} + +// Ensure no assert fires while handling lifetime end of partial_apply +sil [ossa] @testPartialApplyStack2 : $@convention(thin) (@guaranteed C) -> () { +bb0(%0 : @guaranteed $C): + test_specification "ossa-lifetime-completion @instruction[1]" + %2 = alloc_stack $C + %3 = copy_value %0 : $C + %4 = begin_borrow %3 : $C + %5 = function_ref @foo : $@convention(thin) (@guaranteed C) -> () + %6 = partial_apply [callee_guaranteed] [on_stack] %5(%4) : $@convention(thin) (@guaranteed C) -> () + %7 = mark_dependence %6 : $@noescape @callee_guaranteed () -> () on %2 : $*C + destroy_value %7 : $@noescape @callee_guaranteed () -> () + end_borrow %4 : $C + destroy_value %3 : $C + dealloc_stack %2 : $*C + %12 = tuple () + return %12 : $() +} + diff --git a/validation-test/SILOptimizer/gh68328.swift b/validation-test/SILOptimizer/gh68328.swift new file mode 100644 index 0000000000000..b440ae02497b4 --- /dev/null +++ b/validation-test/SILOptimizer/gh68328.swift @@ -0,0 +1,19 @@ +// RUN: %empty-directory(%t) +// RUN: %target-build-swift %s -o %t/bin +// RUN: %target-codesign %t/bin +// RUN: %target-run %t/bin 2> %t/out.txt || true +// RUN: %FileCheck %s < %t/out.txt + +// REQUIRES: executable_test + +struct Example: ~Copyable { + private var failureString: String { "Goodbye." } + deinit { fatalError("FATAL ERROR: \(failureString)") } +} + +func doit() { + let e = Example() + // CHECK: FATAL ERROR: Goodbye. +} + +doit()