[pmo] Assigns should result in PMO not eliminating an allocation.
The reason this is true is that an assign is an instruction in PMO parlance that
must destroy the value stored into memory at that location previously. So PMO
would need to be taught to ensure that said destroy is promoted. Consider the
following example:
%0 = alloc_stack $Foo
store %1 to [init] %0 : $*Foo
store %2 to [assign] %0 : $*Foo
destroy_addr %0 : $*Foo
dealloc_stack %0 : $*Foo
If PMO were to try to eliminate the alloc_stack as PMO is written today, one
would have:
destroy_value %2 : $Foo
That is clearly wrong.
diff --git a/lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp b/lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp
index 9121685..3a36c73 100644
--- a/lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp
+++ b/lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp
@@ -1287,6 +1287,9 @@
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:
diff --git a/test/SILOptimizer/predictable_deadalloc_elim.sil b/test/SILOptimizer/predictable_deadalloc_elim.sil
index 44acb44..7e40d5a 100644
--- a/test/SILOptimizer/predictable_deadalloc_elim.sil
+++ b/test/SILOptimizer/predictable_deadalloc_elim.sil
@@ -93,3 +93,175 @@
%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 : $()
+}
