Merge pull request #67370 from gottesmm/release-5.9-112434492

gottesmm · web-flow · commit c3fc62c0cd63 · 2023-07-19T13:36:24.000-07:00
[5.9][move-only] Make sure that we mask out liveness bits and use |= when merging successors
diff --git a/include/swift/SIL/FieldSensitivePrunedLiveness.h b/include/swift/SIL/FieldSensitivePrunedLiveness.h
@@ -724,17 +724,22 @@ class FieldSensitivePrunedLiveness {
 
     /// Populates the provided vector with contiguous ranges of bits which are
     /// users of the same sort.
+    ///
+    /// All bits not selected by \p selectedBits are assumed to be
+    /// IsInterestingUser::NonUser.
     void getContiguousRanges(
         SmallVectorImpl<std::pair<TypeTreeLeafTypeRange, IsInterestingUser>>
-            &ranges) const {
+            &ranges,
+        const SmallBitVector &selectedBits) const {
       if (liveBits.size() == 0)
         return;
 
       assert(ranges.empty());
       llvm::Optional<std::pair<unsigned, IsInterestingUser>> current =
           llvm::None;
       for (unsigned bit = 0, size = liveBits.size(); bit < size; ++bit) {
-        auto interesting = isInterestingUser(bit);
+        auto interesting = selectedBits.test(bit) ? isInterestingUser(bit)
+                                                  : IsInterestingUser::NonUser;
         if (!current) {
           current = {bit, interesting};
           continue;
diff --git a/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp b/lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp
@@ -2770,7 +2770,7 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
     auto interestingUser = liveness.getInterestingUser(inst);
     SmallVector<std::pair<TypeTreeLeafTypeRange, IsInterestingUser>, 4> ranges;
     if (interestingUser) {
-      interestingUser->getContiguousRanges(ranges);
+      interestingUser->getContiguousRanges(ranges, bv);
     }
 
     for (auto rangePair : ranges) {
@@ -2808,12 +2808,14 @@ void MoveOnlyAddressCheckerPImpl::insertDestroysOnBoundary(
           auto *block = inst->getParent();
           for (auto *succBlock : block->getSuccessorBlocks()) {
             auto iter = mergeBlocks.find(succBlock);
-            if (iter == mergeBlocks.end())
+            if (iter == mergeBlocks.end()) {
               iter = mergeBlocks.insert({succBlock, bits}).first;
-            else {
+            } else {
+              // NOTE: We use |= here so that different regions of the same
+              // terminator get updated appropriately.
               SmallBitVector &alreadySetBits = iter->second;
               bool hadCommon = alreadySetBits.anyCommon(bits);
-              alreadySetBits &= bits;
+              alreadySetBits |= bits;
               if (hadCommon)
                 continue;
             }
diff --git a/test/SILOptimizer/moveonly_addresschecker.sil b/test/SILOptimizer/moveonly_addresschecker.sil
@@ -69,6 +69,18 @@ sil @finalizeSingleTrivialFieldAndDeinit : $@convention(thin) (@owned SingleTriv
 sil_global hidden @$s23moveonly_addresschecker9varGlobalAA16NonTrivialStructVvp : $NonTrivialStruct
 sil_global hidden [let] @$s23moveonly_addresschecker9letGlobalAA16NonTrivialStructVvp : $NonTrivialStruct
 
+struct NonCopyableNativeObjectIntPair : ~Copyable {
+  var a: Builtin.NativeObject
+  var currentPosition: Int
+}
+
+struct NonCopyableNativeObjectPair : ~Copyable {
+  var a: Builtin.NativeObject
+  var currentPosition: Builtin.NativeObject
+}
+
+sil @get_nativeobject : $@convention(thin) () -> @owned Builtin.NativeObject
+
 ///////////
 // Tests //
 ///////////
@@ -602,7 +614,7 @@ sil @get_M2 : $@convention(thin) () -> @owned M2
 sil @end_addr_see_addr : $@convention(thin) (@in M, @in_guaranteed M) -> ()
 
 /// A single instruction, apply @end_addr_see_addr, consumes one field and
-/// borrows another.  
+/// borrows another.
 
 /// Varify that the consumed value isn't destroyed twice and that the borrowed
 /// value isn't destroyed before it's used.
@@ -768,3 +780,67 @@ bb3:
   %9999 = tuple ()
   return %9999 : $()
 }
+
+sil @testUseCorrectBitsClosureCallee : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectIntPair) -> Bool
+sil @testUseCorrectBitsClosureUser : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> Bool) -> ()
+
+// CHECK-LABEL: sil [ossa] @testUseCorrectBits : $@convention(method) (Int, @inout NonCopyableNativeObjectIntPair) -> () {
+// CHECK-NOT: destroy_addr
+// CHECK: } // end sil function 'testUseCorrectBits'
+sil [ossa] @testUseCorrectBits : $@convention(method) (Int, @inout NonCopyableNativeObjectIntPair) -> () {
+bb0(%0 : $Int, %1a : $*NonCopyableNativeObjectIntPair):
+  debug_value %0 : $Int, let, name "index", argno 1
+  debug_value %1a : $*NonCopyableNativeObjectIntPair, var, name "self", argno 2, implicit, expr op_deref
+  %1 = mark_must_check [consumable_and_assignable] %1a : $*NonCopyableNativeObjectIntPair
+  %4 = function_ref @testUseCorrectBitsClosureCallee : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectIntPair) -> Bool
+  %5 = partial_apply [callee_guaranteed] [on_stack] %4(%0, %1) : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectIntPair) -> Bool
+  %6 = mark_dependence %5 : $@noescape @callee_guaranteed () -> Bool on %1 : $*NonCopyableNativeObjectIntPair
+  %7 = function_ref @testUseCorrectBitsClosureUser : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> Bool) -> ()
+  %8 = apply %7(%6) : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> Bool) -> ()
+  %9 = struct_element_addr %1 : $*NonCopyableNativeObjectIntPair, #NonCopyableNativeObjectIntPair.currentPosition
+  debug_value undef : $*NonCopyableNativeObjectIntPair, var, name "self", argno 2, implicit, expr op_deref
+  destroy_value %6 : $@noescape @callee_guaranteed () -> Bool
+  %12 = begin_access [modify] [static] %1 : $*NonCopyableNativeObjectIntPair
+  %13 = struct_element_addr %12 : $*NonCopyableNativeObjectIntPair, #NonCopyableNativeObjectIntPair.currentPosition
+  store %0 to [trivial] %13 : $*Int
+  end_access %12 : $*NonCopyableNativeObjectIntPair
+  %16 = tuple ()
+  return %16 : $()
+}
+
+sil @testUseCorrectBits2ClosureCallee : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectPair) -> Bool
+
+// CHECK-LABEL: sil [ossa] @testUseCorrectBits2 : $@convention(method) (Int, @inout NonCopyableNativeObjectPair) -> () {
+// CHECK: bb0({{%.*}} : $Int, [[ARG:%.*]] : $*
+// CHECK-NOT: destroy_addr
+// CHECK:   [[ACCESS:%.*]] = begin_access [modify] [static] [[ARG]]
+// CHECK-NEXT: [[GEP:%.*]] = struct_element_addr [[ACCESS]]
+// CHECK-NEXT: function_ref get_nativeobject
+// CHECK-NEXT: function_ref @get_nativeobject
+// CHECK-NEXT: apply {{%.*}}
+// CHECK-NEXT: [[GEP2:%.*]] = struct_element_addr [[ARG]] : $*NonCopyableNativeObjectPair, #NonCopyableNativeObjectPair.currentPosition
+// CHECK-NEXT: destroy_addr [[GEP2]]
+// CHECK-NOT: destroy_addr
+// CHECK: } // end sil function 'testUseCorrectBits2'
+sil [ossa] @testUseCorrectBits2 : $@convention(method) (Int, @inout NonCopyableNativeObjectPair) -> () {
+bb0(%0 : $Int, %1a : $*NonCopyableNativeObjectPair):
+  debug_value %0 : $Int, let, name "index", argno 1
+  debug_value %1a : $*NonCopyableNativeObjectPair, var, name "self", argno 2, implicit, expr op_deref
+  %1 = mark_must_check [consumable_and_assignable] %1a : $*NonCopyableNativeObjectPair
+  %4 = function_ref @testUseCorrectBits2ClosureCallee : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectPair) -> Bool
+  %5 = partial_apply [callee_guaranteed] [on_stack] %4(%0, %1) : $@convention(thin) (Int, @inout_aliasable NonCopyableNativeObjectPair) -> Bool
+  %6 = mark_dependence %5 : $@noescape @callee_guaranteed () -> Bool on %1 : $*NonCopyableNativeObjectPair
+  %7 = function_ref @testUseCorrectBitsClosureUser : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> Bool) -> ()
+  %8 = apply %7(%6) : $@convention(thin) (@guaranteed @noescape @callee_guaranteed () -> Bool) -> ()
+  %9 = struct_element_addr %1 : $*NonCopyableNativeObjectPair, #NonCopyableNativeObjectPair.currentPosition
+  debug_value undef : $*NonCopyableNativeObjectPair, var, name "self", argno 2, implicit, expr op_deref
+  destroy_value %6 : $@noescape @callee_guaranteed () -> Bool
+  %12 = begin_access [modify] [static] %1 : $*NonCopyableNativeObjectPair
+  %13 = struct_element_addr %12 : $*NonCopyableNativeObjectPair, #NonCopyableNativeObjectPair.currentPosition
+  %f = function_ref @get_nativeobject : $@convention(thin) () -> @owned Builtin.NativeObject
+  %14 = apply %f() : $@convention(thin) () -> @owned Builtin.NativeObject
+  store %14 to [assign] %13 : $*Builtin.NativeObject
+  end_access %12 : $*NonCopyableNativeObjectPair
+  %16 = tuple ()
+  return %16 : $()
+}
diff --git a/test/SILOptimizer/moveonly_addresschecker.swift b/test/SILOptimizer/moveonly_addresschecker.swift
@@ -1,3 +1,4 @@
+// RUN: %target-swift-emit-sil -sil-verify-all -verify -enable-experimental-feature MoveOnlyPartialConsumption -enable-experimental-feature NoImplicitCopy -enable-experimental-feature MoveOnlyClasses %s -Xllvm -sil-print-final-ossa-module | %FileCheck %s
 // 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
@@ -14,3 +15,23 @@ struct TestTrivialReturnValue : ~Copyable {
         return buffer
     }
 }
+
+
+//////////////////////
+// MARK: Misc Tests //
+//////////////////////
+
+func testAssertLikeUseDifferentBits() {
+    struct S : ~Copyable {
+        var s: [Int] = []
+        var currentPosition = 5
+
+        // CHECK-LABEL: sil private @$s23moveonly_addresschecker30testAssertLikeUseDifferentBitsyyF1SL_V6resume2atySi_tF : $@convention(method) (Int, @inout S) -> () {
+        // CHECK-NOT: destroy_addr
+        // CHECK: } // end sil function '$s23moveonly_addresschecker30testAssertLikeUseDifferentBitsyyF1SL_V6resume2atySi_tF'
+        mutating func resume(at index: Int) {
+            assert(index >= currentPosition)
+            currentPosition = index
+        }
+    }
+}
