Skip to content

Commit 9cff942

Browse files
gottesmmgottesmm
committed
[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.
1 parent cf2c478 commit 9cff942

File tree

2 files changed

+175
-0
lines changed

2 files changed

+175
-0
lines changed

‎lib/SILOptimizer/Mandatory/PredictableMemOpt.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1287,6 +1287,9 @@ bool AllocOptimize::tryToRemoveDeadAllocation() {
12871287

12881288
switch (u.Kind) {
12891289
case PMOUseKind::Assign:
1290+
// Until we can promote the value being destroyed by the assign, we can
1291+
// not remove deallocations with such assigns.
1292+
return false;
12901293
case PMOUseKind::InitOrAssign:
12911294
break; // These don't prevent removal.
12921295
case PMOUseKind::Initialization:

‎test/SILOptimizer/predictable_deadalloc_elim.sil

Lines changed: 172 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,3 +93,175 @@ bb0(%0 : $Builtin.NativeObject):
9393
%9999 = tuple()
9494
return %9999 : $()
9595
}
96+
97+
//////////////////
98+
// Assign Tests //
99+
//////////////////
100+
101+
// Make sure that we do eliminate this allocation
102+
// CHECK-LABEL: sil @simple_assign_take_trivial : $@convention(thin) (Builtin.Int32, @in Builtin.Int32) -> () {
103+
// CHECK-NOT: alloc_stack
104+
// CHECK: } // end sil function 'simple_assign_take_trivial'
105+
sil @simple_assign_take_trivial : $@convention(thin) (Builtin.Int32, @in Builtin.Int32) -> () {
106+
bb0(%0 : $Builtin.Int32, %1 : $*Builtin.Int32):
107+
%2 = alloc_stack $Builtin.Int32
108+
store %0 to %2 : $*Builtin.Int32
109+
copy_addr [take] %1 to %2 : $*Builtin.Int32
110+
dealloc_stack %2 : $*Builtin.Int32
111+
%9999 = tuple()
112+
return %9999 : $()
113+
}
114+
115+
// In this case, we perform an init, copy. Since we do not want to lose the +1
116+
// on the argument, we do not eliminate this (even though with time perhaps we
117+
// could).
118+
// CHECK-LABEL: sil @simple_init_copy : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
119+
// CHECK: alloc_stack
120+
// CHECK: copy_addr
121+
// CHECK: } // end sil function 'simple_init_copy'
122+
sil @simple_init_copy : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
123+
bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
124+
%2 = alloc_stack $Builtin.NativeObject
125+
store %0 to %2 : $*Builtin.NativeObject
126+
destroy_addr %2 : $*Builtin.NativeObject
127+
copy_addr %1 to [initialization] %2 : $*Builtin.NativeObject
128+
destroy_addr %2 : $*Builtin.NativeObject
129+
dealloc_stack %2 : $*Builtin.NativeObject
130+
%9999 = tuple()
131+
return %9999 : $()
132+
}
133+
134+
// This we can promote successfully.
135+
// CHECK-LABEL: sil @simple_init_take : $@convention(thin) (@owned Builtin.NativeObject, @in Builtin.NativeObject) -> () {
136+
// CHECK: bb0([[ARG0:%.*]] : $Builtin.NativeObject, [[ARG1:%.*]] : $*Builtin.NativeObject):
137+
// CHECK-NOT: alloc_stack
138+
// CHECK: strong_release [[ARG0]]
139+
// CHECK: [[ARG1_LOADED:%.*]] = load [[ARG1]]
140+
// CHECK: strong_release [[ARG1_LOADED]]
141+
// CHECK: } // end sil function 'simple_init_take'
142+
sil @simple_init_take : $@convention(thin) (@owned Builtin.NativeObject, @in Builtin.NativeObject) -> () {
143+
bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
144+
%2 = alloc_stack $Builtin.NativeObject
145+
store %0 to %2 : $*Builtin.NativeObject
146+
destroy_addr %2 : $*Builtin.NativeObject
147+
copy_addr [take] %1 to [initialization] %2 : $*Builtin.NativeObject
148+
destroy_addr %2 : $*Builtin.NativeObject
149+
dealloc_stack %2 : $*Builtin.NativeObject
150+
%9999 = tuple()
151+
return %9999 : $()
152+
}
153+
154+
// Since we are copying the input argument, we can not get rid of the copy_addr,
155+
// meaning we shouldn't eliminate the allocation here.
156+
// CHECK-LABEL: sil @simple_assign_no_take : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
157+
// CHECK: alloc_stack
158+
// CHECK: copy_addr
159+
// CHECK: } // end sil function 'simple_assign_no_take'
160+
sil @simple_assign_no_take : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
161+
bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
162+
%2 = alloc_stack $Builtin.NativeObject
163+
store %0 to %2 : $*Builtin.NativeObject
164+
copy_addr %1 to %2 : $*Builtin.NativeObject
165+
destroy_addr %2 : $*Builtin.NativeObject
166+
dealloc_stack %2 : $*Builtin.NativeObject
167+
%9999 = tuple()
168+
return %9999 : $()
169+
}
170+
171+
// If PMO understood how to promote assigns, we should be able to handle this
172+
// case.
173+
// CHECK-LABEL: sil @simple_assign_take : $@convention(thin) (@owned Builtin.NativeObject, @in Builtin.NativeObject) -> () {
174+
// CHECK: alloc_stack
175+
// CHECK: copy_addr
176+
// CHECK: } // end sil function 'simple_assign_take'
177+
sil @simple_assign_take : $@convention(thin) (@owned Builtin.NativeObject, @in Builtin.NativeObject) -> () {
178+
bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
179+
%2 = alloc_stack $Builtin.NativeObject
180+
store %0 to %2 : $*Builtin.NativeObject
181+
copy_addr [take] %1 to %2 : $*Builtin.NativeObject
182+
destroy_addr %2 : $*Builtin.NativeObject
183+
dealloc_stack %2 : $*Builtin.NativeObject
184+
%9999 = tuple()
185+
return %9999 : $()
186+
}
187+
188+
// CHECK-LABEL: sil @simple_diamond_without_assign : $@convention(thin) (@owned Builtin.NativeObject) -> () {
189+
// CHECK: bb0([[ARG:%.*]] :
190+
// CHECK-NOT: alloc_stack
191+
// CHECK-NOT: store
192+
// CHECK: bb3:
193+
// CHECK-NEXT: strong_release [[ARG]]
194+
// CHECK: } // end sil function 'simple_diamond_without_assign'
195+
sil @simple_diamond_without_assign : $@convention(thin) (@owned Builtin.NativeObject) -> () {
196+
bb0(%0 : $Builtin.NativeObject):
197+
%1 = alloc_stack $Builtin.NativeObject
198+
store %0 to %1 : $*Builtin.NativeObject
199+
cond_br undef, bb1, bb2
200+
201+
bb1:
202+
br bb3
203+
204+
bb2:
205+
br bb3
206+
207+
bb3:
208+
destroy_addr %1 : $*Builtin.NativeObject
209+
dealloc_stack %1 : $*Builtin.NativeObject
210+
%9999 = tuple()
211+
return %9999 : $()
212+
}
213+
214+
// We should not promote this due to this being an assign to %2.
215+
// CHECK-LABEL: sil @simple_diamond_with_assign : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
216+
// CHECK: alloc_stack
217+
// CHECK: copy_addr
218+
// CHECK: } // end sil function 'simple_diamond_with_assign'
219+
sil @simple_diamond_with_assign : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
220+
bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
221+
%2 = alloc_stack $Builtin.NativeObject
222+
store %0 to %2 : $*Builtin.NativeObject
223+
cond_br undef, bb1, bb2
224+
225+
bb1:
226+
copy_addr [take] %1 to %2 : $*Builtin.NativeObject
227+
br bb3
228+
229+
bb2:
230+
br bb3
231+
232+
bb3:
233+
destroy_addr %2 : $*Builtin.NativeObject
234+
dealloc_stack %2 : $*Builtin.NativeObject
235+
%9999 = tuple()
236+
return %9999 : $()
237+
}
238+
239+
// Today PMO can not handle different available values coming from different
240+
// BBs. With time it can be taught to do that if necessary. That being said,
241+
// this test shows that we /tried/ and failed with the available value test
242+
// instead of failing earlier due to the copy_addr being an assign since we
243+
// explode the copy_addr.
244+
// CHECK-LABEL: sil @simple_diamond_with_assign_remove : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
245+
// CHECK: alloc_stack
246+
// CHECK-NOT: copy_addr
247+
// CHECK: } // end sil function 'simple_diamond_with_assign_remove'
248+
sil @simple_diamond_with_assign_remove : $@convention(thin) (@owned Builtin.NativeObject, @in_guaranteed Builtin.NativeObject) -> () {
249+
bb0(%0 : $Builtin.NativeObject, %1 : $*Builtin.NativeObject):
250+
%2 = alloc_stack $Builtin.NativeObject
251+
store %0 to %2 : $*Builtin.NativeObject
252+
cond_br undef, bb1, bb2
253+
254+
bb1:
255+
destroy_addr %2 : $*Builtin.NativeObject
256+
copy_addr [take] %1 to [initialization] %2 : $*Builtin.NativeObject
257+
br bb3
258+
259+
bb2:
260+
br bb3
261+
262+
bb3:
263+
destroy_addr %2 : $*Builtin.NativeObject
264+
dealloc_stack %2 : $*Builtin.NativeObject
265+
%9999 = tuple()
266+
return %9999 : $()
267+
}

0 commit comments

Comments
 (0)