[move-only] Make sure that we mask out liveness bits and use |= when merging successors

Both of these can cause us to insert destroy_addr in the wrong locations.

1. The first causes us to insert destroys for parts of values that are not
actually on the boundary since we didn't use our mask and instead used all of
the liveness information.

2. We were merging successor information using '&=' instead of '|=. This caused
a problem if we had multiple regions for the same successor. In such a case, we
would not have anything in common for the regions causing us to not have any
bits in common, resulting in us inserting too many destroy_addr instead of
skipping as we were supposed to.

rdar://112434492
(cherry picked from commit d7d7ab6)

Author: gottesmm

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;

lib/SILOptimizer/Mandatory/MoveOnlyAddressCheckerUtils.cpp

@@ -2761,7 +2761,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) {
@@ -2799,12 +2799,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;
             }

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

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