Merge pull request #67012 from gottesmm/release-5.9-rdar111497657

[5.9][move-only] If we have a guaranteed forwarding instruction with only trivial results, treat it as a liveness use.

Author: gottesmm

lib/SILOptimizer/Mandatory/MoveOnlyBorrowToDestructureUtils.cpp

@@ -369,16 +369,46 @@ bool Implementation::gatherUses(SILValue value) {
       continue;
     }
 
-    case OperandOwnership::GuaranteedForwarding:
-      // Look through guaranteed forwarding.
+    case OperandOwnership::GuaranteedForwarding: {
+      // Look through guaranteed forwarding if we have at least one non-trivial
+      // value. If we have all non-trivial values, treat this as a liveness use.
+      SmallVector<SILValue, 8> forwardedValues;
+      auto *fn = nextUse->getUser()->getFunction();
       ForwardingOperand(nextUse).visitForwardedValues([&](SILValue value) {
-        for (auto *use : value->getUses()) {
-          useWorklist.push_back(use);
-        }
+        if (value->getType().isTrivial(fn))
+          return true;
+        forwardedValues.push_back(value);
         return true;
       });
-      continue;
 
+      if (forwardedValues.empty()) {
+        auto leafRange =
+          TypeTreeLeafTypeRange::get(nextUse->get(), getRootValue());
+        if (!leafRange) {
+          LLVM_DEBUG(llvm::dbgs() << "        Failed to compute leaf range?!\n");
+          return false;
+        }
+
+        LLVM_DEBUG(llvm::dbgs() << "        Found non lifetime ending use!\n");
+        blocksToUses.insert(nextUse->getParentBlock(),
+                            {nextUse,
+                             {liveness.getNumSubElements(), *leafRange,
+                              false /*is lifetime ending*/}});
+        liveness.updateForUse(nextUse->getUser(), *leafRange,
+                              false /*is lifetime ending*/);
+        instToInterestingOperandIndexMap.insert(nextUse->getUser(), nextUse);
+        continue;
+      }
+
+      // If we had at least one forwarded value that is non-trivial, we need to
+      // visit those uses.
+      while (!forwardedValues.empty()) {
+        for (auto *use : forwardedValues.pop_back_val()->getUses()) {
+          useWorklist.push_back(use);
+        }
+      }
+      continue;
+    }
     case OperandOwnership::Borrow: {
       // Look through borrows.
       if (auto *bbi = dyn_cast<BeginBorrowInst>(nextUse->getUser())) {

test/Interpreter/moveonly_fixedqueue.swift

@@ -0,0 +1,66 @@
+// RUN: %target-run-simple-swift(-Xfrontend -sil-verify-all)
+// RUN: %target-run-simple-swift(-O -Xfrontend -sil-verify-all)
+
+enum FixedSizeQueueError : Error {
+    case outOfSpace
+}
+
+struct FixedSizeQueue<T> : ~Copyable {
+    private var readOffset: Int = 0
+    private var writeOffset: Int = 0
+    private var ptr: UnsafeMutableBufferPointer<T>
+
+    init(size: Int) {
+        ptr = UnsafeMutableBufferPointer.allocate(capacity: size)
+    }
+
+    deinit {
+        ptr.deinitialize().deallocate()
+    }
+
+    mutating func read() -> T? {
+        if readOffset == writeOffset {
+            return nil
+        }
+
+        let x = ptr[readOffset]
+        readOffset = (readOffset + 1) % ptr.count
+        return x
+    }
+
+    mutating func write(_ element: T) throws {
+        if ((writeOffset + 1) % ptr.count) == readOffset {
+            throw FixedSizeQueueError.outOfSpace
+        }
+        ptr[writeOffset] = element
+        writeOffset = (writeOffset + 1) % ptr.count
+    }
+
+    // Drain the queue... returning the value.
+    consuming func drain() -> UnsafeMutableBufferPointer<T> {
+        let buffer = self.ptr
+        discard self
+        return buffer
+    }
+}
+
+func main() throws {
+    var x = FixedSizeQueue<Int>(size: 10)
+    precondition(x.read() == nil)
+    try x.write(0)
+    try x.write(9)
+    try x.write(1)
+    try x.write(3)
+    precondition(x.read() == 0)
+    precondition(x.read() == 9)
+    precondition(x.read() == 1)
+    precondition(x.read() == 3)
+
+    let d = x.drain()
+    precondition(d[0] == 0)
+    precondition(d[1] == 9)
+    precondition(d[2] == 1)
+    precondition(d[3] == 3)
+}
+
+try main()

test/SILOptimizer/moveonly_addresschecker.sil

@@ -677,3 +677,58 @@ bb0:
   %21 = tuple ()
   return %21 : $()
 }
+
+struct FixedSizeQueue : ~Copyable {
+  var ptr: Builtin.Int32
+}
+
+sil @get_fixedsize_queue : $@convention(thin) () -> @owned FixedSizeQueue
+
+// CHECK-LABEL: sil [ossa] @test_return_trivial_type : $@convention(thin) (@owned FixedSizeQueue) -> Builtin.Int32 {
+// CHECK: bb0([[ARG:%.*]] : @owned $
+// CHECK:   [[ALLOC:%.*]] = alloc_stack [lexical] $FixedSizeQueue
+// CHECK:   store [[ARG]] to [init] [[ALLOC]]
+// CHECK:   [[ACCESS:%.*]] = begin_access [modify] [static] [[ALLOC]]
+// CHECK:   [[ALLOC2:%.*]] = alloc_stack $FixedSizeQueue
+// CHECK:   copy_addr [take] [[ACCESS]] to [init] [[ALLOC2]]
+// CHECK:   [[LOADED:%.*]] = load_borrow [[ALLOC2]]
+// CHECK:   end_access [[ACCESS]]
+// CHECK:   [[BORROW:%.*]] = begin_borrow [[LOADED]]
+// CHECK:   [[TRIVIAL_VALUE:%.*]] = struct_extract [[BORROW]]
+// CHECK:   end_borrow [[BORROW]]
+// CHECK:   end_borrow [[LOADED]]
+// CHECK:   destroy_addr [[ALLOC2]]
+// CHECK:   dealloc_stack [[ALLOC2]]
+// CHECK:   [[NEW_VALUE:%.*]] = apply {{%.*}}()
+// CHECK:   [[ACCESS:%.*]] = begin_access [modify] [static] [[ALLOC]]
+// CHECK:   store [[NEW_VALUE]] to [init] [[ACCESS]]
+// CHECK:   end_access [[ACCESS]]
+// CHECK:   destroy_addr [[ALLOC]]
+// CHECK:   dealloc_stack [[ALLOC]]
+// CHECK:   return [[TRIVIAL_VALUE]]
+// CHECK: } // end sil function 'test_return_trivial_type'
+sil [ossa] @test_return_trivial_type : $@convention(thin) (@owned FixedSizeQueue) -> Builtin.Int32 {
+bb0(%0 : @owned $FixedSizeQueue):
+  %1 = alloc_stack [lexical] $FixedSizeQueue, var, name "self", implicit
+  %2 = mark_must_check [consumable_and_assignable] %1 : $*FixedSizeQueue
+  store %0 to [init] %2 : $*FixedSizeQueue
+  %4 = begin_access [modify] [static] %2 : $*FixedSizeQueue
+  %5 = alloc_stack $FixedSizeQueue
+  %6 = mark_must_check [consumable_and_assignable] %5 : $*FixedSizeQueue
+  copy_addr %4 to [init] %6 : $*FixedSizeQueue
+  %8 = load [take] %6 : $*FixedSizeQueue
+  end_access %4 : $*FixedSizeQueue
+  %10 = begin_borrow %8 : $FixedSizeQueue
+  %11 = struct_extract %10 : $FixedSizeQueue, #FixedSizeQueue.ptr
+  end_borrow %10 : $FixedSizeQueue
+  destroy_value %8 : $FixedSizeQueue
+  dealloc_stack %5 : $*FixedSizeQueue
+  %21 = function_ref @get_fixedsize_queue : $@convention(thin) () -> @owned FixedSizeQueue
+  %22 = apply %21() : $@convention(thin) () -> @owned FixedSizeQueue
+  %23 = begin_access [modify] [static] %2 : $*FixedSizeQueue
+  store %22 to [assign] %23 : $*FixedSizeQueue
+  end_access %23 : $*FixedSizeQueue
+  destroy_addr %2 : $*FixedSizeQueue
+  dealloc_stack %1 : $*FixedSizeQueue
+  return %11 : $Builtin.Int32
+}

test/SILOptimizer/moveonly_addresschecker.swift

@@ -0,0 +1,16 @@
+// RUN: %target-swift-emit-sil -O -sil-verify-all -verify -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s
+
+// This file contains tests that used to crash due to verifier errors. It must
+// be separate from moveonly_addresschecker_diagnostics since when we fail on
+// the diagnostics in that file, we do not actually run the verifier.
+
+struct TestTrivialReturnValue : ~Copyable {
+    var i: Int = 5
+
+    // We used to error on return buffer.
+    consuming func drain() -> Int {
+        let buffer = (consume self).i
+        self = .init(i: 5)
+        return buffer
+    }
+}

test/SILOptimizer/moveonly_borrow_to_destructure_transform.sil

@@ -147,12 +147,12 @@ bb0(%0 : @owned $MoveOnlyKlass):
 // CHECK:   [[ARG_COPY:%.*]] = copy_value [[ARG]]
 // CHECK:   [[MARKED_ARG:%.*]] = mark_must_check [no_consume_or_assign] [[ARG_COPY]]
 // CHECK:   [[MARKED_ARG_COPY:%.*]] = copy_value [[MARKED_ARG]]
-// CHECK:   [[MISC_USE:%.*]] = function_ref @misc_use : $@convention(thin) () -> ()
-// CHECK:   apply [[MISC_USE]]()
 // CHECK:   [[BORROW:%.*]] = begin_borrow [[MARKED_ARG_COPY]]
 // CHECK:   [[EXT:%.*]] = struct_extract [[BORROW]]
 // CHECK:   end_borrow [[BORROW]]
-// CHECK:   destroy_value [[MARKED_ARG_COPY]]
+// CHECK:   [[MISC_USE:%.*]] = function_ref @misc_use : $@convention(thin) () -> ()
+// CHECK:   apply [[MISC_USE]]()
+// CHECK:   destroy_value [[MARKED_ARG]]
 // CHECK:   return [[EXT]]
 // CHECK: } // end sil function 'test_return_of_extracted_trivial_type'
 sil [ossa] @test_return_of_extracted_trivial_type : $@convention(thin) (@guaranteed SingleIntContainingStruct) -> Builtin.Int32 {
@@ -862,9 +862,9 @@ bbCont:
 // CHECK: [[BB_E_SECOND]]([[BBARG:%.*]] : @owned
 // CHECK:   [[BBARG_BORROW:%.*]] = begin_borrow [[BBARG]]
 // CHECK:   [[BBARG_BORROW_EXT:%.*]] = struct_extract [[BBARG_BORROW]]
-// CHECK:   apply {{%.*}}([[BBARG_BORROW_EXT]])
 // CHECK:   end_borrow [[BBARG_BORROW]]
 // CHECK:   destroy_value [[BBARG]]
+// CHECK:   apply {{%.*}}([[BBARG_BORROW_EXT]])
 // CHECK:   br [[BB_CONT]]
 //
 // CHECK: [[BB_E_THIRD]]([[BBARG:%.*]] : @owned
@@ -955,9 +955,9 @@ bbCont:
 // CHECK: [[BB_E_SECOND]]([[BBARG:%.*]] : @owned
 // CHECK:   [[BBARG_BORROW:%.*]] = begin_borrow [[BBARG]]
 // CHECK:   [[BBARG_BORROW_EXT:%.*]] = struct_extract [[BBARG_BORROW]]
-// CHECK:   apply {{%.*}}([[BBARG_BORROW_EXT]])
 // CHECK:   end_borrow [[BBARG_BORROW]]
 // CHECK:   destroy_value [[BBARG]]
+// CHECK:   apply {{%.*}}([[BBARG_BORROW_EXT]])
 // CHECK:   br [[BB_CONT]]
 //
 // CHECK: [[BB_E_THIRD]]([[BBARG:%.*]] : @owned
@@ -1076,4 +1076,4 @@ bb2:
   end_borrow %3 : $KlassPair
   destroy_value %2 : $KlassPair
   unwind
-}
+}