[semantic-arc-opts] Fix small bug around eliminating copies from begin_borrow, load_borrow, and dead end blocks.

TLDR:

The bug occurs since a copy_value does not need to be balanced by destroy_values
along paths that end in dead end blocks. So our check that all "consuming" uses
of the copy_value are within the lifetime of the begin_borrow, load_borrow
trivially succeed since there are no consuming uses of the copy_value to check!
This then results in us creating a use of a borrowed value after the borrowed
values end borrow.

I go through the bug in detail (via an example) and provide a proof that this
bug can only occur if the copy_value if there exists a set of dead end blocks
that jointly post-dominate the copy.

----

Consider the following SIL:

```
  %1 = begin_borrow %0 : $KlassPair                            (1)
  %2 = struct_extract %1 : $KlassPair, #KlassPair.firstKlass
  %3 = copy_value %2 : $Klass
  ...
  end_borrow %1 : $LintCommand                                 (2)
  cond_br ..., bb1, bb2

  ...

  bbN:
    // Never return type implies dead end block.
    apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> Never (3)
    unreachable
```

For simplicity, note that if bbN post-dominates %3, given that when we compute
linear lifetime errors we ignore dead end blocks, we would not register that the
copy_values only use is outside of the begin_borrow region defined by (1), (2)
and thus would eliminate the copy. This would result in %2 being used by %f,
causing the linear lifetime checker to error.

Naively one may assume that the solution to this is to just check if %3 has
/any/ destroy_values at all and if it doesn't have any reachable destroy_values,
then we are in this case. But is this correct in general? We prove this below:

The only paths along which the copy_value can not be destroyed or consumed is
along paths to dead end blocks. Trivially, we know that such a dead end block,
can not be reachable from the end_borrow since by their definition dead end
blocks do not have any successor instructions or blocks.

So we know that we can only run into this bug if we have a dead end block
reachable from the end_borrow, meaning that the bug can not occur if we branch
before the end_borrow since in that case, the borrow scope would last over the
dead end block's no return meaning that we will not use the borrowed value after
its lifetime is ended by the end_borrow.

With that in hand, we note again that if we have exactly one consumed,
destroy_value /after/ the end_borrow we will not optimize here since the
optimization only attempts to eliminate copies that are not consumed by
non-destroy_value instructions. This means that this bug can only occur if the
copy_value is only post-dominated by dead end blocks that use the value in a
non-consuming way.

NOTE: This can only occur if the copy_value is from a "borrow introducer" that
is associated with an end_borrow. This today includes begin_borrow, load_borrow,
but importantly and notably not function arguments.

rdar://54788632

Author: gottesmm

lib/SILOptimizer/Mandatory/SemanticARCOpts.cpp

@@ -340,6 +340,62 @@ bool SemanticARCOptVisitor::performGuaranteedCopyValueOptimization(CopyValueInst
   if (isConsumed(cvi, destroys, &guaranteedForwardingInsts))
     return false;
 
+  // Next check if we have any destroys at all of our copy_value and an operand
+  // that is not a function argument. Otherwise, due to the way we ignore dead
+  // end blocks, we may eliminate the copy_value, creating a use of the borrowed
+  // value after the end_borrow.
+  //
+  // DISCUSSION: Consider the following SIL:
+  //
+  // ```
+  //   %1 = begin_borrow %0 : $KlassPair                            (1)
+  //   %2 = struct_extract %1 : $KlassPair, #KlassPair.firstKlass
+  //   %3 = copy_value %2 : $Klass
+  //   ...
+  //   end_borrow %1 : $LintCommand                                 (2)
+  //   cond_br ..., bb1, bb2
+  //
+  //   ...
+  //
+  //   bbN:
+  //     // Never return type implies dead end block.
+  //     apply %f(%3) : $@convention(thin) (@guaranteed Klass) -> Never (3)
+  //     unreachable
+  // ```
+  //
+  // For simplicity, note that if bbN post-dominates %3, given that when we
+  // compute linear lifetime errors we ignore dead end blocks, we would not
+  // register that the copy_values only use is outside of the begin_borrow
+  // region defined by (1), (2) and thus would eliminate the copy. This would
+  // result in %2 being used by %f, causing the linear lifetime checker to
+  // error.
+  //
+  // Naively one may assume that the solution to this is to just check if %3 has
+  // /any/ destroy_values at all and if it doesn't have any reachable
+  // destroy_values, then we are in this case. But is this correct in
+  // general. We prove this below:
+  //
+  // The only paths along which the copy_value can not be destroyed or consumed
+  // is along paths to dead end blocks. Trivially, we know that such a dead end
+  // block, can not be reachable from the end_borrow since by their nature dead
+  // end blocks end in unreachables.
+  //
+  // So we know that we can only run into this bug if we have a dead end block
+  // reachable from the end_borrow, meaning that the bug can not occur if we
+  // branch before the end_borrow since in that case, the borrow scope would
+  // last over the dead end block's no return meaning that we will not use the
+  // borrowed value after its lifetime is ended by the end_borrow.
+  //
+  // With that in hand, we note again that if we have exactly one consumed,
+  // destroy_value /after/ the end_borrow we will not optimize here. This means
+  // that this bug can only occur if the copy_value is only post-dominated by
+  // dead end blocks that use the value in a non-consuming way.
+  if (destroys.empty() && llvm::any_of(borrowIntroducers, [](SILValue v) {
+        return !isa<SILFunctionArgument>(v);
+      })) {
+    return false;
+  }
+
   // If we reached this point, then we know that all of our users can
   // accept a guaranteed value and our owned value is destroyed only
   // by destroy_value. Check if all of our destroys are joint

test/SILOptimizer/semantic-arc-opts.sil

@@ -8,9 +8,12 @@ import Builtin
 // Declarations //
 //////////////////
 
+enum MyNever {}
+
 sil @guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> ()
 sil @owned_user : $@convention(thin) (@owned Builtin.NativeObject) -> ()
 sil @get_owned_obj : $@convention(thin) () -> @owned Builtin.NativeObject
+sil @unreachable_guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
 
 struct NativeObjectPair {
   var obj1 : Builtin.NativeObject
@@ -775,4 +778,63 @@ bb0(%0 : $*NativeObjectPair):
   destroy_addr %0 : $*NativeObjectPair
   %9999 = tuple()
   return %9999 : $()
-}
+}
+
+// Make sure we do not eliminate the copy_value below to ensure that all uses of
+// %2 are before %2's end_borrow.
+//
+// We used to eliminate the copy_value and change %func to use %2.
+//
+// CHECK-LABEL: sil [ossa] @begin_borrow_used_by_postdominating_no_return_function : $@convention(thin) () -> MyNever {
+// CHECK: copy_value
+// CHECK: } // end sil function 'begin_borrow_used_by_postdominating_no_return_function'
+sil [ossa] @begin_borrow_used_by_postdominating_no_return_function : $@convention(thin) () -> MyNever {
+bb0:
+  %0 = function_ref @get_nativeobject_pair : $@convention(thin) () -> @owned NativeObjectPair
+  %1 = apply %0() : $@convention(thin) () -> @owned NativeObjectPair
+  %2 = begin_borrow %1 : $NativeObjectPair
+  %3 = struct_extract %2 : $NativeObjectPair, #NativeObjectPair.obj1
+  %4 = copy_value %3 : $Builtin.NativeObject
+  end_borrow %2 : $NativeObjectPair
+  %func = function_ref @unreachable_guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
+  apply %func(%4) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
+  unreachable
+}
+
+// Make sure we do not eliminate the copy_value below to ensure that all uses of
+// %2 are before %2's end_borrow.
+//
+// We used to eliminate the copy_value and change %func to use %2.
+//
+// CHECK-LABEL: sil [ossa] @load_borrow_used_by_postdominating_no_return_function : $@convention(thin) () -> MyNever {
+// CHECK: copy_value
+// CHECK: } // end sil function 'load_borrow_used_by_postdominating_no_return_function'
+sil [ossa] @load_borrow_used_by_postdominating_no_return_function : $@convention(thin) () -> MyNever {
+bb0:
+  %0 = function_ref @get_nativeobject_pair : $@convention(thin) () -> @owned NativeObjectPair
+  %1 = apply %0() : $@convention(thin) () -> @owned NativeObjectPair
+  %stackSlot = alloc_stack $NativeObjectPair
+  store %1 to [init] %stackSlot : $*NativeObjectPair
+  %2 = load_borrow %stackSlot : $*NativeObjectPair
+  %3 = struct_extract %2 : $NativeObjectPair, #NativeObjectPair.obj1
+  %4 = copy_value %3 : $Builtin.NativeObject
+  end_borrow %2 : $NativeObjectPair
+  %func = function_ref @unreachable_guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
+  apply %func(%4) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
+  unreachable
+}
+
+// Make sure we do perform the optimization if our borrowed value is an
+// argument.
+//
+// CHECK-LABEL: sil [ossa] @guaranteed_arg_used_by_postdominating_no_return_function : $@convention(thin) (@guaranteed NativeObjectPair) -> MyNever {
+// CHECK-NOT: copy_value
+// CHECK: } // end sil function 'guaranteed_arg_used_by_postdominating_no_return_function'
+sil [ossa] @guaranteed_arg_used_by_postdominating_no_return_function : $@convention(thin) (@guaranteed NativeObjectPair) -> MyNever {
+bb0(%0 : @guaranteed $NativeObjectPair):
+  %3 = struct_extract %0 : $NativeObjectPair, #NativeObjectPair.obj1
+  %4 = copy_value %3 : $Builtin.NativeObject
+  %func = function_ref @unreachable_guaranteed_user : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
+  apply %func(%4) : $@convention(thin) (@guaranteed Builtin.NativeObject) -> MyNever
+  unreachable
+}