[pmo] Assigns should result in PMO not eliminating an allocation.

lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp

@@ -1287,6 +1287,9 @@ bool AllocOptimize::tryToRemoveDeadAllocation() {
 
     switch (u.Kind) {
     case PMOUseKind::Assign:
+      // Until we can promote the value being destroyed by the assign, we can
+      // not remove deallocations with such assigns.
+      return false;
     case PMOUseKind::InitOrAssign:
       break;    // These don't prevent removal.
     case PMOUseKind::Initialization:

test/SILOptimizer/predictable_deadalloc_elim.sil

@@ -93,3 +93,175 @@ bb0(%0 : $Builtin.NativeObject):
   %9999 = tuple()
   return %9999 : $()
 }
+
+//////////////////
+// Assign Tests //
+//////////////////
+
+// Make sure that we do eliminate this allocation
+// CHECK-LABEL: sil @simple_assign_take_trivial : $@convention(thin) (Builtin.Int32, @in Builtin.Int32) -> () {
+// CHECK-NOT: alloc_stack
+// CHECK: } // end sil function 'simple_assign_take_trivial'
+sil @simple_assign_take_trivial : $@convention(thin) (Builtin.Int32, @in Builtin.Int32) -> () {
+bb0(%0 : $Builtin.Int32, %1 : $*Builtin.Int32):
+  %2 = alloc_stack $Builtin.Int32
+  store %0 to %2 : $*Builtin.Int32
+  copy_addr [take] %1 to %2 : $*Builtin.Int32
+  dealloc_stack %2 : $*Builtin.Int32
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// In this case, we perform an init, copy. Since we do not want to lose the +1
+// on the argument, we do not eliminate this (even though with time perhaps we
+// could).
+// CHECK-LABEL: sil @simple_init_copy : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
+// CHECK: alloc_stack
+// CHECK: copy_addr
+// CHECK: } // end sil function 'simple_init_copy'
+sil @simple_init_copy : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
+bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
+  %2 = alloc_stack $Builtin.NativeObject
+  store %0 to %2 : $*Builtin.NativeObject
+  destroy_addr %2 : $*Builtin.NativeObject
+  copy_addr %1 to [initialization] %2 : $*Builtin.NativeObject
+  destroy_addr %2 : $*Builtin.NativeObject
+  dealloc_stack %2 : $*Builtin.NativeObject
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// This we can promote successfully.
+// CHECK-LABEL: sil @simple_init_take : $@convention(thin) (@owned Builtin.NativeObject, @in Builtin.NativeObject) -> () {
+// CHECK: bb0([[ARG0:%.*]] : $Builtin.NativeObject, [[ARG1:%.*]] : $*Builtin.NativeObject):
+// CHECK-NOT: alloc_stack
+// CHECK:  strong_release [[ARG0]]
+// CHECK:  [[ARG1_LOADED:%.*]] = load [[ARG1]]
+// CHECK:  strong_release [[ARG1_LOADED]]
+// CHECK: } // end sil function 'simple_init_take'
+sil @simple_init_take : $@convention(thin) (@owned Builtin.NativeObject, @in Builtin.NativeObject) -> () {
+bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
+  %2 = alloc_stack $Builtin.NativeObject
+  store %0 to %2 : $*Builtin.NativeObject
+  destroy_addr %2 : $*Builtin.NativeObject
+  copy_addr [take] %1 to [initialization] %2 : $*Builtin.NativeObject
+  destroy_addr %2 : $*Builtin.NativeObject
+  dealloc_stack %2 : $*Builtin.NativeObject
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// Since we are copying the input argument, we can not get rid of the copy_addr,
+// meaning we shouldn't eliminate the allocation here.
+// CHECK-LABEL: sil @simple_assign_no_take : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
+// CHECK: alloc_stack
+// CHECK: copy_addr
+// CHECK: } // end sil function 'simple_assign_no_take'
+sil @simple_assign_no_take : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
+bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
+  %2 = alloc_stack $Builtin.NativeObject
+  store %0 to %2 : $*Builtin.NativeObject
+  copy_addr %1 to %2 : $*Builtin.NativeObject
+  destroy_addr %2 : $*Builtin.NativeObject
+  dealloc_stack %2 : $*Builtin.NativeObject
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// If PMO understood how to promote assigns, we should be able to handle this
+// case.
+// CHECK-LABEL: sil @simple_assign_take : $@convention(thin) (@owned Builtin.NativeObject, @in Builtin.NativeObject) -> () {
+// CHECK: alloc_stack
+// CHECK: copy_addr
+// CHECK: } // end sil function 'simple_assign_take'
+sil @simple_assign_take : $@convention(thin) (@owned Builtin.NativeObject, @in Builtin.NativeObject) -> () {
+bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
+  %2 = alloc_stack $Builtin.NativeObject
+  store %0 to %2 : $*Builtin.NativeObject
+  copy_addr [take] %1 to %2 : $*Builtin.NativeObject
+  destroy_addr %2 : $*Builtin.NativeObject
+  dealloc_stack %2 : $*Builtin.NativeObject
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// CHECK-LABEL: sil @simple_diamond_without_assign : $@convention(thin) (@owned Builtin.NativeObject) -> () {
+// CHECK: bb0([[ARG:%.*]] :
+// CHECK-NOT: alloc_stack
+// CHECK-NOT: store
+// CHECK: bb3:
+// CHECK-NEXT: strong_release [[ARG]]
+// CHECK: } // end sil function 'simple_diamond_without_assign'
+sil @simple_diamond_without_assign : $@convention(thin) (@owned Builtin.NativeObject) -> () {
+bb0(%0 : $Builtin.NativeObject):
+  %1 = alloc_stack $Builtin.NativeObject
+  store %0 to %1 : $*Builtin.NativeObject
+  cond_br undef, bb1, bb2
+
+bb1:
+  br bb3
+
+bb2:
+  br bb3
+
+bb3:
+  destroy_addr %1 : $*Builtin.NativeObject
+  dealloc_stack %1 : $*Builtin.NativeObject
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// We should not promote this due to this being an assign to %2.
+// CHECK-LABEL: sil @simple_diamond_with_assign : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
+// CHECK: alloc_stack
+// CHECK: copy_addr
+// CHECK: } // end sil function 'simple_diamond_with_assign'
+sil @simple_diamond_with_assign : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
+bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
+  %2 = alloc_stack $Builtin.NativeObject
+  store %0 to %2 : $*Builtin.NativeObject
+  cond_br undef, bb1, bb2
+
+bb1:
+  copy_addr [take] %1 to %2 : $*Builtin.NativeObject
+  br bb3
+
+bb2:
+  br bb3
+
+bb3:
+  destroy_addr %2 : $*Builtin.NativeObject
+  dealloc_stack %2 : $*Builtin.NativeObject
+  %9999 = tuple()
+  return %9999 : $()
+}
+
+// Today PMO can not handle different available values coming from different
+// BBs. With time it can be taught to do that if necessary. That being said,
+// this test shows that we /tried/ and failed with the available value test
+// instead of failing earlier due to the copy_addr being an assign since we
+// explode the copy_addr.
+// CHECK-LABEL: sil @simple_diamond_with_assign_remove : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
+// CHECK: alloc_stack
+// CHECK-NOT: copy_addr
+// CHECK: } // end sil function 'simple_diamond_with_assign_remove'
+sil @simple_diamond_with_assign_remove : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
+bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
+  %2 = alloc_stack $Builtin.NativeObject
+  store %0 to %2 : $*Builtin.NativeObject
+  cond_br undef, bb1, bb2
+
+bb1:
+  destroy_addr %2 : $*Builtin.NativeObject
+  copy_addr [take] %1 to [initialization] %2 : $*Builtin.NativeObject
+  br bb3
+
+bb2:
+  br bb3
+
+bb3:
+  destroy_addr %2 : $*Builtin.NativeObject
+  dealloc_stack %2 : $*Builtin.NativeObject
+  %9999 = tuple()
+  return %9999 : $()
+}