Merge pull request swiftlang#75050 from atrick/vardecl_lifetime

[SILGen] Create SIL variable declaration scopes for trivial values.

Author: atrick

include/swift/SIL/SILBuilder.h

@@ -1459,7 +1459,7 @@ class SILBuilder {
       HasPointerEscape_t hasPointerEscape = DoesNotHavePointerEscape,
       IsFromVarDecl_t fromVarDecl = IsNotFromVarDecl) {
     assert(getFunction().hasOwnership());
-    assert(!operand->getType().isTrivial(getFunction()) &&
+    assert(fromVarDecl || !operand->getType().isTrivial(getFunction()) &&
            "Should not be passing trivial values to this api. Use instead "
            "emitMoveValueOperation");
     return insert(new (getModule())

include/swift/SIL/SILInstruction.h

@@ -1978,6 +1978,7 @@ enum HasPointerEscape_t : bool {
   HasPointerEscape = true,
 };
 
+// See SILValue::isFromVarDecl()
 enum IsFromVarDecl_t : bool {
   IsNotFromVarDecl = false,
   IsFromVarDecl = true,

include/swift/SIL/SILValue.h

@@ -609,6 +609,9 @@ class ValueBase : public SILNode, public SILAllocated<ValueBase> {
   /// (it has a debug_value [trace] user).
   bool hasDebugTrace() const;
 
+  /// Does this SILValue begin a VarDecl scope? Only true in OSSA.
+  bool isFromVarDecl();
+
   static bool classof(SILNodePointer node) {
     return node->getKind() >= SILNodeKind::First_ValueBase &&
            node->getKind() <= SILNodeKind::Last_ValueBase;

include/swift/SILOptimizer/Utils/StackNesting.h

@@ -160,7 +160,11 @@ class StackNesting {
   /// Returns the stack allocation instruction for a stack deallocation
   /// instruction.
   SILInstruction *getAllocForDealloc(SILInstruction *Dealloc) const {
-    return Dealloc->getOperand(0)->getDefiningInstruction();
+    SILValue op = Dealloc->getOperand(0);
+    while (auto *mvi = dyn_cast<MoveValueInst>(op)) {
+      op = mvi->getOperand();
+    }
+    return op->getDefiningInstruction();
   }
 
   /// Insert deallocations at block boundaries.

lib/SIL/IR/SILValue.cpp

@@ -209,6 +209,16 @@ bool ValueBase::hasDebugTrace() const {
   return false;
 }
 
+bool ValueBase::isFromVarDecl() {
+  if (auto *mvi = dyn_cast<MoveValueInst>(this)) {
+    return mvi->isFromVarDecl();
+  }
+  if (auto *bbi = dyn_cast<BeginBorrowInst>(this)) {
+    return bbi->isFromVarDecl();
+  }
+  return false;
+}
+
 SILBasicBlock *SILNode::getParentBlock() const {
   if (auto *Inst = dyn_cast<SILInstruction>(this))
     return Inst->getParent();

lib/SIL/IR/TypeLowering.cpp

@@ -1061,6 +1061,15 @@ namespace {
 
     void emitDestroyValue(SILBuilder &B, SILLocation loc,
                           SILValue value) const override {
+      if (B.getFunction().hasOwnership()
+          && B.getModule().getStage() == SILStage::Raw
+          && value->isFromVarDecl()) {
+        // Do not use destroy_value for trivial values. The lifetime introducer
+        // may be implicitly copied and used outside of its original scope,
+        // which violates the invariants of destroy_value.
+        B.createExtendLifetime(loc, value);
+        return;
+      }
       // Trivial
     }
   };

lib/SIL/IR/ValueOwnership.cpp

@@ -85,7 +85,6 @@ CONSTANT_OWNERSHIP_INST(Owned, CopyBlock)
 CONSTANT_OWNERSHIP_INST(Owned, CopyBlockWithoutEscaping)
 CONSTANT_OWNERSHIP_INST(Owned, CopyValue)
 CONSTANT_OWNERSHIP_INST(Owned, ExplicitCopyValue)
-CONSTANT_OWNERSHIP_INST(Owned, MoveValue)
 CONSTANT_OWNERSHIP_INST(Owned, EndCOWMutation)
 CONSTANT_OWNERSHIP_INST(Owned, EndInitLetRef)
 CONSTANT_OWNERSHIP_INST(Owned, BeginDeallocRef)
@@ -212,7 +211,9 @@ CONSTANT_OR_NONE_OWNERSHIP_INST(Guaranteed, OpenExistentialBoxValue)
 // value).
 CONSTANT_OR_NONE_OWNERSHIP_INST(Owned, MarkUninitialized)
 
-// unchecked_bitwise_cast is a bitwise copy. It produces a trivial or unowned
+// In raw SIL, a MoveValue delimits the scope of trivial variables.
+CONSTANT_OR_NONE_OWNERSHIP_INST(Owned, MoveValue)
+
 // result.
 //
 // If the operand is nontrivial and the result is trivial, then it is the

lib/SIL/Verifier/SILOwnershipVerifier.cpp

@@ -607,8 +607,8 @@ bool SILValueOwnershipChecker::checkYieldWithoutLifetimeEndingUses(
 
 bool SILValueOwnershipChecker::checkValueWithoutLifetimeEndingUses(
     ArrayRef<Operand *> regularUses, ArrayRef<Operand *> extendLifetimeUses) {
-  if (extendLifetimeUses.size()) {
-  }
+  if (value->getOwnershipKind() == OwnershipKind::None)
+    return true;
 
   LLVM_DEBUG(llvm::dbgs() << "No lifetime ending users?! Bailing early.\n");
   if (auto *arg = dyn_cast<SILFunctionArgument>(value)) {

lib/SIL/Verifier/SILVerifier.cpp

@@ -2702,13 +2702,18 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
   }
 
   void checkExtendLifetimeInst(ExtendLifetimeInst *I) {
-    require(!I->getOperand()->getType().isTrivial(*I->getFunction()),
-            "Source value should be non-trivial");
     require(F.hasOwnership(),
             "extend_lifetime is only valid in functions with qualified "
             "ownership");
+    // In Raw SIL, extend_lifetime marks the end of variable scopes.
+    if (F.getModule().getStage() == SILStage::Raw)
+      return;
+
+    require(!I->getOperand()->getType().isTrivial(*I->getFunction()),
+            "Source value should be non-trivial after diagnostics");
     require(getDeadEndBlocks().isDeadEnd(I->getParent()),
-            "extend_lifetime in non-dead-end!?");
+            "extend_lifetime in non-dead-end after diagnostics");
+
     auto value = I->getOperand();
     LinearLiveness linearLiveness(value,
                                   LinearLiveness::DoNotIncludeExtensions);
@@ -3403,7 +3408,8 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
   void checkDestroyValueInst(DestroyValueInst *I) {
     require(I->getOperand()->getType().isObject(),
             "Source value should be an object value");
-    require(!I->getOperand()->getType().isTrivial(*I->getFunction()),
+    require(!I->getOperand()->getType().isTrivial(*I->getFunction())
+            || I->getOperand()->isFromVarDecl(),
             "Source value should be non-trivial");
     require(!fnConv.useLoweredAddresses() || F.hasOwnership(),
             "destroy_value is only valid in functions with qualified "
@@ -6779,6 +6785,9 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
 
         } else if (i.isDeallocatingStack()) {
           SILValue op = i.getOperand(0);
+          while (auto *mvi = dyn_cast<MoveValueInst>(op)) {
+            op = mvi->getOperand();
+          }
           require(!state.Stack.empty(),
                   "stack dealloc with empty stack");
           if (op != state.Stack.back()) {