Merge pull request #62486 from nate-chandler/opaque-values/1/20221209

[AddressLowering] Handle store_borrow.

Author: nate-chandler

docs/SIL.rst

@@ -4070,11 +4070,14 @@ The stack location must not be modified by other instructions than
 ``store_borrow``.
 All uses of the store_borrow destination ```%1`` should be via the store_borrow
 return address ``%2`` except dealloc_stack.
-The stored value is alive until the ``end_borrow``. During the its lifetime,the
+The stored value is alive until the ``end_borrow``. During its lifetime, the
 stored value must not be modified or destroyed.
-The source value ``%0`` is borrowed (i.e. not copied) and it's borrow scope
+The source value ``%0`` is borrowed (i.e. not copied) and its borrow scope
 must outlive the lifetime of the stored value.
 
+Notionally, the outer borrow scope ensures that there's something to be
+addressed.  The inner borrow scope provides the address to work with.
+
 begin_borrow
 ````````````
 

lib/SIL/Verifier/MemoryLifetimeVerifier.cpp

@@ -562,15 +562,16 @@ void MemoryLifetimeVerifier::checkFunction(BitDataflow &dataFlow) {
         require(expectedReturnBits & ~bs.data.exitSet,
           "indirect argument is not alive at function return", term);
         require(bs.data.exitSet & ~expectedReturnBits & nonTrivialLocations,
-          "memory is initialized at function return but shouldn't", term,
-           /*excludeTrivialEnums*/ true);
+                "memory is initialized at function return but shouldn't be",
+                term,
+                /*excludeTrivialEnums*/ true);
         break;
       case SILInstructionKind::ThrowInst:
         require(expectedThrowBits & ~bs.data.exitSet,
           "indirect argument is not alive at throw", term);
         require(bs.data.exitSet & ~expectedThrowBits & nonTrivialLocations,
-          "memory is initialized at throw but shouldn't", term,
-           /*excludeTrivialEnums*/ true);
+                "memory is initialized at throw but shouldn't be", term,
+                /*excludeTrivialEnums*/ true);
         break;
       default:
         break;

lib/SIL/Verifier/SILVerifier.cpp

@@ -2243,6 +2243,10 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
     require(
         F.hasOwnership(),
         "Inst with qualified ownership in a function that is not qualified");
+    if (EBI->getOperand()->getType().isAddress()) {
+      require(isa<StoreBorrowInst>(EBI->getOperand()),
+              "end_borrow of an address not produced by store_borrow");
+    }
   }
 
   void checkEndLifetimeInst(EndLifetimeInst *I) {
@@ -2446,23 +2450,22 @@ class SILVerifier : public SILVerifierBase<SILVerifier> {
   }
 
   void checkStoreBorrowInst(StoreBorrowInst *SI) {
+    // A store_borrow must be to an alloc_stack.  That alloc_stack can only be
+    // used by store_borrows (and dealloc_stacks).
     require(SI->getSrc()->getType().isObject(),
             "Can't store from an address source");
     require(!fnConv.useLoweredAddresses()
                 || SI->getSrc()->getType().isLoadable(*SI->getFunction()),
             "Can't store a non loadable type");
     require(SI->getDest()->getType().isAddress(),
             "Must store to an address dest");
+    // Note: This is the current implementation and the design is not final.
     require(isa<AllocStackInst>(SI->getDest()),
-            "store_borrow destination should be alloc_stack");
+            "store_borrow destination can only be an alloc_stack");
     requireSameType(SI->getDest()->getType().getObjectType(),
                     SI->getSrc()->getType(),
                     "Store operand type and dest type mismatch");
 
-    // Note: This is the current implementation and the design is not final.
-    require(isa<AllocStackInst>(SI->getDest()),
-            "store_borrow destination can only be an alloc_stack");
-
     SSAPrunedLiveness scopedAddressLiveness;
     ScopedAddressValue scopedAddress(SI);
     // FIXME: Reenable @test_load_borrow_store_borrow_nested in

lib/SILOptimizer/Mandatory/AddressLowering.cpp

@@ -3101,8 +3101,17 @@ class UseRewriter : SILInstructionVisitor<UseRewriter> {
     llvm::report_fatal_error("Unimplemented SelectValue use.");
   }
 
-  // Opaque enum operand to a switch_enum.
-  void visitSwitchEnumInst(SwitchEnumInst *SEI);
+  void visitStoreBorrowInst(StoreBorrowInst *sbi) {
+    auto addr = addrMat.materializeAddress(use->get());
+    SmallVector<Operand *, 4> uses(sbi->getUses());
+    for (auto *use : uses) {
+      if (auto *ebi = dyn_cast<EndBorrowInst>(use->getUser())) {
+        pass.deleter.forceDelete(ebi);
+      }
+    }
+    sbi->replaceAllUsesWith(addr);
+    pass.deleter.forceDelete(sbi);
+  }
 
   void rewriteStore(SILValue srcVal, SILValue destAddr,
                     IsInitialization_t isInit);
@@ -3134,6 +3143,9 @@ class UseRewriter : SILInstructionVisitor<UseRewriter> {
   // member implies an address-only Struct.
   void visitStructInst(StructInst *structInst) {}
 
+  // Opaque enum operand to a switch_enum.
+  void visitSwitchEnumInst(SwitchEnumInst *SEI);
+
   // Opaque call argument.
   void visitTryApplyInst(TryApplyInst *tryApplyInst) {
     CallArgRewriter(tryApplyInst, pass).rewriteIndirectArgument(use);

test/SIL/store_borrow_verify_errors.sil

@@ -196,7 +196,7 @@ bb1:
 }
 
 // CHECK: Begin Error in function test_store_borrow_dest
-// CHECK: SIL verification failed: store_borrow destination should be alloc_stack: isa<AllocStackInst>(SI->getDest())
+// CHECK: SIL verification failed: store_borrow destination can only be an alloc_stack: isa<AllocStackInst>(SI->getDest())
 // CHECK: Verifying instruction:
 // CHECK:    %0 = argument of bb0 : $Klass                  // user: %3
 // CHECK:      %2 = struct_element_addr %1 : $*NonTrivialStruct, #NonTrivialStruct.val // user: %3

test/SIL/verifier_failures.sil

@@ -0,0 +1,20 @@
+// RUN: %target-sil-opt -emit-sorted-sil -verify-continue-on-failure -o /dev/null %s 2>&1 | %FileCheck %s
+// REQUIRES: asserts
+
+sil_stage canonical
+
+import Builtin
+import Swift
+import SwiftShims
+
+class C {}
+
+// CHECK:      Begin Error in function end_borrow_1_addr_alloc_stack
+// CHECK-NEXT: SIL verification failed: end_borrow of an address not produced by store_borrow
+sil [ossa] @end_borrow_1_addr_alloc_stack : $@convention(thin) () -> () {
+  %addr = alloc_stack $C
+  end_borrow %addr : $*C
+  dealloc_stack %addr : $*C
+  %retval = tuple ()
+  return %retval : $()
+}

test/SILOptimizer/address_lowering.sil

@@ -2149,6 +2149,36 @@ bb0:
   return %retval : $()
 }
 
+// CHECK-LABEL: sil [ossa] @test_store_borrow_1_copy_addr : {{.*}} {
+// CHECK:         [[ADDR:%[^,]+]] = alloc_stack
+// CHECK:         apply undef<T>([[ADDR]])
+// CHECK:         [[COPY:%[^,]+]] = alloc_stack
+// CHECK:         copy_addr [[ADDR]] to [init] [[COPY]]
+// CHECK:         apply undef<T>([[COPY]])
+// CHECK:         destroy_addr [[COPY]]
+// CHECK:         dealloc_stack [[COPY]]
+// CHECK-NOT:     end_borrow [[ADDR]]
+// CHECK:         destroy_addr [[ADDR]]
+// CHECK-LABEL: } // end sil function 'test_store_borrow_1_copy_addr'
+sil [ossa] @test_store_borrow_1_copy_addr : $@convention(thin) <T> () -> () {
+entry:
+  %addr = alloc_stack $T
+  %instance = apply undef<T>() : $@convention(thin) <τ> () -> (@out τ)
+  %lifetime = begin_borrow %instance : $T
+  %borrow = store_borrow %lifetime to %addr : $*T
+  %copy = alloc_stack $T
+  copy_addr %borrow to [init] %copy : $*T
+  apply undef<T>(%copy) : $@convention(thin) <τ> (@inout τ) -> ()
+  destroy_addr %copy : $*T
+  dealloc_stack %copy : $*T
+  end_borrow %borrow : $*T
+  end_borrow %lifetime : $T
+  dealloc_stack %addr : $*T
+  destroy_value %instance : $T
+  %retval = tuple ()
+  return %retval : $()
+}
+
 // CHECK-LABEL: sil hidden [ossa] @test_unchecked_bitwise_cast :
 // CHECK: bb0(%0 : $*U, %1 : $*T, %2 : $@thick U.Type):
 // CHECK:   [[STK:%.*]] = alloc_stack $T