Merge pull request #34820 from gottesmm/simple-lifetime-join

[semantic-arc-opts] Let the simple lifetime join optimization handle certain copies with forwarding insts.

Author: gottesmm

lib/SILOptimizer/SemanticARC/CopyValueOpts.cpp

@@ -289,23 +289,21 @@ bool SemanticARCOptVisitor::eliminateDeadLiveRangeCopyValue(
 static bool canSafelyJoinSimpleRange(SILValue cviOperand,
                                      DestroyValueInst *cviOperandDestroy,
                                      CopyValueInst *cvi) {
-  // We only handle cases where our copy_value has a single consuming use that
-  // is not a forwarding use. We need to use the LiveRange functionality to
-  // guarantee correctness in the presence of forwarding uses.
-  //
-  // NOTE: This use may be any type of consuming use and may not be a
-  // destroy_value.
+  // We only handle cases where our copy_value has a single lifetime ending
+  // use. We are not working with live ranges here so we do can treat forwarding
+  // insts like any other value.
   auto *cviConsumer = cvi->getSingleConsumingUse();
-  if (!cviConsumer || isOwnedForwardingUse(cviConsumer)) {
+  if (!cviConsumer) {
     return false;
   }
 
   // Ok, we may be able to eliminate this. The main thing we need to be careful
-  // of here is that if the destroy_value is /after/ the consuming use of the
-  // operand of copy_value, we may have normal uses of the copy_value's operand
-  // that would become use-after-frees since we would be shrinking the lifetime
-  // of the object potentially. Consider the following SIL:
+  // of here is that if the lifetime of %0 ends /after/ the lifetime of
+  // %1. Otherwise we would be shrinking the lifetime of the object
+  // potentially. Consider the following SIL that we would miscompile in such a
+  // case.
   //
+  //   // potential_miscompile.sil
   //   %0 = ...
   //   %1 = copy_value %0
   //   apply %cviConsumer(%1)

test/SILOptimizer/semantic-arc-opts.sil

@@ -90,6 +90,8 @@ struct StructWithEnumWithIndirectCaseField {
   var field : EnumWithIndirectCase
 }
 
+sil @get_fakeoptional_nativeobject : $@convention(thin) () -> @owned FakeOptional<Builtin.NativeObject>
+
 ///////////
 // Tests //
 ///////////
@@ -1747,10 +1749,8 @@ bb1:
   return %9999 : $()
 }
 
-// Forwarding case. We need LiveRanges for this.
-//
 // CHECK-LABEL: sil [ossa] @donot_join_simple_liveranges_in_same_block_2 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
-// CHECK: copy_value
+// CHECK-NOT: copy_value
 // CHECK: } // end sil function 'donot_join_simple_liveranges_in_same_block_2'
 sil [ossa] @donot_join_simple_liveranges_in_same_block_2 : $@convention(thin) (@owned Builtin.NativeObject) -> () {
 bb0(%0 : @owned $Builtin.NativeObject):
@@ -2832,3 +2832,39 @@ bb1:
 bb2:
   unreachable
 }
+
+// Make sure we leave only one copy in bb2 and no destroys
+//
+// CHECK-LABEL: sil [ossa] @join_test_with_forwarding_inst : $@convention(thin) () -> @owned FakeOptional<Builtin.NativeObject> {
+// CHECK: bb2:
+// CHECK: copy_value
+// CHECK-NOT: destroy_value
+// CHECK-NOT: copy_value
+// CHECK:   br bb3(
+// CHECK: } // end sil function 'join_test_with_forwarding_inst'
+sil [ossa] @join_test_with_forwarding_inst : $@convention(thin) () -> @owned FakeOptional<Builtin.NativeObject> {
+bb0:
+  %allocStack = alloc_stack $Builtin.NativeObject
+  %0 = function_ref @get_fakeoptional_nativeobject : $@convention(thin) () -> @owned FakeOptional<Builtin.NativeObject>
+  %1 = apply %0() : $@convention(thin) () -> @owned FakeOptional<Builtin.NativeObject>
+  cond_br undef, bb1, bb2
+
+bb1:
+  destroy_value %1 : $FakeOptional<Builtin.NativeObject>
+  %2 = enum $FakeOptional<Builtin.NativeObject>, #FakeOptional.none!enumelt
+  br bb3(%2 : $FakeOptional<Builtin.NativeObject>)
+
+bb2:
+  %3 = unchecked_enum_data %1 : $FakeOptional<Builtin.NativeObject>, #FakeOptional.some!enumelt
+  %4 = copy_value %3 : $Builtin.NativeObject
+  store %3 to [init] %allocStack : $*Builtin.NativeObject
+  %4c = copy_value %4 : $Builtin.NativeObject
+  destroy_value %4 : $Builtin.NativeObject
+  %5 = enum $FakeOptional<Builtin.NativeObject>, #FakeOptional.some!enumelt, %4c : $Builtin.NativeObject
+  destroy_addr %allocStack : $*Builtin.NativeObject
+  br bb3(%5 : $FakeOptional<Builtin.NativeObject>)
+
+bb3(%result : @owned $FakeOptional<Builtin.NativeObject>):
+  dealloc_stack %allocStack : $*Builtin.NativeObject
+  return %result : $FakeOptional<Builtin.NativeObject>
+}