Merge pull request #72777 from atrick/fix-lifedep-throw

LifetimeDependence: handle dependent values in throwing calls.

Author: atrick

SwiftCompilerSources/Sources/Optimizer/DataStructures/InstructionRange.swift

@@ -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!

SwiftCompilerSources/Sources/Optimizer/FunctionPasses/LifetimeDependenceInsertion.swift

@@ -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) =

SwiftCompilerSources/Sources/Optimizer/Utilities/LifetimeDependenceUtils.swift

@@ -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")
@@ -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.

SwiftCompilerSources/Sources/SIL/ForwardingInstruction.swift

@@ -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
   }
 }
@@ -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
 

SwiftCompilerSources/Sources/SIL/Value.swift

@@ -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!

include/swift/SIL/InstWrappers.h

@@ -140,6 +140,7 @@ struct ConversionOperation {
     case SILInstructionKind::CopyableToMoveOnlyWrapperValueInst:
     case SILInstructionKind::MoveOnlyWrapperToCopyableValueInst:
     case SILInstructionKind::MoveOnlyWrapperToCopyableBoxInst:
+    case SILInstructionKind::DropDeinitInst:
       return true;
     default:
       return false;

include/swift/SIL/SILInstruction.h

@@ -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
@@ -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
@@ -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;

lib/SIL/IR/SILInstruction.cpp

@@ -1885,6 +1885,10 @@ bool MarkDependenceInst::visitNonEscapingLifetimeEnds(
   return !noUsers;
 }
 
+bool DestroyValueInst::isFullDeinitialization() {
+  return !isa<DropDeinitInst>(lookThroughOwnershipInsts(getOperand()));
+}
+
 PartialApplyInst *
 DestroyValueInst::getNonescapingClosureAllocation() const {
   SILValue operand = getOperand();

lib/SILOptimizer/Utils/CanonicalizeInstruction.cpp

@@ -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);

test/SILOptimizer/lifetime_dependence_diagnostics.swift

@@ -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)
+}

test/SILOptimizer/lifetime_dependence_util.sil

@@ -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
@@ -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 : $()
+}

test/SILOptimizer/ownership_liveness_unit.sil

@@ -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)
@@ -243,7 +248,7 @@ exit(%reborrow : @guaranteed $C, %phi : @guaranteed $D):
 }
 
 // =============================================================================
-// InteriorLiveness and visitAdjacentPhis
+// InteriorLiveness
 // =============================================================================
 
 // CHECK-LABEL: testInteriorRefElementEscape: interior-liveness with: %0
@@ -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 {

test/SILOptimizer/sil_combine_moveonly.sil

@@ -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 : $()
+}